From 2313b1b03fcf46ae8d4d4a73e57f8d32cab76b77 Mon Sep 17 00:00:00 2001 From: Arun Babu Neelicattu Date: Sun, 12 Apr 2020 15:10:40 +0200 Subject: [PATCH] Improve support for PEP-440 direct references With this change we allow for PEP-508 string for file dependencies to use PEP-440 direct reference using the file URI scheme (RFC-8089). Note that this implementation will not allow non RFC-8089 file path references. URI will maintain relative paths in order to allow for sdist to be portable in all sane use cases. References: - https://www.python.org/dev/peps/pep-0508/#backwards-compatibility - https://www.python.org/dev/peps/pep-0440/#direct-references - https://tools.ietf.org/html/rfc8089 - https://discuss.python.org/t/what-is-the-correct-interpretation-of-path-based-pep-508-uri-reference/2815/11 --- poetry/core/packages/__init__.py | 6 ++- poetry/core/packages/file_dependency.py | 13 +++++ poetry/core/packages/utils/utils.py | 64 +++++++++++++++++------- poetry/core/utils/_compat.py | 8 +-- poetry/core/version/requirements.py | 7 ++- tests/masonry/builders/test_builder.py | 12 ++--- tests/packages/test_file_dependency.py | 39 +++++++++++++++ tests/packages/utils/test_utils_urls.py | 66 +++++++++++++++++++++++++ 8 files changed, 185 insertions(+), 30 deletions(-) create mode 100644 tests/packages/utils/test_utils_urls.py diff --git a/poetry/core/packages/__init__.py b/poetry/core/packages/__init__.py index 3e98a5398..39d5ee117 100644 --- a/poetry/core/packages/__init__.py +++ b/poetry/core/packages/__init__.py @@ -19,6 +19,7 @@ from .utils.utils import is_url from .utils.utils import path_to_url from .utils.utils import strip_extras +from .utils.utils import url_to_path from .vcs_dependency import VCSDependency @@ -84,7 +85,10 @@ def dependency_from_pep_508(name): elif link.scheme == "git": dep = VCSDependency(name, "git", link.url_without_fragment) elif link.scheme in ["http", "https"]: - dep = URLDependency(name, link.url_without_fragment) + dep = URLDependency(name, link.url) + elif link.scheme == "file": + # handle RFC 8089 references + dep = FileDependency(name, url_to_path(req.url)) else: dep = Dependency(name, "*") else: diff --git a/poetry/core/packages/file_dependency.py b/poetry/core/packages/file_dependency.py index aee85e6d5..c3ae16ae2 100644 --- a/poetry/core/packages/file_dependency.py +++ b/poetry/core/packages/file_dependency.py @@ -4,6 +4,7 @@ from poetry.core._vendor.pkginfo.distribution import HEADER_ATTRS from poetry.core._vendor.pkginfo.distribution import HEADER_ATTRS_2_0 +from poetry.core.packages.utils.utils import path_to_url from poetry.core.utils._compat import Path from .dependency import Dependency @@ -59,3 +60,15 @@ def hash(self): h.update(content) return h.hexdigest() + + @property + def base_pep_508_name(self): # type: () -> str + requirement = self.pretty_name + + if self.extras: + requirement += "[{}]".format(",".join(self.extras)) + + path = path_to_url(self.path, absolute=False) + requirement += " @ {}".format(path) + + return requirement diff --git a/poetry/core/packages/utils/utils.py b/poetry/core/packages/utils/utils.py index 77ac0746e..7916ab72f 100644 --- a/poetry/core/packages/utils/utils.py +++ b/poetry/core/packages/utils/utils.py @@ -1,6 +1,14 @@ import os import posixpath import re +import sys + +from typing import Union + +from poetry.core._vendor.six.moves.urllib.parse import urljoin +from poetry.core._vendor.six.moves.urllib.parse import urlsplit +from poetry.core._vendor.six.moves.urllib.request import pathname2url +from poetry.core._vendor.six.moves.urllib.request import url2pathname from poetry.core.packages.constraints.constraint import Constraint from poetry.core.packages.constraints.multi_constraint import MultiConstraint @@ -11,24 +19,13 @@ from poetry.core.semver import VersionRange from poetry.core.semver import VersionUnion from poetry.core.semver import parse_constraint +from poetry.core.utils._compat import Path from poetry.core.version.markers import BaseMarker from poetry.core.version.markers import MarkerUnion from poetry.core.version.markers import MultiMarker from poetry.core.version.markers import SingleMarker -try: - import urllib.parse as urlparse -except ImportError: - import urlparse - - -try: - import urllib.request as urllib2 -except ImportError: - import urllib2 - - BZ2_EXTENSIONS = (".tar.bz2", ".tbz") XZ_EXTENSIONS = (".tar.xz", ".txz", ".tlz", ".tar.lz", ".tar.lzma") ZIP_EXTENSIONS = (".zip", ".whl") @@ -52,16 +49,49 @@ pass -def path_to_url(path): +def path_to_url(path, absolute=True): # type: (Union[str, Path], bool) -> str """ - Convert a path to a file: URL. The path will be made absolute and have - quoted path parts. + Convert a path to a file: URL. The path will be made absolute unless otherwise + specified and have quoted path parts. """ - path = os.path.normpath(os.path.abspath(path)) - url = urlparse.urljoin("file:", urllib2.pathname2url(path)) + path = str(path) + if absolute: + path = os.path.abspath(path) + + path = os.path.normpath(path) + url = urljoin("file:", pathname2url(path)) return url +def url_to_path(url): # type: (str) -> Path + """ + Convert an RFC8089 file URI to path. + + The logic used here is borrowed from pip + https://github.com/pypa/pip/blob/4d1932fcdd1974c820ea60b3286984ebb0c3beaa/src/pip/_internal/utils/urls.py#L31 + """ + if not url.startswith("file:"): + raise ValueError("{} is not a valid file URI".format(url)) + + _, netloc, path, _, _ = urlsplit(url) + unc_share = "\\\\" + + is_relative = netloc in {".", ".."} + + if not netloc or netloc == "localhost": + # According to RFC 8089, same as empty authority. + netloc = "" + elif not is_relative and sys.platform == "win32": + # If we have a UNC path, prepend UNC share notation. + netloc = unc_share + netloc + elif not is_relative: + raise ValueError( + "non-local file URIs are not supported on this platform: {}".format(url) + ) + + return Path(url2pathname(netloc + path)) + + def is_url(name): if ":" not in name: return False diff --git a/poetry/core/utils/_compat.py b/poetry/core/utils/_compat.py index 9935e102b..4c96e1ce9 100644 --- a/poetry/core/utils/_compat.py +++ b/poetry/core/utils/_compat.py @@ -1,10 +1,10 @@ import sys +import poetry.core._vendor.six.moves.urllib.parse as urllib_parse + + +urlparse = urllib_parse -try: - import urllib.parse as urlparse -except ImportError: - import urlparse try: # Python 2 long = long diff --git a/poetry/core/version/requirements.py b/poetry/core/version/requirements.py index d2748cb53..dab8f8ce9 100644 --- a/poetry/core/version/requirements.py +++ b/poetry/core/version/requirements.py @@ -216,10 +216,13 @@ def __init__(self, requirement_string): self.name = req.name if req.url: parsed_url = urlparse.urlparse(req.url) - if not (parsed_url.scheme and parsed_url.netloc) or ( + if parsed_url.scheme == "file": + if urlparse.urlunparse(parsed_url) != req.url: + raise InvalidRequirement("Invalid URL given") + elif not (parsed_url.scheme and parsed_url.netloc) or ( not parsed_url.scheme and not parsed_url.netloc ): - raise InvalidRequirement("Invalid URL given") + raise InvalidRequirement("Invalid URL: {0}".format(req.url)) self.url = req.url else: self.url = None diff --git a/tests/masonry/builders/test_builder.py b/tests/masonry/builders/test_builder.py index 39de435f1..0b82599f9 100644 --- a/tests/masonry/builders/test_builder.py +++ b/tests/masonry/builders/test_builder.py @@ -24,7 +24,7 @@ def test_builder_find_case_sensitive_excluded_files(mocker): builder = Builder( Factory().create_poetry( Path(__file__).parent / "fixtures" / "case_sensitive_exclusions" - ), + ) ) assert builder.find_excluded_files() == { @@ -45,7 +45,7 @@ def test_builder_find_invalid_case_sensitive_excluded_files(mocker): builder = Builder( Factory().create_poetry( Path(__file__).parent / "fixtures" / "invalid_case_sensitive_exclusions" - ), + ) ) assert {"my_package/Bar/foo/bar/Foo.py"} == builder.find_excluded_files() @@ -53,7 +53,7 @@ def test_builder_find_invalid_case_sensitive_excluded_files(mocker): def test_get_metadata_content(): builder = Builder( - Factory().create_poetry(Path(__file__).parent / "fixtures" / "complete"), + Factory().create_poetry(Path(__file__).parent / "fixtures" / "complete") ) metadata = builder.get_metadata_content() @@ -103,7 +103,7 @@ def test_get_metadata_content(): def test_metadata_homepage_default(): builder = Builder( - Factory().create_poetry(Path(__file__).parent / "fixtures" / "simple_version"), + Factory().create_poetry(Path(__file__).parent / "fixtures" / "simple_version") ) metadata = Parser().parsestr(builder.get_metadata_content()) @@ -115,7 +115,7 @@ def test_metadata_with_vcs_dependencies(): builder = Builder( Factory().create_poetry( Path(__file__).parent / "fixtures" / "with_vcs_dependency" - ), + ) ) metadata = Parser().parsestr(builder.get_metadata_content()) @@ -129,7 +129,7 @@ def test_metadata_with_url_dependencies(): builder = Builder( Factory().create_poetry( Path(__file__).parent / "fixtures" / "with_url_dependency" - ), + ) ) metadata = Parser().parsestr(builder.get_metadata_content()) diff --git a/tests/packages/test_file_dependency.py b/tests/packages/test_file_dependency.py index e80b35f9e..d0db7806a 100644 --- a/tests/packages/test_file_dependency.py +++ b/tests/packages/test_file_dependency.py @@ -1,6 +1,7 @@ import pytest from poetry.core.packages import FileDependency +from poetry.core.packages import dependency_from_pep_508 from poetry.core.utils._compat import Path @@ -15,3 +16,41 @@ def test_file_dependency_wrong_path(): def test_file_dependency_dir(): with pytest.raises(ValueError): FileDependency("demo", DIST_PATH) + + +def _test_file_dependency_pep_508( + mocker, name, path, pep_508_input, pep_508_output=None +): + mock_path_exists = mocker.patch.object(Path, "exists") + mock_path_exists.return_value = True + + dep = dependency_from_pep_508(pep_508_input) + + assert dep.is_file() + assert dep.name == name + assert dep.path == path + assert dep.to_pep_508() == pep_508_output or pep_508_input + + +def test_file_dependency_pep_508_local_file_absolute(mocker): + path = DIST_PATH / "demo-0.2.0.tar.gz" + requirement = "{} @ file://{}".format("demo", path) + + _test_file_dependency_pep_508(mocker, "demo", path, requirement) + + +def test_file_dependency_pep_508_local_file_localhost(mocker): + path = DIST_PATH / "demo-0.2.0.tar.gz" + requirement = "{} @ file://localhost{}".format("demo", path) + requirement_expected = "{} @ file://{}".format("demo", path) + + _test_file_dependency_pep_508( + mocker, "demo", path, requirement, requirement_expected + ) + + +def test_file_dependency_pep_508_local_file_relative_path(mocker): + path = Path("..") / "fixtures" / "distributions" / "demo-0.2.0.tar.gz" + requirement = "{} @ file://{}".format("demo", path) + + _test_file_dependency_pep_508(mocker, "demo", path, requirement) diff --git a/tests/packages/utils/test_utils_urls.py b/tests/packages/utils/test_utils_urls.py new file mode 100644 index 000000000..481778e0d --- /dev/null +++ b/tests/packages/utils/test_utils_urls.py @@ -0,0 +1,66 @@ +# These test scenarios are ported over from pypa/pip +# https://raw.githubusercontent.com/pypa/pip/b447f438df08303f4f07f2598f190e73876443ba/tests/unit/test_urls.py + +import os +import sys + +import pytest + +from poetry.core.packages import path_to_url +from poetry.core.packages import url_to_path as u2p +from poetry.core.packages.utils.utils import pathname2url + + +def url_to_path(*args, **kwargs): + return str(u2p(*args, **kwargs)) + + +@pytest.mark.skipif("sys.platform == 'win32'") +def test_path_to_url_unix(): + assert path_to_url("/tmp/file") == "file:///tmp/file" + path = os.path.join(os.getcwd(), "file") + assert path_to_url("file") == "file://" + pathname2url(path) + + +@pytest.mark.skipif("sys.platform != 'win32'") +def test_path_to_url_win(): + assert path_to_url("c:/tmp/file") == "file:///C:/tmp/file" + assert path_to_url("c:\\tmp\\file") == "file:///C:/tmp/file" + assert path_to_url(r"\\unc\as\path") == "file://unc/as/path" + path = os.path.join(os.getcwd(), "file") + assert path_to_url("file") == "file:" + pathname2url(path) + + +@pytest.mark.parametrize( + "url,win_expected,non_win_expected", + [ + ("file:tmp", "tmp", "tmp"), + ("file:c:/path/to/file", r"C:\path\to\file", "c:/path/to/file"), + ("file:/path/to/file", r"\path\to\file", "/path/to/file"), + ("file://localhost/tmp/file", r"\tmp\file", "/tmp/file"), + ("file://localhost/c:/tmp/file", r"C:\tmp\file", "/c:/tmp/file"), + ("file://somehost/tmp/file", r"\\somehost\tmp\file", None), + ("file:///tmp/file", r"\tmp\file", "/tmp/file"), + ("file:///c:/tmp/file", r"C:\tmp\file", "/c:/tmp/file"), + ], +) +def test_url_to_path(url, win_expected, non_win_expected): + if sys.platform == "win32": + expected_path = win_expected + else: + expected_path = non_win_expected + + if expected_path is None: + with pytest.raises(ValueError): + url_to_path(url) + else: + assert url_to_path(url) == expected_path + + +@pytest.mark.skipif("sys.platform != 'win32'") +def test_url_to_path_path_to_url_symmetry_win(): + path = r"C:\tmp\file" + assert url_to_path(path_to_url(path)) == path + + unc_path = r"\\unc\share\path" + assert url_to_path(path_to_url(unc_path)) == unc_path