diff --git a/src/pip/_internal/resolution/resolvelib/base.py b/src/pip/_internal/resolution/resolvelib/base.py index 3ccc48e1f..eacdf8ecc 100644 --- a/src/pip/_internal/resolution/resolvelib/base.py +++ b/src/pip/_internal/resolution/resolvelib/base.py @@ -44,6 +44,11 @@ class Candidate(object): # type: () -> _BaseVersion raise NotImplementedError("Override in subclass") + @property + def is_installed(self): + # type: () -> bool + raise NotImplementedError("Override in subclass") + def get_dependencies(self): # type: () -> Sequence[Requirement] raise NotImplementedError("Override in subclass") diff --git a/src/pip/_internal/resolution/resolvelib/candidates.py b/src/pip/_internal/resolution/resolvelib/candidates.py index 794d14026..ed6e71bab 100644 --- a/src/pip/_internal/resolution/resolvelib/candidates.py +++ b/src/pip/_internal/resolution/resolvelib/candidates.py @@ -101,15 +101,10 @@ 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): + # These are not installed + is_installed = False + def __init__( self, link, # type: Link @@ -279,6 +274,8 @@ class EditableCandidate(_InstallRequirementBackedCandidate): class AlreadyInstalledCandidate(Candidate): + is_installed = True + def __init__( self, dist, # type: Distribution @@ -400,6 +397,11 @@ class ExtrasCandidate(Candidate): # type: () -> _BaseVersion return self.base.version + @property + def is_installed(self): + # type: () -> _BaseVersion + return self.base.is_installed + def get_dependencies(self): # type: () -> Sequence[Requirement] factory = self.base._factory @@ -436,6 +438,8 @@ class ExtrasCandidate(Candidate): class RequiresPythonCandidate(Candidate): + is_installed = False + def __init__(self, py_version_info): # type: (Optional[Tuple[int, ...]]) -> None if py_version_info is not None: diff --git a/src/pip/_internal/resolution/resolvelib/factory.py b/src/pip/_internal/resolution/resolvelib/factory.py index 79343da32..2dc3a6ac9 100644 --- a/src/pip/_internal/resolution/resolvelib/factory.py +++ b/src/pip/_internal/resolution/resolvelib/factory.py @@ -114,6 +114,12 @@ class Factory(object): def iter_found_candidates(self, ireq, extras): # type: (InstallRequirement, Set[str]) -> Iterator[Candidate] name = canonicalize_name(ireq.req.name) + + # We use this to ensure that we only yield a single candidate for + # each version (the finder's preferred one for that version). The + # requirement needs to return only one candidate per version, so we + # implement that logic here so that requirements using this helper + # don't all have to do the same thing later. seen_versions = set() # Yield the installed version, if it matches, unless the user diff --git a/src/pip/_internal/resolution/resolvelib/provider.py b/src/pip/_internal/resolution/resolvelib/provider.py index b8e7413f9..f74fcae2f 100644 --- a/src/pip/_internal/resolution/resolvelib/provider.py +++ b/src/pip/_internal/resolution/resolvelib/provider.py @@ -3,8 +3,6 @@ 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, Set, Tuple, Union @@ -40,16 +38,16 @@ class PipProvider(AbstractProvider): constraints, # type: Dict[str, SpecifierSet] ignore_dependencies, # type: bool upgrade_strategy, # type: str - roots, # type: Set[str] + user_requested, # type: Set[str] ): # type: (...) -> None self._factory = factory self._constraints = constraints self._ignore_dependencies = ignore_dependencies self._upgrade_strategy = upgrade_strategy - self.roots = roots + self.user_requested = user_requested - def sort_matches(self, matches): + def _sort_matches(self, matches): # type: (Sequence[Candidate]) -> Sequence[Candidate] # The requirement is responsible for returning a sequence of potential @@ -76,28 +74,36 @@ class PipProvider(AbstractProvider): def _eligible_for_upgrade(name): # type: (str) -> bool + """Are upgrades allowed for this project? + + This checks the upgrade strategy, and whether the project was one + that the user specified in the command line, in order to decide + whether we should upgrade if there's a newer version available. + + (Note that we don't need access to the `--upgrade` flag, because + an upgrade strategy of "to-satisfy-only" means that `--upgrade` + was not specified). + """ if self._upgrade_strategy == "eager": return True elif self._upgrade_strategy == "only-if-needed": - return (name in self.roots) + return (name in self.user_requested) 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): + def sort_key(c): # type: (Candidate) -> Tuple[int, _BaseVersion] - return (keep_installed(c), c.version) + """Return a sort key for the matches. - return sorted(matches, key=key) + The highest priority should be given to installed candidates that + are not eligible for upgrade. We use the integer value in the first + part of the key to sort these before other candidates. + """ + if c.is_installed and not _eligible_for_upgrade(c.name): + return (1, c.version) + + return (0, c.version) + + return sorted(matches, key=sort_key) def get_install_requirement(self, c): # type: (Candidate) -> Optional[InstallRequirement] @@ -121,7 +127,7 @@ class PipProvider(AbstractProvider): # type: (Requirement) -> Sequence[Candidate] constraint = self._constraints.get(requirement.name, SpecifierSet()) matches = requirement.find_matches(constraint) - return self.sort_matches(matches) + return self._sort_matches(matches) 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 c32187c6c..5c7d00c2a 100644 --- a/src/pip/_internal/resolution/resolvelib/requirements.py +++ b/src/pip/_internal/resolution/resolvelib/requirements.py @@ -76,6 +76,10 @@ class SpecifierRequirement(Requirement): 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( diff --git a/src/pip/_internal/resolution/resolvelib/resolver.py b/src/pip/_internal/resolution/resolvelib/resolver.py index b363f3032..34c74ef95 100644 --- a/src/pip/_internal/resolution/resolvelib/resolver.py +++ b/src/pip/_internal/resolution/resolvelib/resolver.py @@ -69,7 +69,7 @@ class Resolver(BaseResolver): # type: (List[InstallRequirement], bool) -> RequirementSet constraints = {} # type: Dict[str, SpecifierSet] - roots = set() + user_requested = set() requirements = [] for req in root_reqs: if req.constraint: @@ -82,7 +82,7 @@ class Resolver(BaseResolver): constraints[name] = req.specifier else: if req.is_direct and req.name: - roots.add(canonicalize_name(req.name)) + user_requested.add(canonicalize_name(req.name)) requirements.append( self.factory.make_requirement_from_install_req(req) ) @@ -92,7 +92,7 @@ class Resolver(BaseResolver): constraints=constraints, ignore_dependencies=self.ignore_dependencies, upgrade_strategy=self.upgrade_strategy, - roots=roots, + user_requested=user_requested, ) reporter = BaseReporter() resolver = RLResolver(provider, reporter) diff --git a/tests/unit/resolution_resolvelib/conftest.py b/tests/unit/resolution_resolvelib/conftest.py index 3327d9cc0..45deca109 100644 --- a/tests/unit/resolution_resolvelib/conftest.py +++ b/tests/unit/resolution_resolvelib/conftest.py @@ -66,5 +66,5 @@ def provider(factory): constraints={}, ignore_dependencies=False, upgrade_strategy="to-satisfy-only", - roots=set(), + user_requested=set(), )