From 7f6f95e247745e59396f6ef9711059d05e7704a1 Mon Sep 17 00:00:00 2001 From: Richard Si Date: Sun, 12 Jan 2025 12:52:16 -0500 Subject: [PATCH 1/2] resolvelib: emit Requires-Python dependency first This makes the resolver always inspect Requires-Python first when checking a candidate's consistency, ensuring that no other candidates are prepared if the Requires-Python check fails. This regression was masked due to a broken test which checked for the (nonpresence of the) wrong package name. --- The resolvelib provider was also updated to return dependencies lazily. While ideally we wouldn't prepare candidates unnecessarily, pip has grown numerous metadata checks (for reporting bad metadata, skipping candidates with unsupported legacy metadata, etc.) so it's infeasible to stop preparing candidates upon creation (without a serious architectural redesign). However, we can create the candidates one-by-one as they're processed instead of all dependencies at once. This is necessary so the resolver can process Requires-Python first without processing other dependencies. Co-authored-by: Tzu-ping Chung --- news/13270.bugfix.rst | 3 +++ src/pip/_internal/resolution/resolvelib/candidates.py | 2 +- src/pip/_internal/resolution/resolvelib/provider.py | 5 +++-- tests/functional/test_new_resolver_errors.py | 8 +++++--- 4 files changed, 12 insertions(+), 6 deletions(-) create mode 100644 news/13270.bugfix.rst diff --git a/news/13270.bugfix.rst b/news/13270.bugfix.rst new file mode 100644 index 00000000000..9828b666daf --- /dev/null +++ b/news/13270.bugfix.rst @@ -0,0 +1,3 @@ +Fix a regression that causes dependencies to be checked *before* ``Requires-Python`` +project metadata is checked, leading to wasted cycles when the Python version is +unsupported. diff --git a/src/pip/_internal/resolution/resolvelib/candidates.py b/src/pip/_internal/resolution/resolvelib/candidates.py index 1d21ede72cc..3ddffe0375f 100644 --- a/src/pip/_internal/resolution/resolvelib/candidates.py +++ b/src/pip/_internal/resolution/resolvelib/candidates.py @@ -249,10 +249,10 @@ def _prepare(self) -> BaseDistribution: return dist def iter_dependencies(self, with_requires: bool) -> Iterable[Optional[Requirement]]: + yield self._factory.make_requires_python_requirement(self.dist.requires_python) requires = self.dist.iter_dependencies() if with_requires else () for r in requires: yield from self._factory.make_requirements_from_spec(str(r), self._ireq) - yield self._factory.make_requires_python_requirement(self.dist.requires_python) def get_install_requirement(self) -> Optional[InstallRequirement]: return self._ireq diff --git a/src/pip/_internal/resolution/resolvelib/provider.py b/src/pip/_internal/resolution/resolvelib/provider.py index 40326001eaa..4e270887366 100644 --- a/src/pip/_internal/resolution/resolvelib/provider.py +++ b/src/pip/_internal/resolution/resolvelib/provider.py @@ -218,9 +218,10 @@ def _eligible_for_upgrade(identifier: str) -> bool: def is_satisfied_by(self, requirement: Requirement, candidate: Candidate) -> bool: return requirement.is_satisfied_by(candidate) - def get_dependencies(self, candidate: Candidate) -> Sequence[Requirement]: + def get_dependencies(self, candidate: Candidate) -> Iterable[Requirement]: with_requires = not self._ignore_dependencies - return [r for r in candidate.iter_dependencies(with_requires) if r is not None] + # iter_dependencies() can perform nontrivial work so delay until needed. + return (r for r in candidate.iter_dependencies(with_requires) if r is not None) @staticmethod def is_backtrack_cause( diff --git a/tests/functional/test_new_resolver_errors.py b/tests/functional/test_new_resolver_errors.py index 5976de52e39..ebca917d0a5 100644 --- a/tests/functional/test_new_resolver_errors.py +++ b/tests/functional/test_new_resolver_errors.py @@ -126,7 +126,9 @@ def test_new_resolver_checks_requires_python_before_dependencies( expect_error=True, ) - # Resolution should fail because of pkg-a's Requires-Python. - # This check should be done before pkg-b, so pkg-b should never be pulled. + # Resolution should fail because of pkg-root's Requires-Python. + # This is done before dependencies so pkg-dep should never be pulled. assert incompatible_python in result.stderr, str(result) - assert "pkg-b" not in result.stderr, str(result) + # Setuptools produces wheels with normalized names. + assert "pkg_dep" not in result.stderr, str(result) + assert "pkg_dep" not in result.stdout, str(result) From 5bd1255dda9f2d82c7de3c7ab62d934a4e517b96 Mon Sep 17 00:00:00 2001 From: Richard Si Date: Thu, 6 Mar 2025 20:27:12 -0500 Subject: [PATCH 2/2] Add comment about requires-python being first --- src/pip/_internal/resolution/resolvelib/candidates.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/pip/_internal/resolution/resolvelib/candidates.py b/src/pip/_internal/resolution/resolvelib/candidates.py index 3ddffe0375f..d976026ac18 100644 --- a/src/pip/_internal/resolution/resolvelib/candidates.py +++ b/src/pip/_internal/resolution/resolvelib/candidates.py @@ -249,6 +249,8 @@ def _prepare(self) -> BaseDistribution: return dist def iter_dependencies(self, with_requires: bool) -> Iterable[Optional[Requirement]]: + # Emit the Requires-Python requirement first to fail fast on + # unsupported candidates and avoid pointless downloads/preparation. yield self._factory.make_requires_python_requirement(self.dist.requires_python) requires = self.dist.iter_dependencies() if with_requires else () for r in requires: