From 190c424b1e711e2ddf83a7eb04d140774dcd28e4 Mon Sep 17 00:00:00 2001 From: Tzu-ping Chung Date: Wed, 1 Apr 2020 18:14:36 +0800 Subject: [PATCH 1/7] Implement Python as a dependency If a dist contains Requires-Python metadata, it is converted into a Requirement for the resolver based on whether the Requires-Python is compatible or not. If it is compatible, an ExplicitRequirement is returned to hold the Python information (either sys.version_info, or the user-supplied --python-version). If it is incompatible, a special NoMatchRequirement is returned, which never matches to anything, generating a ResolutionImpossible to report the Python version incompatibility. The --ignore-requires-python flag is implemented as to not return a Requirement for Requires-Python at all. --- .../resolution/resolvelib/candidates.py | 61 +++++++++++++++++-- .../resolution/resolvelib/factory.py | 24 +++++++- .../resolution/resolvelib/requirements.py | 21 +++++++ .../resolution/resolvelib/resolver.py | 2 + tests/unit/resolution_resolvelib/conftest.py | 2 + 5 files changed, 103 insertions(+), 7 deletions(-) diff --git a/src/pip/_internal/resolution/resolvelib/candidates.py b/src/pip/_internal/resolution/resolvelib/candidates.py index d2acfe1e144..1f3cca9c5da 100644 --- a/src/pip/_internal/resolution/resolvelib/candidates.py +++ b/src/pip/_internal/resolution/resolvelib/candidates.py @@ -1,21 +1,26 @@ import logging +import sys +from pip._vendor.packaging.specifiers import InvalidSpecifier, SpecifierSet from pip._vendor.packaging.utils import canonicalize_name +from pip._vendor.packaging.version import Version from pip._internal.req.constructors import install_req_from_line from pip._internal.req.req_install import InstallRequirement +from pip._internal.utils.misc import normalize_version_info +from pip._internal.utils.packaging import get_requires_python from pip._internal.utils.typing import MYPY_CHECK_RUNNING from .base import Candidate, format_name if MYPY_CHECK_RUNNING: - from typing import Any, Optional, Sequence, Set - - from pip._internal.models.link import Link + from typing import Any, Optional, Sequence, Set, Tuple from pip._vendor.packaging.version import _BaseVersion from pip._vendor.pkg_resources import Distribution + from pip._internal.models.link import Link + from .base import Requirement from .factory import Factory @@ -95,12 +100,32 @@ def dist(self): self._version == self.dist.parsed_version) return self._dist + def _get_requires_python_specifier(self): + # type: () -> Optional[SpecifierSet] + requires_python = get_requires_python(self.dist) + if requires_python is None: + return None + try: + spec = SpecifierSet(requires_python) + except InvalidSpecifier as e: + logger.warning( + "Package %r has an invalid Requires-Python: %s", self.name, e, + ) + return None + return spec + def get_dependencies(self): # type: () -> Sequence[Requirement] - return [ + deps = [ self._factory.make_requirement_from_spec(str(r), self._ireq) for r in self.dist.requires() ] + python_dep = self._factory.make_requires_python_requirement( + self._get_requires_python_specifier(), + ) + if python_dep: + deps.append(python_dep) + return deps def get_install_requirement(self): # type: () -> Optional[InstallRequirement] @@ -179,3 +204,31 @@ def get_install_requirement(self): # depend on the base candidate, and we'll get the # install requirement from that. return None + + +class RequiresPythonCandidate(Candidate): + def __init__(self, py_version_info): + # type: (Optional[Tuple[int, ...]]) -> None + if py_version_info is not None: + version_info = normalize_version_info(py_version_info) + else: + version_info = sys.version_info[:3] + self._version = Version(".".join(str(c) for c in version_info)) + + @property + def name(self): + # type: () -> str + return "" # Avoid conflicting with the PyPI package "Python". + + @property + def version(self): + # type: () -> _BaseVersion + return self._version + + def get_dependencies(self): + # type: () -> Sequence[Requirement] + return [] + + def get_install_requirement(self): + # type: () -> Optional[InstallRequirement] + return None diff --git a/src/pip/_internal/resolution/resolvelib/factory.py b/src/pip/_internal/resolution/resolvelib/factory.py index b5a490e6ab0..c005675176c 100644 --- a/src/pip/_internal/resolution/resolvelib/factory.py +++ b/src/pip/_internal/resolution/resolvelib/factory.py @@ -1,10 +1,16 @@ from pip._internal.utils.typing import MYPY_CHECK_RUNNING -from .candidates import ExtrasCandidate, LinkCandidate -from .requirements import ExplicitRequirement, SpecifierRequirement +from .candidates import ExtrasCandidate, LinkCandidate, RequiresPythonCandidate +from .requirements import ( + ExplicitRequirement, + NoMatchRequirement, + SpecifierRequirement, +) if MYPY_CHECK_RUNNING: - from typing import Dict, Set + from typing import Dict, Optional, Set, Tuple + + from pip._vendor.packaging.specifiers import SpecifierSet from pip._internal.index.package_finder import PackageFinder from pip._internal.models.link import Link @@ -21,10 +27,14 @@ def __init__( finder, # type: PackageFinder preparer, # type: RequirementPreparer make_install_req, # type: InstallRequirementProvider + ignore_requires_python, # type: bool + py_version_info=None, # type: Optional[Tuple[int, ...]] ): # type: (...) -> None self.finder = finder self.preparer = preparer + self._python_candidate = RequiresPythonCandidate(py_version_info) + self._ignore_requires_python = ignore_requires_python self._make_install_req_from_spec = make_install_req self._candidate_cache = {} # type: Dict[Link, LinkCandidate] @@ -56,3 +66,11 @@ def make_requirement_from_spec(self, specifier, comes_from): # type: (str, InstallRequirement) -> Requirement ireq = self._make_install_req_from_spec(specifier, comes_from) return self.make_requirement_from_install_req(ireq) + + def make_requires_python_requirement(self, specifier): + # type: (Optional[SpecifierSet]) -> Optional[Requirement] + if self._ignore_requires_python or specifier is None: + return None + if self._python_candidate.version in specifier: + return ExplicitRequirement(self._python_candidate) + return NoMatchRequirement(self._python_candidate.name) diff --git a/src/pip/_internal/resolution/resolvelib/requirements.py b/src/pip/_internal/resolution/resolvelib/requirements.py index 1fe328597cb..cdfa93c2c67 100644 --- a/src/pip/_internal/resolution/resolvelib/requirements.py +++ b/src/pip/_internal/resolution/resolvelib/requirements.py @@ -33,6 +33,27 @@ def is_satisfied_by(self, candidate): return candidate == self.candidate +class NoMatchRequirement(Requirement): + """A requirement that never matches anything. + """ + def __init__(self, name): + # type: (str) -> None + self._name = canonicalize_name(name) + + @property + def name(self): + # type: () -> str + return self._name + + def find_matches(self): + # type: () -> Sequence[Candidate] + return [] + + def is_satisfied_by(self, candidate): + # type: (Candidate) -> bool + return False + + class SpecifierRequirement(Requirement): def __init__(self, ireq, factory): # type: (InstallRequirement, Factory) -> None diff --git a/src/pip/_internal/resolution/resolvelib/resolver.py b/src/pip/_internal/resolution/resolvelib/resolver.py index 7c8be9df58d..d2ec3d094a3 100644 --- a/src/pip/_internal/resolution/resolvelib/resolver.py +++ b/src/pip/_internal/resolution/resolvelib/resolver.py @@ -43,6 +43,8 @@ def __init__( finder=finder, preparer=preparer, make_install_req=make_install_req, + ignore_requires_python=ignore_requires_python, + py_version_info=py_version_info, ) self.ignore_dependencies = ignore_dependencies self._result = None # type: Optional[Result] diff --git a/tests/unit/resolution_resolvelib/conftest.py b/tests/unit/resolution_resolvelib/conftest.py index de268e829a1..0bea2bf8116 100644 --- a/tests/unit/resolution_resolvelib/conftest.py +++ b/tests/unit/resolution_resolvelib/conftest.py @@ -52,6 +52,8 @@ def factory(finder, preparer): finder=finder, preparer=preparer, make_install_req=install_req_from_line, + ignore_requires_python=False, + py_version_info=None, ) From 630339e577ebcb7b6116b96b28a00ec5b672a3dd Mon Sep 17 00:00:00 2001 From: Tzu-ping Chung Date: Thu, 2 Apr 2020 04:27:29 +0800 Subject: [PATCH 2/7] Add new resolver test for Requires-Python --- tests/functional/test_new_resolver.py | 55 +++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/tests/functional/test_new_resolver.py b/tests/functional/test_new_resolver.py index bb258b72e3f..e2b3da1b814 100644 --- a/tests/functional/test_new_resolver.py +++ b/tests/functional/test_new_resolver.py @@ -1,6 +1,9 @@ import json +import pytest + from tests.lib import create_basic_wheel_for_package +from tests.lib.wheel import make_wheel def assert_installed(script, **kwargs): @@ -137,3 +140,55 @@ def test_new_resolver_installs_extras(script): assert "WARNING: Invalid extras specified" in result.stderr, str(result) assert ": missing" in result.stderr, str(result) assert_installed(script, base="0.1.0", dep="0.1.0") + + +@pytest.mark.parametrize( + "requires_python, ignore_requires_python, dep_version", + [ + # Something impossible to satisfy. + ("<2", False, "0.1.0"), + ("<2", True, "0.2.0"), + + # Something guarentees to satisfy. + (">=2", False, "0.2.0"), + (">=2", True, "0.2.0"), + ], +) +def test_new_resolver_requires_python( + script, + requires_python, + ignore_requires_python, + dep_version, +): + create_basic_wheel_for_package( + script, + "base", + "0.1.0", + depends=["dep"], + ) + + # TODO: Use create_basic_wheel_for_package when it handles Requires-Python. + make_wheel( + "dep", + "0.1.0", + ).save_to_dir(script.scratch_path) + make_wheel( + "dep", + "0.2.0", + metadata_updates={"Requires-Python": requires_python}, + ).save_to_dir(script.scratch_path) + + args = [ + "install", + "--unstable-feature=resolver", + "--no-cache-dir", + "--no-index", + "--find-links", script.scratch_path, + ] + if ignore_requires_python: + args.append("--ignore-requires-python") + args.append("base") + + script.pip(*args) + + assert_installed(script, base="0.1.0", dep=dep_version) From 22f7c883ad8afb7e1841d878ddfe9531a9fb592f Mon Sep 17 00:00:00 2001 From: Tzu-ping Chung Date: Thu, 2 Apr 2020 18:38:41 +0800 Subject: [PATCH 3/7] Requires-Python support in test helper --- tests/lib/__init__.py | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/tests/lib/__init__.py b/tests/lib/__init__.py index 67c66b9ce22..682f66c711e 100644 --- a/tests/lib/__init__.py +++ b/tests/lib/__init__.py @@ -979,7 +979,13 @@ def add_file(path, text): def create_basic_wheel_for_package( - script, name, version, depends=None, extras=None, extra_files=None + script, + name, + version, + depends=None, + extras=None, + requires_python=None, + extra_files=None, ): if depends is None: depends = [] @@ -1007,14 +1013,18 @@ def hello(): for package in packages ] + metadata_updates = { + "Provides-Extra": list(extras), + "Requires-Dist": requires_dist, + } + if requires_python is not None: + metadata_updates["Requires-Python"] = requires_python + wheel_builder = make_wheel( name=name, version=version, wheel_metadata_updates={"Tag": ["py2-none-any", "py3-none-any"]}, - metadata_updates={ - "Provides-Extra": list(extras), - "Requires-Dist": requires_dist, - }, + metadata_updates=metadata_updates, extra_metadata_files={"top_level.txt": name}, extra_files=extra_files, From 34c24f6e795f7eca44bca7d8d66383c49e3f7b0b Mon Sep 17 00:00:00 2001 From: Tzu-ping Chung Date: Thu, 2 Apr 2020 18:40:48 +0800 Subject: [PATCH 4/7] Switch to create_basic_wheel_for_package --- tests/functional/test_new_resolver.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/tests/functional/test_new_resolver.py b/tests/functional/test_new_resolver.py index e2b3da1b814..6d4200f872d 100644 --- a/tests/functional/test_new_resolver.py +++ b/tests/functional/test_new_resolver.py @@ -3,7 +3,6 @@ import pytest from tests.lib import create_basic_wheel_for_package -from tests.lib.wheel import make_wheel def assert_installed(script, **kwargs): @@ -166,17 +165,17 @@ def test_new_resolver_requires_python( "0.1.0", depends=["dep"], ) - - # TODO: Use create_basic_wheel_for_package when it handles Requires-Python. - make_wheel( + create_basic_wheel_for_package( + script, "dep", "0.1.0", - ).save_to_dir(script.scratch_path) - make_wheel( + ) + create_basic_wheel_for_package( + script, "dep", "0.2.0", - metadata_updates={"Requires-Python": requires_python}, - ).save_to_dir(script.scratch_path) + requires_python=requires_python, + ) args = [ "install", From 65ffd338f4eb39ed46daf4c4eeb0a7c1fa19951c Mon Sep 17 00:00:00 2001 From: Tzu-ping Chung Date: Thu, 2 Apr 2020 18:50:23 +0800 Subject: [PATCH 5/7] Explain how Requires-Python matching works --- src/pip/_internal/resolution/resolvelib/factory.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/pip/_internal/resolution/resolvelib/factory.py b/src/pip/_internal/resolution/resolvelib/factory.py index c005675176c..4c4bf2bee75 100644 --- a/src/pip/_internal/resolution/resolvelib/factory.py +++ b/src/pip/_internal/resolution/resolvelib/factory.py @@ -71,6 +71,11 @@ def make_requires_python_requirement(self, specifier): # type: (Optional[SpecifierSet]) -> Optional[Requirement] if self._ignore_requires_python or specifier is None: return None + # The logic here is different from SpecifierRequirement, for which we + # "find" candidates matching the specifier. But for Requires-Python, + # there is always exactly one candidate (the one specified with + # py_version_info). Here we decide whether to return that based on + # whether Requires-Python matches that one candidate or not. if self._python_candidate.version in specifier: return ExplicitRequirement(self._python_candidate) return NoMatchRequirement(self._python_candidate.name) From 9ca2240530070d3719a42abfbe8c5f4677633a04 Mon Sep 17 00:00:00 2001 From: Tzu-ping Chung Date: Thu, 2 Apr 2020 21:42:26 +0800 Subject: [PATCH 6/7] More clarification on the name --- src/pip/_internal/resolution/resolvelib/candidates.py | 3 ++- src/pip/_internal/resolution/resolvelib/requirements.py | 5 ++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/pip/_internal/resolution/resolvelib/candidates.py b/src/pip/_internal/resolution/resolvelib/candidates.py index 1f3cca9c5da..31453dbd4d4 100644 --- a/src/pip/_internal/resolution/resolvelib/candidates.py +++ b/src/pip/_internal/resolution/resolvelib/candidates.py @@ -218,7 +218,8 @@ def __init__(self, py_version_info): @property def name(self): # type: () -> str - return "" # Avoid conflicting with the PyPI package "Python". + # Avoid conflicting with the PyPI package "Python". + return "" @property def version(self): diff --git a/src/pip/_internal/resolution/resolvelib/requirements.py b/src/pip/_internal/resolution/resolvelib/requirements.py index cdfa93c2c67..b8711534db6 100644 --- a/src/pip/_internal/resolution/resolvelib/requirements.py +++ b/src/pip/_internal/resolution/resolvelib/requirements.py @@ -35,10 +35,13 @@ def is_satisfied_by(self, candidate): class NoMatchRequirement(Requirement): """A requirement that never matches anything. + + Note: Similar to ExplicitRequirement, the caller should handle name + canonicalisation; this class does not perform it. """ def __init__(self, name): # type: (str) -> None - self._name = canonicalize_name(name) + self._name = name @property def name(self): From 557f7670ea8b870692b26610a38dfbceb1af056a Mon Sep 17 00:00:00 2001 From: Tzu-ping Chung Date: Thu, 2 Apr 2020 21:44:54 +0800 Subject: [PATCH 7/7] Typo in comment --- tests/functional/test_new_resolver.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/test_new_resolver.py b/tests/functional/test_new_resolver.py index 6d4200f872d..9bf34651b79 100644 --- a/tests/functional/test_new_resolver.py +++ b/tests/functional/test_new_resolver.py @@ -148,7 +148,7 @@ def test_new_resolver_installs_extras(script): ("<2", False, "0.1.0"), ("<2", True, "0.2.0"), - # Something guarentees to satisfy. + # Something guaranteed to satisfy. (">=2", False, "0.2.0"), (">=2", True, "0.2.0"), ],