From 68a86c53c7719cac5129561c72ed5b74f8c30263 Mon Sep 17 00:00:00 2001 From: Tzu-ping Chung Date: Wed, 27 Jan 2021 19:40:01 +0800 Subject: [PATCH 1/3] Failing test for repeated fetch --- tests/functional/test_new_resolver.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/functional/test_new_resolver.py b/tests/functional/test_new_resolver.py index 6943fd7de80..e2688f05b12 100644 --- a/tests/functional/test_new_resolver.py +++ b/tests/functional/test_new_resolver.py @@ -1253,3 +1253,28 @@ def test_new_resolver_lazy_fetch_candidates(script, upgrade): # But should reach there in the best route possible, without trying # candidates it does not need to. assert "myuberpkg-2" not in result.stdout, str(result) + + +def test_new_resolver_no_fetch_no_satisfying(script): + create_basic_wheel_for_package(script, "myuberpkg", "1") + + # Install the package. This should emit a "Processing" message for + # fetching the distribution from the --find-links page. + result = script.pip( + "install", + "--no-cache-dir", "--no-index", + "--find-links", script.scratch_path, + "myuberpkg", + ) + assert "Processing ./myuberpkg-1-" in result.stdout, str(result) + + # Try to upgrade the package. This should NOT emit the "Processing" + # message because the currently installed version is latest. + result = script.pip( + "install", + "--no-cache-dir", "--no-index", + "--find-links", script.scratch_path, + "--upgrade", + "myuberpkg", + ) + assert "Processing ./myuberpkg-1-" not in result.stdout, str(result) From 2e70ec075126d4928e17c32d9b63b3839f8b0b74 Mon Sep 17 00:00:00 2001 From: Tzu-ping Chung Date: Wed, 27 Jan 2021 19:47:35 +0800 Subject: [PATCH 2/3] Create the candidate lazily to avoid download --- news/9516.bugfix.rst | 3 + .../resolution/resolvelib/factory.py | 19 ++--- .../resolution/resolvelib/found_candidates.py | 82 ++++++++++++++----- 3 files changed, 74 insertions(+), 30 deletions(-) create mode 100644 news/9516.bugfix.rst diff --git a/news/9516.bugfix.rst b/news/9516.bugfix.rst new file mode 100644 index 00000000000..9b9cc812614 --- /dev/null +++ b/news/9516.bugfix.rst @@ -0,0 +1,3 @@ +New resolver: Download and prepare a distribution only at the last possible +moment to avoid unnecessary network access when the same version is already +installed locally. diff --git a/src/pip/_internal/resolution/resolvelib/factory.py b/src/pip/_internal/resolution/resolvelib/factory.py index bfaa0520ace..be0729e3994 100644 --- a/src/pip/_internal/resolution/resolvelib/factory.py +++ b/src/pip/_internal/resolution/resolvelib/factory.py @@ -1,3 +1,4 @@ +import functools import logging from pip._vendor.packaging.utils import canonicalize_name @@ -65,6 +66,7 @@ from .base import Candidate, Requirement from .candidates import BaseCandidate + from .found_candidates import IndexCandidateInfo C = TypeVar("C") Cache = Dict[Link, C] @@ -213,8 +215,8 @@ def _iter_found_candidates( template=template, ) - def iter_index_candidates(): - # type: () -> Iterator[Candidate] + def iter_index_candidate_infos(): + # type: () -> Iterator[IndexCandidateInfo] result = self._finder.find_best_candidate( project_name=name, specifier=specifier, @@ -228,26 +230,21 @@ def iter_index_candidates(): all_yanked = all(ican.link.is_yanked for ican in icans) # PackageFinder returns earlier versions first, so we reverse. - versions_found = set() # type: Set[_BaseVersion] for ican in reversed(icans): if not all_yanked and ican.link.is_yanked: continue - if ican.version in versions_found: - continue - candidate = self._make_candidate_from_link( + func = functools.partial( + self._make_candidate_from_link, link=ican.link, extras=extras, template=template, name=name, version=ican.version, ) - if candidate is None: - continue - yield candidate - versions_found.add(ican.version) + yield ican.version, func return FoundCandidates( - iter_index_candidates, + iter_index_candidate_infos, installed_candidate, prefers_installed, ) diff --git a/src/pip/_internal/resolution/resolvelib/found_candidates.py b/src/pip/_internal/resolution/resolvelib/found_candidates.py index c3f95c1d41d..5d4b4654988 100644 --- a/src/pip/_internal/resolution/resolvelib/found_candidates.py +++ b/src/pip/_internal/resolution/resolvelib/found_candidates.py @@ -9,20 +9,62 @@ """ import functools -import itertools from pip._vendor.six.moves import collections_abc # type: ignore from pip._internal.utils.typing import MYPY_CHECK_RUNNING if MYPY_CHECK_RUNNING: - from typing import Callable, Iterator, Optional + from typing import Callable, Iterator, Optional, Set, Tuple + + from pip._vendor.packaging.version import _BaseVersion from .base import Candidate + IndexCandidateInfo = Tuple[_BaseVersion, Callable[[], Optional[Candidate]]] + + +def _iter_built(infos): + # type: (Iterator[IndexCandidateInfo]) -> Iterator[Candidate] + """Iterator for ``FoundCandidates``. + + This iterator is used the package is not already installed. Candidates + from index come later in their normal ordering. + """ + versions_found = set() # type: Set[_BaseVersion] + for version, func in infos: + if version in versions_found: + continue + candidate = func() + if candidate is None: + continue + yield candidate + versions_found.add(version) + + +def _iter_built_with_prepended(installed, infos): + # type: (Candidate, Iterator[IndexCandidateInfo]) -> Iterator[Candidate] + """Iterator for ``FoundCandidates``. + + This iterator is used when the resolver prefers the already-installed + candidate and NOT to upgrade. The installed candidate is therefore + always yielded first, and candidates from index come later in their + normal ordering, except skipped when the version is already installed. + """ + yield installed + versions_found = {installed.version} # type: Set[_BaseVersion] + for version, func in infos: + if version in versions_found: + continue + candidate = func() + if candidate is None: + continue + yield candidate + versions_found.add(version) + -def _insert_installed(installed, others): - # type: (Candidate, Iterator[Candidate]) -> Iterator[Candidate] +def _iter_built_with_inserted(installed, infos): + # type: (Candidate, Iterator[IndexCandidateInfo]) -> Iterator[Candidate] """Iterator for ``FoundCandidates``. This iterator is used when the resolver prefers to upgrade an @@ -33,16 +75,22 @@ def _insert_installed(installed, others): the installed candidate exactly once before we start yielding older or equivalent candidates, or after all other candidates if they are all newer. """ - installed_yielded = False - for candidate in others: + versions_found = set() # type: Set[_BaseVersion] + for version, func in infos: + if version in versions_found: + continue # If the installed candidate is better, yield it first. - if not installed_yielded and installed.version >= candidate.version: + if installed.version >= version: yield installed - installed_yielded = True + versions_found.add(installed.version) + candidate = func() + if candidate is None: + continue yield candidate + versions_found.add(version) # If the installed candidate is older than all other candidates. - if not installed_yielded: + if installed.version not in versions_found: yield installed @@ -56,11 +104,11 @@ class FoundCandidates(collections_abc.Sequence): """ def __init__( self, - get_others, # type: Callable[[], Iterator[Candidate]] + get_infos, # type: Callable[[], Iterator[IndexCandidateInfo]] installed, # type: Optional[Candidate] prefers_installed, # type: bool ): - self._get_others = get_others + self._get_infos = get_infos self._installed = installed self._prefers_installed = prefers_installed @@ -73,16 +121,12 @@ def __getitem__(self, index): def __iter__(self): # type: () -> Iterator[Candidate] + infos = self._get_infos() if not self._installed: - return self._get_others() - others = ( - candidate - for candidate in self._get_others() - if candidate.version != self._installed.version - ) + return _iter_built(infos) if self._prefers_installed: - return itertools.chain([self._installed], others) - return _insert_installed(self._installed, others) + return _iter_built_with_prepended(self._installed, infos) + return _iter_built_with_inserted(self._installed, infos) def __len__(self): # type: () -> int From 7d43ec5166088ed02c7339e1aa700adca44856b4 Mon Sep 17 00:00:00 2001 From: Tzu-ping Chung Date: Wed, 27 Jan 2021 21:55:46 +0800 Subject: [PATCH 3/3] More permissive output check --- tests/functional/test_new_resolver.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/functional/test_new_resolver.py b/tests/functional/test_new_resolver.py index e2688f05b12..4d2acbb23b4 100644 --- a/tests/functional/test_new_resolver.py +++ b/tests/functional/test_new_resolver.py @@ -1266,7 +1266,7 @@ def test_new_resolver_no_fetch_no_satisfying(script): "--find-links", script.scratch_path, "myuberpkg", ) - assert "Processing ./myuberpkg-1-" in result.stdout, str(result) + assert "Processing " in result.stdout, str(result) # Try to upgrade the package. This should NOT emit the "Processing" # message because the currently installed version is latest. @@ -1277,4 +1277,4 @@ def test_new_resolver_no_fetch_no_satisfying(script): "--upgrade", "myuberpkg", ) - assert "Processing ./myuberpkg-1-" not in result.stdout, str(result) + assert "Processing " not in result.stdout, str(result)