From 6c6b6a7765e18138678bd0e38858df45fe9a9271 Mon Sep 17 00:00:00 2001 From: Tzu-ping Chung Date: Tue, 19 May 2020 17:04:15 +0800 Subject: [PATCH] Implement new Provider.find_matches() --- .../_internal/resolution/resolvelib/base.py | 21 +++-- .../resolution/resolvelib/candidates.py | 16 +++- .../resolution/resolvelib/factory.py | 89 +++++++++++++++---- .../resolution/resolvelib/provider.py | 28 ++++-- .../resolution/resolvelib/requirements.py | 53 ++++------- src/pip/_internal/utils/hashes.py | 12 +++ .../resolution_resolvelib/test_requirement.py | 10 ++- 7 files changed, 152 insertions(+), 77 deletions(-) diff --git a/src/pip/_internal/resolution/resolvelib/base.py b/src/pip/_internal/resolution/resolvelib/base.py index 17513d336..57013b7b2 100644 --- a/src/pip/_internal/resolution/resolvelib/base.py +++ b/src/pip/_internal/resolution/resolvelib/base.py @@ -3,15 +3,20 @@ from pip._vendor.packaging.utils import canonicalize_name from pip._internal.utils.typing import MYPY_CHECK_RUNNING if MYPY_CHECK_RUNNING: - from typing import Iterable, Optional, Sequence, Set + from typing import FrozenSet, Iterable, Optional, Tuple + + from pip._vendor.packaging.version import _BaseVersion from pip._internal.req.req_install import InstallRequirement - from pip._vendor.packaging.specifiers import SpecifierSet - from pip._vendor.packaging.version import _BaseVersion + + CandidateLookup = Tuple[ + Optional["Candidate"], + Optional[InstallRequirement], + ] def format_name(project, extras): - # type: (str, Set[str]) -> str + # type: (str, FrozenSet[str]) -> str if not extras: return project canonical_extras = sorted(canonicalize_name(e) for e in extras) @@ -24,14 +29,14 @@ class Requirement(object): # type: () -> str raise NotImplementedError("Subclass should override") - def find_matches(self, constraint): - # type: (SpecifierSet) -> Sequence[Candidate] - raise NotImplementedError("Subclass should override") - def is_satisfied_by(self, candidate): # type: (Candidate) -> bool return False + def get_candidate_lookup(self): + # type: () -> CandidateLookup + raise NotImplementedError("Subclass should override") + class Candidate(object): @property diff --git a/src/pip/_internal/resolution/resolvelib/candidates.py b/src/pip/_internal/resolution/resolvelib/candidates.py index ed5173fe0..1f729198f 100644 --- a/src/pip/_internal/resolution/resolvelib/candidates.py +++ b/src/pip/_internal/resolution/resolvelib/candidates.py @@ -17,7 +17,7 @@ from pip._internal.utils.typing import MYPY_CHECK_RUNNING from .base import Candidate, format_name if MYPY_CHECK_RUNNING: - from typing import Any, Iterable, Optional, Set, Tuple, Union + from typing import Any, FrozenSet, Iterable, Optional, Tuple, Union from pip._vendor.packaging.version import _BaseVersion from pip._vendor.pkg_resources import Distribution @@ -132,6 +132,10 @@ class _InstallRequirementBackedCandidate(Candidate): link=str(self.link), ) + def __hash__(self): + # type: () -> int + return hash((self.__class__, self.link)) + def __eq__(self, other): # type: (Any) -> bool if isinstance(other, self.__class__): @@ -313,6 +317,10 @@ class AlreadyInstalledCandidate(Candidate): distribution=self.dist, ) + def __hash__(self): + # type: () -> int + return hash((self.__class__, self.name, self.version)) + def __eq__(self, other): # type: (Any) -> bool if isinstance(other, self.__class__): @@ -371,7 +379,7 @@ class ExtrasCandidate(Candidate): def __init__( self, base, # type: BaseCandidate - extras, # type: Set[str] + extras, # type: FrozenSet[str] ): # type: (...) -> None self.base = base @@ -385,6 +393,10 @@ class ExtrasCandidate(Candidate): extras=self.extras, ) + def __hash__(self): + # type: () -> int + return hash((self.base, self.extras)) + def __eq__(self, other): # type: (Any) -> bool if isinstance(other, self.__class__): diff --git a/src/pip/_internal/resolution/resolvelib/factory.py b/src/pip/_internal/resolution/resolvelib/factory.py index 51b7a6f79..bc044e168 100644 --- a/src/pip/_internal/resolution/resolvelib/factory.py +++ b/src/pip/_internal/resolution/resolvelib/factory.py @@ -9,6 +9,7 @@ from pip._internal.exceptions import ( UnsupportedPythonVersion, ) from pip._internal.utils.compatibility_tags import get_supported +from pip._internal.utils.hashes import Hashes from pip._internal.utils.misc import ( dist_in_site_packages, dist_in_usersite, @@ -31,7 +32,17 @@ from .requirements import ( ) if MYPY_CHECK_RUNNING: - from typing import Dict, Iterable, Iterator, Optional, Set, Tuple, TypeVar + from typing import ( + FrozenSet, + Dict, + Iterable, + List, + Optional, + Sequence, + Set, + Tuple, + TypeVar, + ) from pip._vendor.packaging.specifiers import SpecifierSet from pip._vendor.packaging.version import _BaseVersion @@ -71,7 +82,7 @@ class Factory(object): ): # type: (...) -> None - self.finder = finder + self._finder = finder self.preparer = preparer self._wheel_cache = wheel_cache self._python_candidate = RequiresPythonCandidate(py_version_info) @@ -94,7 +105,7 @@ class Factory(object): def _make_candidate_from_dist( self, dist, # type: Distribution - extras, # type: Set[str] + extras, # type: FrozenSet[str] parent, # type: InstallRequirement ): # type: (...) -> Candidate @@ -130,9 +141,28 @@ class Factory(object): return ExtrasCandidate(base, extras) return base - def iter_found_candidates(self, ireq, extras): - # type: (InstallRequirement, Set[str]) -> Iterator[Candidate] - name = canonicalize_name(ireq.req.name) + def _iter_found_candidates( + self, + ireqs, # type: Sequence[InstallRequirement] + specifier, # type: SpecifierSet + ): + # type: (...) -> Iterable[Candidate] + if not ireqs: + return () + + # The InstallRequirement implementation requires us to give it a + # "parent", which doesn't really fit with graph-based resolution. + # Here we just choose the first requirement to represent all of them. + # Hopefully the Project model can correct this mismatch in the future. + parent = ireqs[0] + name = canonicalize_name(parent.req.name) + + hashes = Hashes() + extras = frozenset() # type: FrozenSet[str] + for ireq in ireqs: + specifier &= ireq.req.specifier + hashes |= ireq.hashes(trust_internet=False) + extras |= ireq.req.extras # We use this to ensure that we only yield a single candidate for # each version (the finder's preferred one for that version). The @@ -148,21 +178,18 @@ class Factory(object): if not self._force_reinstall and name in self._installed_dists: installed_dist = self._installed_dists[name] installed_version = installed_dist.parsed_version - if ireq.req.specifier.contains( - installed_version, - prereleases=True - ): + if specifier.contains(installed_version, prereleases=True): candidate = self._make_candidate_from_dist( dist=installed_dist, extras=extras, - parent=ireq, + parent=parent, ) candidates[installed_version] = candidate - found = self.finder.find_best_candidate( - project_name=ireq.req.name, - specifier=ireq.req.specifier, - hashes=ireq.hashes(trust_internet=False), + found = self._finder.find_best_candidate( + project_name=name, + specifier=specifier, + hashes=hashes, ) for ican in found.iter_applicable(): if ican.version == installed_version: @@ -170,7 +197,7 @@ class Factory(object): candidate = self._make_candidate_from_link( link=ican.link, extras=extras, - parent=ireq, + parent=parent, name=name, version=ican.version, ) @@ -178,10 +205,38 @@ class Factory(object): return six.itervalues(candidates) + def find_candidates(self, requirements, constraint): + # type: (Sequence[Requirement], SpecifierSet) -> Iterable[Candidate] + explicit_candidates = set() # type: Set[Candidate] + ireqs = [] # type: List[InstallRequirement] + for req in requirements: + cand, ireq = req.get_candidate_lookup() + if cand is not None: + explicit_candidates.add(cand) + if ireq is not None: + ireqs.append(ireq) + + # If none of the requirements want an explicit candidate, we can ask + # the finder for candidates. + if not explicit_candidates: + return self._iter_found_candidates(ireqs, constraint) + + if constraint: + name = explicit_candidates.pop().name + raise InstallationError( + "Could not satisfy constraints for {!r}: installation from " + "path or url cannot be constrained to a version".format(name) + ) + + return ( + c for c in explicit_candidates + if all(req.is_satisfied_by(c) for req in requirements) + ) + def make_requirement_from_install_req(self, ireq): # type: (InstallRequirement) -> Requirement if not ireq.link: - return SpecifierRequirement(ireq, factory=self) + return SpecifierRequirement(ireq) cand = self._make_candidate_from_link( ireq.link, extras=set(ireq.extras), diff --git a/src/pip/_internal/resolution/resolvelib/provider.py b/src/pip/_internal/resolution/resolvelib/provider.py index e9a41f04f..98b9f9420 100644 --- a/src/pip/_internal/resolution/resolvelib/provider.py +++ b/src/pip/_internal/resolution/resolvelib/provider.py @@ -4,7 +4,16 @@ from pip._vendor.resolvelib.providers import AbstractProvider from pip._internal.utils.typing import MYPY_CHECK_RUNNING if MYPY_CHECK_RUNNING: - from typing import Any, Dict, Optional, Sequence, Set, Tuple, Union + from typing import ( + Any, + Dict, + Iterable, + Optional, + Sequence, + Set, + Tuple, + Union, + ) from .base import Requirement, Candidate from .factory import Factory @@ -45,7 +54,7 @@ class PipProvider(AbstractProvider): self.user_requested = user_requested def _sort_matches(self, matches): - # type: (Sequence[Candidate]) -> Sequence[Candidate] + # type: (Iterable[Candidate]) -> Sequence[Candidate] # The requirement is responsible for returning a sequence of potential # candidates, one per version. The provider handles the logic of @@ -68,7 +77,6 @@ class PipProvider(AbstractProvider): # - The project was specified on the command line, or # - The project is a dependency and the "eager" upgrade strategy # was requested. - def _eligible_for_upgrade(name): # type: (str) -> bool """Are upgrades allowed for this project? @@ -121,11 +129,15 @@ class PipProvider(AbstractProvider): # Use the "usual" value for now return len(candidates) - def find_matches(self, requirement): - # type: (Requirement) -> Sequence[Candidate] - constraint = self._constraints.get(requirement.name, SpecifierSet()) - matches = requirement.find_matches(constraint) - return self._sort_matches(matches) + def find_matches(self, requirements): + # type: (Sequence[Requirement]) -> Iterable[Candidate] + if not requirements: + return [] + constraint = self._constraints.get( + requirements[0].name, SpecifierSet(), + ) + candidates = self._factory.find_candidates(requirements, constraint) + return reversed(self._sort_matches(candidates)) def is_satisfied_by(self, requirement, candidate): # type: (Requirement, Candidate) -> bool diff --git a/src/pip/_internal/resolution/resolvelib/requirements.py b/src/pip/_internal/resolution/resolvelib/requirements.py index f21e37a4a..a10df9494 100644 --- a/src/pip/_internal/resolution/resolvelib/requirements.py +++ b/src/pip/_internal/resolution/resolvelib/requirements.py @@ -1,19 +1,15 @@ from pip._vendor.packaging.utils import canonicalize_name -from pip._internal.exceptions import InstallationError from pip._internal.utils.typing import MYPY_CHECK_RUNNING from .base import Requirement, format_name if MYPY_CHECK_RUNNING: - from typing import Sequence - from pip._vendor.packaging.specifiers import SpecifierSet from pip._internal.req.req_install import InstallRequirement - from .base import Candidate - from .factory import Factory + from .base import Candidate, CandidateLookup class ExplicitRequirement(Requirement): @@ -34,15 +30,9 @@ class ExplicitRequirement(Requirement): # No need to canonicalise - the candidate did this return self.candidate.name - def find_matches(self, constraint): - # type: (SpecifierSet) -> Sequence[Candidate] - if len(constraint) > 0: - raise InstallationError( - "Could not satisfy constraints for '{}': " - "installation from path or url cannot be " - "constrained to a version".format(self.name) - ) - return [self.candidate] + def get_candidate_lookup(self): + # type: () -> CandidateLookup + return self.candidate, None def is_satisfied_by(self, candidate): # type: (Candidate) -> bool @@ -50,12 +40,11 @@ class ExplicitRequirement(Requirement): class SpecifierRequirement(Requirement): - def __init__(self, ireq, factory): - # type: (InstallRequirement, Factory) -> None + def __init__(self, ireq): + # type: (InstallRequirement) -> None assert ireq.link is None, "This is a link, not a specifier" self._ireq = ireq - self._factory = factory - self.extras = set(ireq.extras) + self._extras = frozenset(ireq.extras) def __str__(self): # type: () -> str @@ -72,21 +61,11 @@ class SpecifierRequirement(Requirement): def name(self): # type: () -> str canonical_name = canonicalize_name(self._ireq.req.name) - return format_name(canonical_name, self.extras) + return format_name(canonical_name, self._extras) - def find_matches(self, constraint): - # type: (SpecifierSet) -> Sequence[Candidate] - - # We should only return one candidate per version, but - # iter_found_candidates does that for us, so we don't need - # to do anything special here. - return [ - c - for c in self._factory.iter_found_candidates( - self._ireq, self.extras - ) - if constraint.contains(c.version, prereleases=True) - ] + def get_candidate_lookup(self): + # type: () -> CandidateLookup + return None, self._ireq def is_satisfied_by(self, candidate): # type: (Candidate) -> bool @@ -120,13 +99,11 @@ class RequiresPythonRequirement(Requirement): # type: () -> str return self._candidate.name - def find_matches(self, constraint): - # type: (SpecifierSet) -> Sequence[Candidate] - assert len(constraint) == 0, \ - "RequiresPythonRequirement cannot have constraints" + def get_candidate_lookup(self): + # type: () -> CandidateLookup if self.specifier.contains(self._candidate.version, prereleases=True): - return [self._candidate] - return [] + return self._candidate, None + return None, None def is_satisfied_by(self, candidate): # type: (Candidate) -> bool diff --git a/src/pip/_internal/utils/hashes.py b/src/pip/_internal/utils/hashes.py index 396cf82e7..d1b062fed 100644 --- a/src/pip/_internal/utils/hashes.py +++ b/src/pip/_internal/utils/hashes.py @@ -46,6 +46,18 @@ class Hashes(object): """ self._allowed = {} if hashes is None else hashes + def __or__(self, other): + # type: (Hashes) -> Hashes + if not isinstance(other, Hashes): + return NotImplemented + new = self._allowed.copy() + for alg, values in iteritems(other._allowed): + try: + new[alg] += values + except KeyError: + new[alg] = values + return Hashes(new) + @property def digest_count(self): # type: () -> int diff --git a/tests/unit/resolution_resolvelib/test_requirement.py b/tests/unit/resolution_resolvelib/test_requirement.py index 0b7dec02d..07cd0c0f0 100644 --- a/tests/unit/resolution_resolvelib/test_requirement.py +++ b/tests/unit/resolution_resolvelib/test_requirement.py @@ -57,16 +57,18 @@ def test_new_resolver_requirement_has_name(test_cases, factory): def test_new_resolver_correct_number_of_matches(test_cases, factory): """Requirements should return the correct number of candidates""" - for spec, name, matches in test_cases: + for spec, name, match_count in test_cases: req = factory.make_requirement_from_spec(spec, comes_from=None) - assert len(req.find_matches(SpecifierSet())) == matches + matches = factory.find_candidates([req], SpecifierSet()) + assert len(list(matches)) == match_count def test_new_resolver_candidates_match_requirement(test_cases, factory): - """Candidates returned from find_matches should satisfy the requirement""" + """Candidates returned from find_candidates should satisfy the requirement + """ for spec, name, matches in test_cases: req = factory.make_requirement_from_spec(spec, comes_from=None) - for c in req.find_matches(SpecifierSet()): + for c in factory.find_candidates([req], SpecifierSet()): assert isinstance(c, Candidate) assert req.is_satisfied_by(c)