Merge pull request #8126 from pfmoore/nr_upgrade_strategy

Implement upgrade strategies for the new resolver

src/pip/_internal/resolution/resolvelib/factory.py

@@ -42,6 +42,8 @@ if MYPY_CHECK_RUNNING:
 
 
 class Factory(object):
+    _allowed_strategies = {"eager", "only-if-needed", "to-satisfy-only"}
+
     def __init__(
         self,
         finder,  # type: PackageFinder
@@ -50,15 +52,21 @@ 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
         self._python_candidate = RequiresPythonCandidate(py_version_info)
         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]
@@ -110,13 +118,23 @@ 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
 
         found = self.finder.find_best_candidate(
             project_name=ireq.req.name,
@@ -126,6 +144,12 @@ class Factory(object):
         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,
@@ -138,6 +162,7 @@ class Factory(object):
         # Return installed distribution if it matches the specifier. This is
         # done last so the resolver will prefer it over downloading links.
         if (installed_dist is not None and
+                not can_upgrade and
                 installed_dist.parsed_version in ireq.req.specifier):
             yield self._make_candidate_from_dist(
                 dist=installed_dist,
@@ -147,6 +172,9 @@ class Factory(object):
 
     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"

src/pip/_internal/resolution/resolvelib/resolver.py

@@ -53,6 +53,7 @@ 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
@@ -61,6 +62,13 @@ class Resolver(BaseResolver):
     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"
+
         # FIXME: Implement constraints.
         if any(r.constraint for r in root_reqs):
             raise InstallationError("Constraints are not yet supported.")

tests/functional/test_new_resolver.py

@@ -531,6 +531,91 @@ def test_new_resolver_handles_prerelease(
     assert_installed(script, pkg=expected_version)
 
 
+def test_new_resolver_upgrade_needs_option(script):
+    # Install pkg 1.0.0
+    create_basic_wheel_for_package(script, "pkg", "1.0.0")
+    script.pip(
+        "install", "--unstable-feature=resolver",
+        "--no-cache-dir", "--no-index",
+        "--find-links", script.scratch_path,
+        "pkg",
+    )
+
+    # Now release a new version
+    create_basic_wheel_for_package(script, "pkg", "2.0.0")
+
+    # This should not upgrade because we don't specify --upgrade
+    result = script.pip(
+        "install", "--unstable-feature=resolver",
+        "--no-cache-dir", "--no-index",
+        "--find-links", script.scratch_path,
+        "pkg",
+    )
+
+    assert "Requirement already satisfied" in result.stdout, str(result)
+    assert_installed(script, pkg="1.0.0")
+
+    # This should upgrade
+    result = script.pip(
+        "install", "--unstable-feature=resolver",
+        "--no-cache-dir", "--no-index",
+        "--find-links", script.scratch_path,
+        "--upgrade",
+        "PKG",  # Deliberately uppercase to check canonicalization
+    )
+
+    assert "Uninstalling pkg-1.0.0" in result.stdout, str(result)
+    assert "Successfully uninstalled pkg-1.0.0" in result.stdout, str(result)
+    assert script.site_packages / "pkg" in result.files_updated, (
+        "pkg not upgraded"
+    )
+    assert_installed(script, pkg="2.0.0")
+
+
+def test_new_resolver_upgrade_strategy(script):
+    create_basic_wheel_for_package(script, "base", "1.0.0", depends=["dep"])
+    create_basic_wheel_for_package(script, "dep", "1.0.0")
+    script.pip(
+        "install", "--unstable-feature=resolver",
+        "--no-cache-dir", "--no-index",
+        "--find-links", script.scratch_path,
+        "base",
+    )
+
+    assert_installed(script, base="1.0.0")
+    assert_installed(script, dep="1.0.0")
+
+    # Now release new versions
+    create_basic_wheel_for_package(script, "base", "2.0.0", depends=["dep"])
+    create_basic_wheel_for_package(script, "dep", "2.0.0")
+
+    script.pip(
+        "install", "--unstable-feature=resolver",
+        "--no-cache-dir", "--no-index",
+        "--find-links", script.scratch_path,
+        "--upgrade",
+        "base",
+    )
+
+    # With upgrade strategy "only-if-needed" (the default), dep should not
+    # be upgraded.
+    assert_installed(script, base="2.0.0")
+    assert_installed(script, dep="1.0.0")
+
+    create_basic_wheel_for_package(script, "base", "3.0.0", depends=["dep"])
+    script.pip(
+        "install", "--unstable-feature=resolver",
+        "--no-cache-dir", "--no-index",
+        "--find-links", script.scratch_path,
+        "--upgrade", "--upgrade-strategy=eager",
+        "base",
+    )
+
+    # With upgrade strategy "eager", dep should be upgraded.
+    assert_installed(script, base="3.0.0")
+    assert_installed(script, dep="2.0.0")
+
+
 class TestExtraMerge(object):
     """
     Test installing a package that depends the same package with different

tests/unit/resolution_resolvelib/conftest.py

@@ -55,6 +55,7 @@ def factory(finder, preparer):
         force_reinstall=False,
         ignore_installed=False,
         ignore_requires_python=False,
+        upgrade_strategy="to-satisfy-only",
         py_version_info=None,
     )
 

tests/unit/resolution_resolvelib/test_resolver.py

@@ -24,7 +24,7 @@ def resolver(preparer, finder):
         ignore_installed="not-used",
         ignore_requires_python="not-used",
         force_reinstall="not-used",
-        upgrade_strategy="not-used",
+        upgrade_strategy="to-satisfy-only",
     )
     return resolver