diff --git a/docs/html/topics/more-dependency-resolution.md b/docs/html/topics/more-dependency-resolution.md index b955e2ec114..8c05c2db68c 100644 --- a/docs/html/topics/more-dependency-resolution.md +++ b/docs/html/topics/more-dependency-resolution.md @@ -156,10 +156,12 @@ 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 restricted by upper bounds, i.e. + contains operators ``<``, ``<=``, ``~=``, or the specifier ``==N.*``. +* 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. diff --git a/news/13017.bugfix.rst b/news/13017.bugfix.rst new file mode 100644 index 00000000000..6b82fd948ac --- /dev/null +++ b/news/13017.bugfix.rst @@ -0,0 +1 @@ +Better resolution choices for large dependency trees which include upper bounds. diff --git a/src/pip/_internal/resolution/resolvelib/provider.py b/src/pip/_internal/resolution/resolvelib/provider.py index fb0dd85f112..aa09fbf6635 100644 --- a/src/pip/_internal/resolution/resolvelib/provider.py +++ b/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,40 @@ 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 + operators_versions = [ + (specifier.operator, specifier.version) for specifier_set in (ireq.specifier for ireq in ireqs if ireq) for specifier in specifier_set ] + upper_bound = any( + op in ("<", "<=", "~=") or (op == "==" and "*" in ver) + for op, ver in operators_versions + ) + pinned = any( + op == "===" or (op == "==" and "*" not in ver) + for op, ver in operators_versions + ) + unfree = bool(operators_versions) - direct = candidate is not None - pinned = any(op[:2] == "==" 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,8 +190,8 @@ def get_preference( return ( not requires_python, - not direct, not pinned, + not upper_bound, not backtrack_cause, inferred_depth, requested_order, diff --git a/tests/unit/resolution_resolvelib/test_provider.py b/tests/unit/resolution_resolvelib/test_provider.py index 5f30e2bc1dd..83917e4a38a 100644 --- a/tests/unit/resolution_resolvelib/test_provider.py +++ b/tests/unit/resolution_resolvelib/test_provider.py @@ -1,21 +1,29 @@ -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 @@ -23,7 +31,7 @@ def build_requirement_information( 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,7 +67,7 @@ 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( @@ -67,8 +75,8 @@ def test_provider_known_depths(factory: Factory) -> None: 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=[], ) @@ -76,3 +84,126 @@ def test_provider_known_depths(factory: Factory) -> None: 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, 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, True, 1.0, 1, False, "pinned-package"), + ), + # Upper bound package with "<" + ( + "upper-bound-package", + {"upper-bound-package": [build_req_info("upper-bound-package<1.0")]}, + [], + {"upper-bound-package": 1}, + {}, + (True, True, False, True, 1.0, 1, False, "upper-bound-package"), + ), + # Test "==N.*" which is not pinned but does create implicit upper bound + ( + "equal-star-package", + {"equal-star-package": [build_req_info("equal-star-package==1.*")]}, + [], + {"equal-star-package": 1}, + {}, + (True, True, False, True, 1.0, 1, False, "equal-star-package"), + ), + # Package that caused backtracking + ( + "backtrack-package", + {"backtrack-package": [build_req_info("backtrack-package")]}, + [build_req_info("backtrack-package")], + {"backtrack-package": 1}, + {}, + (True, 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, 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, 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, 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, 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}"