Make BacktrackingResolver ignore extras when dropping existing constraints

.pre-commit-config.yaml

@@ -34,6 +34,7 @@ repos:
           - pip==20.3.4
           - build==1.0.0
           - pyproject_hooks==1.0.0
+          - pytest>=7.2.0
   - repo: https://github.com/PyCQA/bandit
     rev: 1.7.5
     hooks:

piptools/resolver.py

@@ -648,7 +648,8 @@ def _do_resolve(
 
             # Collect all incompatible install requirement names
             cause_ireq_names = {
-                key_from_req(cause.requirement) for cause in cause_exc.causes
+                strip_extras(key_from_req(cause.requirement))
+                for cause in cause_exc.causes
             }
 
             # Looks like resolution is impossible, try to fix

piptools/utils.py

@@ -23,6 +23,7 @@
 from click.utils import LazyFile
 from pip._internal.req import InstallRequirement
 from pip._internal.req.constructors import install_req_from_line, parse_req_from_line
+from pip._internal.resolution.resolvelib.base import Requirement as PipRequirement
 from pip._internal.utils.misc import redact_auth_from_url
 from pip._internal.vcs import is_url
 from pip._vendor.packaging.markers import Marker
@@ -72,8 +73,20 @@ def key_from_ireq(ireq: InstallRequirement) -> str:
         return key_from_req(ireq.req)
 
 
-def key_from_req(req: InstallRequirement | Requirement) -> str:
-    """Get an all-lowercase version of the requirement's name."""
+def key_from_req(req: InstallRequirement | Requirement | PipRequirement) -> str:
+    """
+    Get an all-lowercase version of the requirement's name.
+
+    **Note:** If the argument is an instance of
+    ``pip._internal.resolution.resolvelib.base.Requirement`` (like
+    ``pip._internal.resolution.resolvelib.requirements.SpecifierRequirement``),
+    then the name might include an extras specification.
+    Apply :py:func:`strip_extras` to the result of this function if you need
+    the package name only.
+
+    :param req: the requirement the key is computed for
+    :return: the canonical name of the requirement
+    """
     return str(canonicalize_name(req.name))
 
 

tests/test_resolver.py

@@ -1,11 +1,23 @@
 from __future__ import annotations
 
+from itertools import chain
+from typing import Callable, Sequence
+from unittest.mock import Mock, NonCallableMock
+
 import pytest
 from pip._internal.exceptions import DistributionNotFound
+from pip._internal.req.req_install import InstallRequirement
+from pip._internal.resolution.resolvelib.requirements import SpecifierRequirement
+from pip._internal.resolution.resolvelib.resolver import Resolver
 from pip._internal.utils.urls import path_to_url
+from pip._vendor.resolvelib.resolvers import ResolutionImpossible
 
 from piptools.exceptions import NoCandidateFound
-from piptools.resolver import RequirementSummary, combine_install_requirements
+from piptools.resolver import (
+    BacktrackingResolver,
+    RequirementSummary,
+    combine_install_requirements,
+)
 
 
 @pytest.mark.parametrize(
@@ -575,3 +587,73 @@ def resolve(self, *args, **kwargs):
             resolver=FakePipResolver(),
             compatible_existing_constraints={},
         )
+
+
+@pytest.mark.parametrize(
+    ("constraints", "conflicting_existing", "compatible_existing"),
+    (
+        (
+            [
+                "poetry==1.6.1",
+            ],
+            ["cachecontrol[filecache]==0.12.14"],
+            ["doesnotexist==1.0.0"],
+        ),
+        (
+            [
+                "poetry==1.6.1",
+            ],
+            ["keyring==22.1.0", "pkginfo==1.7.2"],
+            ["platformdirs==3.0.0", "doesnotexist==1.0.0"],
+        ),
+    ),
+)
+def test_backtracking_resolver_drops_existing_conflicting_constraints(
+    backtracking_resolver: Callable[..., BacktrackingResolver],
+    from_line: Callable[..., InstallRequirement],
+    constraints: Sequence[str],
+    conflicting_existing: Sequence[str],
+    compatible_existing: Sequence[str],
+) -> None:
+    def wrap_resolution_impossible(*args, **kwargs):
+        """
+        Raise a ``DistributionNotFound`` exception that has a ``ResolutionImpossible``
+        exception as its cause.
+        """
+        causes = [
+            NonCallableMock(
+                requirement=SpecifierRequirement(existing_constraints[ireq.name])
+            )
+            for ireq in conflicting_ireqs
+        ]
+        raise DistributionNotFound("resolution impossible") from ResolutionImpossible(
+            causes
+        )
+
+    constraint_ireqs = [from_line(req, constraint=False) for req in constraints]
+    conflicting_ireqs = [
+        from_line(req, constraint=True) for req in conflicting_existing
+    ]
+    compatible_ireqs = [from_line(req, constraint=True) for req in compatible_existing]
+    existing_constraints = {
+        ireq.name: ireq for ireq in chain(compatible_ireqs, conflicting_ireqs)
+    }
+
+    bt_resolver = backtracking_resolver(
+        constraint_ireqs, existing_constraints=existing_constraints
+    )
+    resolver = NonCallableMock(
+        spec=Resolver,
+        resolve=Mock(side_effect=wrap_resolution_impossible),
+    )
+
+    # resolver has been rigged to raise a DistributionNotFound exception with
+    # a cause that refers to the entries of conflicting_ireqs.
+    # We expect _do_resolve() to handle this exception by dropping
+    # these entries from existing_constraints and returning False.
+    # It should _not_ drop any compatible constraint or re-raise
+    # the DistributionNotFound exception.
+    result = bt_resolver._do_resolve(resolver, existing_constraints)
+
+    assert result is False
+    assert set(existing_constraints.keys()) == {ireq.name for ireq in compatible_ireqs}

tests/test_utils.py

@@ -7,10 +7,13 @@
 import sys
 from pathlib import Path
 from textwrap import dedent
+from typing import Callable
 
 import pip
 import pytest
 from click import BadOptionUsage, Context, FileError
+from pip._internal.req import InstallRequirement
+from pip._internal.resolution.resolvelib.requirements import SpecifierRequirement
 from pip._vendor.packaging.version import Version
 
 from piptools.scripts.compile import cli as compile_cli
@@ -29,6 +32,7 @@
     is_pinned_requirement,
     is_url_requirement,
     key_from_ireq,
+    key_from_req,
     lookup_table,
     lookup_table_from_tuples,
     override_defaults_from_config_file,
@@ -285,6 +289,46 @@ def test_key_from_ireq_normalization(from_line):
     assert len(keys) == 1
 
 
+@pytest.mark.parametrize(
+    ("line", "expected"),
+    (
+        ("build", "build"),
+        ("cachecontrol[filecache]", "cachecontrol"),
+        ("some-package[a-b,c_d]", "some-package"),
+        ("other_package[a.b]", "other-package"),
+    ),
+)
+def test_key_from_req_on_install_requirement(
+    from_line: Callable[[str], InstallRequirement],
+    line: str,
+    expected: str,
+) -> None:
+    ireq = from_line(line)
+    result = key_from_req(ireq)
+
+    assert result == expected
+
+
+@pytest.mark.parametrize(
+    ("line", "expected"),
+    (
+        ("build", "build"),
+        ("cachecontrol[filecache]", "cachecontrol[filecache]"),
+        ("some-package[a-b,c_d]", "some-package[a-b,c-d]"),
+        ("other_package[a.b]", "other-package[a-b]"),
+    ),
+)
+def test_key_from_req_on_specifier_requirement(
+    from_line: Callable[[str], InstallRequirement],
+    line: str,
+    expected: str,
+) -> None:
+    req = SpecifierRequirement(from_line(line))
+    result = key_from_req(req)
+
+    assert result == expected
+
+
 @pytest.mark.parametrize(
     ("line", "expected"),
     (