Simplify, fix, and add unit tests for PipProvider.get_preference

docs/html/topics/more-dependency-resolution.md

@@ -156,10 +156,10 @@ grey parts of the graph).
 Pip's current implementation of the provider implements `get_preference` as
 follows:
 
-* Prefer if any of the known requirements is "direct", e.g. points to an
-    explicit URL.
 * If equal, prefer if any requirement is "pinned", i.e. contains
     operator ``===`` or ``==``.
+* If equal, prefer if any requirement is part of the current causes
+    for backtracking.
 * If equal, calculate an approximate "depth" and resolve requirements
     closer to the user-specified requirements first.
 * Order user-specified requirements by the order they are specified.

news/12982.bugfix.rst

@@ -0,0 +1 @@
+Improve consistency in package selection during dependency resolution.

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

@@ -118,18 +118,18 @@ def get_preference(
         The lower the return value is, the more preferred this group of
         arguments is.
 
-        Currently pip considers the following in order:
+        Currently, pip considers the following in order:
 
-        * Prefer if any of the known requirements is "direct", e.g. points to an
-          explicit URL.
-        * If equal, prefer if any requirement is "pinned", i.e. contains
+        * If equal, prefer if any requirement is "pinned", i.e., contains
           operator ``===`` or ``==``.
+        * If equal, prefer if any requirement is part of the current causes
+          for backtracking.
         * If equal, calculate an approximate "depth" and resolve requirements
           closer to the user-specified requirements first. If the depth cannot
-          by determined (eg: due to no matching parents), it is considered
+          be determined (e.g., due to no matching parents), it is considered
           infinite.
         * Order user-specified requirements by the order they are specified.
-        * If equal, prefers "non-free" requirements, i.e. contains at least one
+        * If equal, prefer "non-free" requirements, i.e., contains at least one
           operator, such as ``>=`` or ``<``.
         * If equal, order alphabetically for consistency (helps debuggability).
         """
@@ -144,37 +144,34 @@ def get_preference(
 
         if has_information:
             lookups = (r.get_candidate_lookup() for r, _ in information[identifier])
-            candidate, ireqs = zip(*lookups)
+            _icandidates, ireqs = zip(*lookups)
         else:
-            candidate, ireqs = None, ()
+            _icandidates, ireqs = (), ()
 
         operators = [
             specifier.operator
             for specifier_set in (ireq.specifier for ireq in ireqs if ireq)
             for specifier in specifier_set
         ]
 
-        direct = candidate is not None
-        pinned = any(op[:2] == "==" for op in operators)
+        pinned = any(op in ("==", "===") for op in operators)
         unfree = bool(operators)
 
-        try:
-            requested_order: Union[int, float] = self._user_requested[identifier]
-        except KeyError:
+        if identifier in self._user_requested:
+            requested_order: float = self._user_requested[identifier]
+            inferred_depth = 1.0
+        elif not has_information:
             requested_order = math.inf
-            if has_information:
-                parent_depths = (
-                    self._known_depths[parent.name] if parent is not None else 0.0
-                    for _, parent in information[identifier]
-                )
-                inferred_depth = min(d for d in parent_depths) + 1.0
-            else:
-                inferred_depth = math.inf
+            inferred_depth = math.inf
         else:
-            inferred_depth = 1.0
-        self._known_depths[identifier] = inferred_depth
+            requested_order = math.inf
+            parent_depths = (
+                0.0 if parent is None else self._known_depths[parent.name]
+                for _, parent in information[identifier]
+            )
+            inferred_depth = min(parent_depths) + 1.0
 
-        requested_order = self._user_requested.get(identifier, math.inf)
+        self._known_depths[identifier] = inferred_depth
 
         # Requires-Python has only one candidate and the check is basically
         # free, so we always do it first to avoid needless work if it fails.
@@ -187,7 +184,6 @@ def get_preference(
 
         return (
             not requires_python,
-            not direct,
             not pinned,
             not backtrack_cause,
             inferred_depth,

tests/unit/resolution_resolvelib/test_provider.py

@@ -1,29 +1,37 @@
-from typing import TYPE_CHECKING, List, Optional
+import math
+from typing import TYPE_CHECKING, Dict, Iterable, Optional, Sequence
+
+import pytest
 
 from pip._vendor.resolvelib.resolvers import RequirementInformation
 
 from pip._internal.models.candidate import InstallationCandidate
 from pip._internal.models.link import Link
 from pip._internal.req.constructors import install_req_from_req_string
+from pip._internal.resolution.resolvelib.candidates import REQUIRES_PYTHON_IDENTIFIER
 from pip._internal.resolution.resolvelib.factory import Factory
 from pip._internal.resolution.resolvelib.provider import PipProvider
 from pip._internal.resolution.resolvelib.requirements import SpecifierRequirement
 
 if TYPE_CHECKING:
-    from pip._internal.resolution.resolvelib.provider import PreferenceInformation
+    from pip._vendor.resolvelib.providers import Preference
+
+    from pip._internal.resolution.resolvelib.base import Candidate, Requirement
 
+    PreferenceInformation = RequirementInformation[Requirement, Candidate]
 
-def build_requirement_information(
-    name: str, parent: Optional[InstallationCandidate]
-) -> List["PreferenceInformation"]:
+
+def build_req_info(
+    name: str, parent: Optional[InstallationCandidate] = None
+) -> "PreferenceInformation":
     install_requirement = install_req_from_req_string(name)
     # RequirementInformation is typed as a tuple, but it is a namedtupled.
     # https://github.com/sarugaku/resolvelib/blob/7bc025aa2a4e979597c438ad7b17d2e8a08a364e/src/resolvelib/resolvers.pyi#L20-L22
     requirement_information: PreferenceInformation = RequirementInformation(
         requirement=SpecifierRequirement(install_requirement),  # type: ignore[call-arg]
         parent=parent,
     )
-    return [requirement_information]
+    return requirement_information
 
 
 def test_provider_known_depths(factory: Factory) -> None:
@@ -38,14 +46,14 @@ def test_provider_known_depths(factory: Factory) -> None:
         user_requested={root_requirement_name: 0},
     )
 
-    root_requirement_information = build_requirement_information(
+    root_requirement_information = build_req_info(
         name=root_requirement_name, parent=None
     )
     provider.get_preference(
         identifier=root_requirement_name,
         resolutions={},
         candidates={},
-        information={root_requirement_name: root_requirement_information},
+        information={root_requirement_name: [root_requirement_information]},
         backtrack_causes=[],
     )
     assert provider._known_depths == {root_requirement_name: 1.0}
@@ -59,20 +67,125 @@ def test_provider_known_depths(factory: Factory) -> None:
     )
     transitive_requirement_name = "my-transitive-package"
 
-    transitive_package_information = build_requirement_information(
+    transitive_package_information = build_req_info(
         name=transitive_requirement_name, parent=root_package_candidate
     )
     provider.get_preference(
         identifier=transitive_requirement_name,
         resolutions={},
         candidates={},
         information={
-            root_requirement_name: root_requirement_information,
-            transitive_requirement_name: transitive_package_information,
+            root_requirement_name: [root_requirement_information],
+            transitive_requirement_name: [transitive_package_information],
         },
         backtrack_causes=[],
     )
     assert provider._known_depths == {
         transitive_requirement_name: 2.0,
         root_requirement_name: 1.0,
     }
+
+
+@pytest.mark.parametrize(
+    "identifier, information, backtrack_causes, user_requested, known_depths, expected",
+    [
+        # Test case for REQUIRES_PYTHON_IDENTIFIER
+        (
+            REQUIRES_PYTHON_IDENTIFIER,
+            {REQUIRES_PYTHON_IDENTIFIER: [build_req_info("python")]},
+            [],
+            {REQUIRES_PYTHON_IDENTIFIER: 1},
+            {},
+            (False, True, True, 1.0, 1, True, REQUIRES_PYTHON_IDENTIFIER),
+        ),
+        # Pinned package with "=="
+        (
+            "pinned-package",
+            {"pinned-package": [build_req_info("pinned-package==1.0")]},
+            [],
+            {"pinned-package": 1},
+            {},
+            (True, False, True, 1.0, 1, False, "pinned-package"),
+        ),
+        # Package that caused backtracking
+        (
+            "backtrack-package",
+            {"backtrack-package": [build_req_info("backtrack-package")]},
+            [build_req_info("backtrack-package")],
+            {"backtrack-package": 1},
+            {},
+            (True, True, False, 1.0, 1, True, "backtrack-package"),
+        ),
+        # Depth inference for child package
+        (
+            "child-package",
+            {
+                "child-package": [
+                    build_req_info(
+                        "child-package",
+                        parent=InstallationCandidate(
+                            "parent-package", "1.0", Link("https://parent-package.com")
+                        ),
+                    )
+                ],
+                "parent-package": [build_req_info("parent-package")],
+            },
+            [],
+            {"parent-package": 1},
+            {"parent-package": 1.0},
+            (True, True, True, 2.0, math.inf, True, "child-package"),
+        ),
+        # Root package requested by user
+        (
+            "root-package",
+            {"root-package": [build_req_info("root-package")]},
+            [],
+            {"root-package": 1},
+            {},
+            (True, True, True, 1.0, 1, True, "root-package"),
+        ),
+        # Unfree package (with specifier operator)
+        (
+            "unfree-package",
+            {"unfree-package": [build_req_info("unfree-package<1")]},
+            [],
+            {"unfree-package": 1},
+            {},
+            (True, True, True, 1.0, 1, False, "unfree-package"),
+        ),
+        # Free package (no operator)
+        (
+            "free-package",
+            {"free-package": [build_req_info("free-package")]},
+            [],
+            {"free-package": 1},
+            {},
+            (True, True, True, 1.0, 1, True, "free-package"),
+        ),
+    ],
+)
+def test_get_preference(
+    identifier: str,
+    information: Dict[str, Iterable["PreferenceInformation"]],
+    backtrack_causes: Sequence["PreferenceInformation"],
+    user_requested: Dict[str, int],
+    known_depths: Dict[str, float],
+    expected: "Preference",
+    factory: Factory,
+) -> None:
+    provider = PipProvider(
+        factory=factory,
+        constraints={},
+        ignore_dependencies=False,
+        upgrade_strategy="to-satisfy-only",
+        user_requested=user_requested,
+    )
+
+    if known_depths:
+        provider._known_depths.update(known_depths)
+
+    preference = provider.get_preference(
+        identifier, {}, {}, information, backtrack_causes
+    )
+
+    assert preference == expected, f"Expected {expected}, got {preference}"