Skip to content

Commit

Permalink
Improve support for PEP-440 direct references
Browse files Browse the repository at this point in the history
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.

In addition to file resource, directory dependencies now support
the same scheme.

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
  • Loading branch information
abn committed Apr 12, 2020
1 parent 9c4cdc9 commit 1687fb9
Show file tree
Hide file tree
Showing 10 changed files with 254 additions and 32 deletions.
22 changes: 20 additions & 2 deletions poetry/core/packages/__init__.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import os
import re

from typing import Optional

from poetry.core.semver import Version
from poetry.core.utils._compat import Path
from poetry.core.utils.patterns import wheel_file_re
from poetry.core.version.requirements import Requirement

Expand All @@ -19,10 +22,18 @@
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


def dependency_from_pep_508(name):
def dependency_from_pep_508(
name, relative_to=None
): # type: (str, Optional[Path]) -> Dependency
"""
Resolve a PEP-508 requirement string to a `Dependency` instance. If a `relative_to`
path is specified, this is used as the base directory if the identified dependency is
of file or directory type.
"""
from poetry.core.vcs.git import ParsedUrl

# Removing comments
Expand Down Expand Up @@ -84,7 +95,14 @@ 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
path = url_to_path(req.url)
if path.is_file():
dep = FileDependency(name, path, base=relative_to)
else:
dep = DirectoryDependency(name, path, base=relative_to)
else:
dep = Dependency(name, "*")
else:
Expand Down
13 changes: 13 additions & 0 deletions poetry/core/packages/directory_dependency.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,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 poetry.core.utils.toml_file import TomlFile

Expand Down Expand Up @@ -79,3 +80,15 @@ def supports_poetry(self):

def is_directory(self):
return True

@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
13 changes: 13 additions & 0 deletions poetry/core/packages/file_dependency.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
66 changes: 48 additions & 18 deletions poetry/core/packages/utils/utils.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
import os
import posixpath
import re
import sys

from typing import Union

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
Expand All @@ -11,24 +18,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")
Expand All @@ -52,14 +48,48 @@
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 unless otherwise
specified and have quoted path parts.
"""
path = Path(path)

if absolute:
path = path.resolve()

if path.is_absolute():
return path.as_uri()

return "file://{}".format(pathname2url(path.as_posix()))


def url_to_path(url): # type: (str) -> Path
"""
Convert a path to a file: URL. The path will be made absolute and have
quoted path parts.
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
"""
path = os.path.normpath(os.path.abspath(path))
url = urlparse.urljoin("file:", urllib2.pathname2url(path))
return url
if not url.startswith("file:"):
raise ValueError("{} is not a valid file URI".format(url))

_, netloc, path, _, _ = urlsplit(url)

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 = "//" + 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):
Expand Down
8 changes: 4 additions & 4 deletions poetry/core/utils/_compat.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down
7 changes: 5 additions & 2 deletions poetry/core/version/requirements.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 6 additions & 6 deletions tests/masonry/builders/test_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -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() == {
Expand All @@ -45,15 +45,15 @@ 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()


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()
Expand Down Expand Up @@ -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())
Expand All @@ -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())
Expand All @@ -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())
Expand Down
40 changes: 40 additions & 0 deletions tests/packages/test_directory_dependency.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import pytest

from poetry.core.packages import dependency_from_pep_508
from poetry.core.packages.directory_dependency import DirectoryDependency
from poetry.core.utils._compat import Path

Expand All @@ -10,3 +11,42 @@
def test_directory_dependency_must_exist():
with pytest.raises(ValueError):
DirectoryDependency("demo", DIST_PATH / "invalid")


def _test_directory_dependency_pep_508(name, path, pep_508_input, pep_508_output=None):
dep = dependency_from_pep_508(pep_508_input, relative_to=Path(__file__).parent)

assert dep.is_directory()
assert dep.name == name
assert dep.path == path
assert dep.to_pep_508() == pep_508_output or pep_508_input


def test_directory_dependency_pep_508_local_absolute():
path = (
Path(__file__).parent.parent
/ "fixtures"
/ "project_with_multi_constraints_dependency"
)
requirement = "{} @ file://{}".format("demo", path)

_test_directory_dependency_pep_508("demo", path, requirement)


def test_directory_dependency_pep_508_localhost():
path = (
Path(__file__).parent.parent
/ "fixtures"
/ "project_with_multi_constraints_dependency"
)
requirement = "{} @ file://localhost{}".format("demo", path)
requirement_expected = "{} @ file://{}".format("demo", path)

_test_directory_dependency_pep_508("demo", path, requirement, requirement_expected)


def test_directory_dependency_pep_508_local_relative():
path = Path("..") / "fixtures" / "project_with_multi_constraints_dependency"
requirement = "{} @ file://{}".format("demo", path)

_test_directory_dependency_pep_508("demo", path, requirement)
39 changes: 39 additions & 0 deletions tests/packages/test_file_dependency.py
Original file line number Diff line number Diff line change
@@ -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


Expand All @@ -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
):
mocker.patch.object(Path, "exists").return_value = True
mocker.patch.object(Path, "is_file").return_value = True

dep = dependency_from_pep_508(pep_508_input, relative_to=Path(__file__).parent)

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)
Loading

0 comments on commit 1687fb9

Please sign in to comment.