diff --git a/src/pip/_internal/resolution/resolvelib/candidates.py b/src/pip/_internal/resolution/resolvelib/candidates.py index c4772c33f..794d14026 100644 --- a/src/pip/_internal/resolution/resolvelib/candidates.py +++ b/src/pip/_internal/resolution/resolvelib/candidates.py @@ -101,6 +101,14 @@ def make_install_req_from_dist(dist, parent): return ireq +def is_already_installed(cand): + # type: (Candidate) -> bool + # For an ExtrasCandidate, we check the base + if isinstance(cand, ExtrasCandidate): + cand = cand.base + return isinstance(cand, AlreadyInstalledCandidate) + + class _InstallRequirementBackedCandidate(Candidate): def __init__( self, diff --git a/src/pip/_internal/resolution/resolvelib/factory.py b/src/pip/_internal/resolution/resolvelib/factory.py index b3cbb22f1..79343da32 100644 --- a/src/pip/_internal/resolution/resolvelib/factory.py +++ b/src/pip/_internal/resolution/resolvelib/factory.py @@ -42,8 +42,6 @@ if MYPY_CHECK_RUNNING: class Factory(object): - _allowed_strategies = {"eager", "only-if-needed", "to-satisfy-only"} - def __init__( self, finder, # type: PackageFinder @@ -52,11 +50,9 @@ class Factory(object): force_reinstall, # type: bool ignore_installed, # type: bool ignore_requires_python, # type: bool - upgrade_strategy, # type: str py_version_info=None, # type: Optional[Tuple[int, ...]] ): # type: (...) -> None - assert upgrade_strategy in self._allowed_strategies self.finder = finder self.preparer = preparer @@ -64,9 +60,6 @@ class Factory(object): self._make_install_req_from_spec = make_install_req self._force_reinstall = force_reinstall self._ignore_requires_python = ignore_requires_python - self._upgrade_strategy = upgrade_strategy - - self.root_reqs = set() # type: Set[str] self._link_candidate_cache = {} # type: Cache[LinkCandidate] self._editable_candidate_cache = {} # type: Cache[EditableCandidate] @@ -118,23 +111,27 @@ class Factory(object): return ExtrasCandidate(base, extras) return base - def _eligible_for_upgrade(self, dist_name): - # type: (str) -> bool - if self._upgrade_strategy == "eager": - return True - elif self._upgrade_strategy == "only-if-needed": - return (dist_name in self.root_reqs) - return False - def iter_found_candidates(self, ireq, extras): # type: (InstallRequirement, Set[str]) -> Iterator[Candidate] name = canonicalize_name(ireq.req.name) - if not self._force_reinstall: - installed_dist = self._installed_dists.get(name) - can_upgrade = self._eligible_for_upgrade(name) - else: - installed_dist = None - can_upgrade = False + seen_versions = set() + + # Yield the installed version, if it matches, unless the user + # specified `--force-reinstall`, when we want the version from + # the index instead. + 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 + ): + seen_versions.add(installed_version) + yield self._make_candidate_from_dist( + dist=installed_dist, + extras=extras, + parent=ireq, + ) found = self.finder.find_best_candidate( project_name=ireq.req.name, @@ -142,40 +139,18 @@ class Factory(object): hashes=ireq.hashes(trust_internet=False), ) for ican in found.iter_applicable(): - if (installed_dist is not None and - installed_dist.parsed_version == ican.version): - if can_upgrade: - yield self._make_candidate_from_dist( - dist=installed_dist, - extras=extras, - parent=ireq, - ) - continue - yield self._make_candidate_from_link( - link=ican.link, - extras=extras, - parent=ireq, - name=name, - version=ican.version, - ) - - # Return installed distribution if it matches the specifier. This is - # done last so the resolver will prefer it over downloading links. - if can_upgrade or installed_dist is None: - return - installed_version = installed_dist.parsed_version - if ireq.req.specifier.contains(installed_version, prereleases=True): - yield self._make_candidate_from_dist( - dist=installed_dist, - extras=extras, - parent=ireq, - ) + if ican.version not in seen_versions: + seen_versions.add(ican.version) + yield self._make_candidate_from_link( + link=ican.link, + extras=extras, + parent=ireq, + name=name, + version=ican.version, + ) def make_requirement_from_install_req(self, ireq): # type: (InstallRequirement) -> Requirement - if ireq.is_direct and ireq.name: - self.root_reqs.add(canonicalize_name(ireq.name)) - if ireq.link: # TODO: Get name and version from ireq, if possible? # Specifically, this might be needed in "name @ URL" diff --git a/src/pip/_internal/resolution/resolvelib/provider.py b/src/pip/_internal/resolution/resolvelib/provider.py index 226dc3687..2dbcc8735 100644 --- a/src/pip/_internal/resolution/resolvelib/provider.py +++ b/src/pip/_internal/resolution/resolvelib/provider.py @@ -3,14 +3,35 @@ from pip._vendor.resolvelib.providers import AbstractProvider from pip._internal.utils.typing import MYPY_CHECK_RUNNING +from .candidates import is_already_installed + if MYPY_CHECK_RUNNING: - from typing import Any, Dict, Optional, Sequence, Tuple, Union + from typing import Any, Dict, Optional, Sequence, Set, Tuple, Union from pip._internal.req.req_install import InstallRequirement + from pip._vendor.packaging.version import _BaseVersion from .base import Requirement, Candidate from .factory import Factory +# Notes on the relationship between the provider, the factory, and the +# candidate and requirement classes. +# +# The provider is a direct implementation of the resolvelib class. Its role +# is to deliver the API that resolvelib expects. +# +# Rather than work with completely abstract "requirement" and "candidate" +# concepts as resolvelib does, pip has concrete classes implementing these two +# ideas. The API of Requirement and Candidate objects are defined in the base +# classes, but essentially map fairly directly to the equivalent provider +# methods. In particular, `find_matches` and `is_satisfied_by` are +# requirement methods, and `get_dependencies` is a candidate method. +# +# The factory is the interface to pip's internal mechanisms. It is stateless, +# and is created by the resolver and held as a property of the provider. It is +# responsible for creating Requirement and Candidate objects, and provides +# services to those objects (access to pip's finder and preparer). + class PipProvider(AbstractProvider): def __init__( @@ -18,11 +39,66 @@ class PipProvider(AbstractProvider): factory, # type: Factory constraints, # type: Dict[str, SpecifierSet] ignore_dependencies, # type: bool + upgrade_strategy, # type: str + roots, # type: Set[str] ): # type: (...) -> None self._factory = factory self._constraints = constraints self._ignore_dependencies = ignore_dependencies + self._upgrade_strategy = upgrade_strategy + self.roots = roots + + def sort_matches(self, matches): + # type: (Sequence[Candidate]) -> Sequence[Candidate] + + # The requirement is responsible for returning a sequence of potential + # candidates, one per version. The provider handles the logic of + # deciding the order in which these candidates should be passed to + # the resolver. + + # The `matches` argument is a sequence of candidates, one per version, + # which are potential options to be installed. The requirement will + # have already sorted out whether to give us an already-installed + # candidate or a version from PyPI (i.e., it will deal with options + # like --force-reinstall and --ignore-installed). + + # We now work out the correct order. + # + # 1. If no other considerations apply, later versions take priority. + # 2. An already installed distribution is preferred over any other, + # unless the user has requested an upgrade. + # Upgrades are allowed when: + # * The --upgrade flag is set, and + # - 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 + if self._upgrade_strategy == "eager": + return True + elif self._upgrade_strategy == "only-if-needed": + print(name, self.roots) + return (name in self.roots) + return False + + def keep_installed(c): + # type: (Candidate) -> int + """Give priority to an installed version?""" + if not is_already_installed(c): + return 0 + + if _eligible_for_upgrade(c.name): + return 0 + + return 1 + + def key(c): + # type: (Candidate) -> Tuple[int, _BaseVersion] + return (keep_installed(c), c.version) + + return sorted(matches, key=key) def get_install_requirement(self, c): # type: (Candidate) -> Optional[InstallRequirement] @@ -45,7 +121,8 @@ class PipProvider(AbstractProvider): def find_matches(self, requirement): # type: (Requirement) -> Sequence[Candidate] constraint = self._constraints.get(requirement.name, SpecifierSet()) - return requirement.find_matches(constraint) + matches = requirement.find_matches(constraint) + return self.sort_matches(matches) def is_satisfied_by(self, requirement, candidate): # type: (Requirement, Candidate) -> bool diff --git a/src/pip/_internal/resolution/resolvelib/resolver.py b/src/pip/_internal/resolution/resolvelib/resolver.py index 5c94d3dc0..b363f3032 100644 --- a/src/pip/_internal/resolution/resolvelib/resolver.py +++ b/src/pip/_internal/resolution/resolvelib/resolver.py @@ -32,6 +32,8 @@ logger = logging.getLogger(__name__) class Resolver(BaseResolver): + _allowed_strategies = {"eager", "only-if-needed", "to-satisfy-only"} + def __init__( self, preparer, # type: RequirementPreparer @@ -47,6 +49,9 @@ class Resolver(BaseResolver): py_version_info=None, # type: Optional[Tuple[int, ...]] ): super(Resolver, self).__init__() + + assert upgrade_strategy in self._allowed_strategies + self.factory = Factory( finder=finder, preparer=preparer, @@ -54,23 +59,17 @@ class Resolver(BaseResolver): force_reinstall=force_reinstall, ignore_installed=ignore_installed, ignore_requires_python=ignore_requires_python, - upgrade_strategy=upgrade_strategy, py_version_info=py_version_info, ) self.ignore_dependencies = ignore_dependencies + self.upgrade_strategy = upgrade_strategy self._result = None # type: Optional[Result] def resolve(self, root_reqs, check_supported_wheels): # type: (List[InstallRequirement], bool) -> RequirementSet - # The factory should not have retained state from any previous usage. - # In theory this could only happen if self was reused to do a second - # resolve, which isn't something we do at the moment. We assert here - # in order to catch the issue if that ever changes. - # The persistent state that we care about is `root_reqs`. - assert len(self.factory.root_reqs) == 0, "Factory is being re-used" - constraints = {} # type: Dict[str, SpecifierSet] + roots = set() requirements = [] for req in root_reqs: if req.constraint: @@ -82,6 +81,8 @@ class Resolver(BaseResolver): else: constraints[name] = req.specifier else: + if req.is_direct and req.name: + roots.add(canonicalize_name(req.name)) requirements.append( self.factory.make_requirement_from_install_req(req) ) @@ -90,6 +91,8 @@ class Resolver(BaseResolver): factory=self.factory, constraints=constraints, ignore_dependencies=self.ignore_dependencies, + upgrade_strategy=self.upgrade_strategy, + roots=roots, ) reporter = BaseReporter() resolver = RLResolver(provider, reporter) diff --git a/tests/unit/resolution_resolvelib/conftest.py b/tests/unit/resolution_resolvelib/conftest.py index 41c479bf3..3327d9cc0 100644 --- a/tests/unit/resolution_resolvelib/conftest.py +++ b/tests/unit/resolution_resolvelib/conftest.py @@ -55,7 +55,6 @@ def factory(finder, preparer): force_reinstall=False, ignore_installed=False, ignore_requires_python=False, - upgrade_strategy="to-satisfy-only", py_version_info=None, ) @@ -66,4 +65,6 @@ def provider(factory): factory=factory, constraints={}, ignore_dependencies=False, + upgrade_strategy="to-satisfy-only", + roots=set(), )