diff --git a/news/9084.bugfix.rst b/news/9084.bugfix.rst new file mode 100644 index 00000000000..f2fbfc754bf --- /dev/null +++ b/news/9084.bugfix.rst @@ -0,0 +1 @@ +Extract the logic from package_finder.find_best_candidate and package_finder.find_all_candidate to static methods. diff --git a/src/pip/_internal/index/package_finder.py b/src/pip/_internal/index/package_finder.py old mode 100644 new mode 100755 index 7f2e04e7c37..f7b2ec5e14a --- a/src/pip/_internal/index/package_finder.py +++ b/src/pip/_internal/index/package_finder.py @@ -7,7 +7,8 @@ import itertools import logging import re -from typing import FrozenSet, Iterable, List, Optional, Set, Tuple, Union +from collections import namedtuple +from typing import Callable, FrozenSet, Iterable, List, Optional, Set, Tuple, Union from pip._vendor.packaging import specifiers from pip._vendor.packaging.tags import Tag @@ -48,6 +49,30 @@ Tuple[int, int, int, _BaseVersion, Optional[int], BuildTag] ) +PackageFinderTuple = namedtuple( + 'PackageFinderTuple', + """ + logged_links + link_collector + target_python + candidate_prefs + process_project_url + """ +) + +LinkEvaluatorTuple = namedtuple( + 'LinkEvaluatorTuple', + """ + project_name + canonical_name + formats + target_python + allow_yanked + ignore_requires_python + py_version_re + """ +) + def _check_link_requires_python( link, # type: Link @@ -142,8 +167,21 @@ def __init__( self.project_name = project_name - def evaluate_link(self, link): - # type: (Link) -> Tuple[bool, Optional[str]] + def get_state_as_tuple(self): + # type: (LinkEvaluator) -> LinkEvaluatorTuple + return LinkEvaluatorTuple( + project_name=self.project_name, + canonical_name=self._canonical_name, + formats=self._formats, + target_python=self._target_python, + allow_yanked=self._allow_yanked, + ignore_requires_python=self._ignore_requires_python, + py_version_re=self._py_version_re + ) + + @staticmethod + def evaluate_link_static(link_evaluator_tuple, link): + # type: (LinkEvaluatorTuple, Link) -> Tuple[bool, Optional[str]] """ Determine whether a link is a candidate for installation. @@ -153,7 +191,7 @@ def evaluate_link(self, link): the link fails to qualify. """ version = None - if link.is_yanked and not self._allow_yanked: + if link.is_yanked and not link_evaluator_tuple.allow_yanked: reason = link.yanked_reason or '' return (False, f'yanked for reason: {reason}') @@ -166,9 +204,9 @@ def evaluate_link(self, link): return (False, 'not a file') if ext not in SUPPORTED_EXTENSIONS: return (False, f'unsupported archive format: {ext}') - if "binary" not in self._formats and ext == WHEEL_EXTENSION: + if "binary" not in link_evaluator_tuple.formats and ext == WHEEL_EXTENSION: reason = 'No binaries permitted for {}'.format( - self.project_name) + link_evaluator_tuple.project_name) return (False, reason) if "macosx10" in link.path and ext == '.zip': return (False, 'macosx10 one') @@ -177,12 +215,12 @@ def evaluate_link(self, link): wheel = Wheel(link.filename) except InvalidWheelFilename: return (False, 'invalid wheel filename') - if canonicalize_name(wheel.name) != self._canonical_name: + if canonicalize_name(wheel.name) != link_evaluator_tuple.canonical_name: reason = 'wrong project name (not {})'.format( - self.project_name) + link_evaluator_tuple.project_name) return (False, reason) - supported_tags = self._target_python.get_tags() + supported_tags = link_evaluator_tuple.target_python.get_tags() if not wheel.supported(supported_tags): # Include the wheel's tags in the reason string to # simplify troubleshooting compatibility issues. @@ -198,28 +236,28 @@ def evaluate_link(self, link): version = wheel.version # This should be up by the self.ok_binary check, but see issue 2700. - if "source" not in self._formats and ext != WHEEL_EXTENSION: - reason = f'No sources permitted for {self.project_name}' + if "source" not in link_evaluator_tuple.formats and ext != WHEEL_EXTENSION: + reason = f'No sources permitted for {link_evaluator_tuple.project_name}' return (False, reason) if not version: version = _extract_version_from_fragment( - egg_info, self._canonical_name, + egg_info, link_evaluator_tuple.canonical_name, ) if not version: - reason = f'Missing project version for {self.project_name}' + reason = f'Missing project version for {link_evaluator_tuple.project_name}' return (False, reason) - match = self._py_version_re.search(version) + match = link_evaluator_tuple.py_version_re.search(version) if match: version = version[:match.start()] py_version = match.group(1) - if py_version != self._target_python.py_version: + if py_version != link_evaluator_tuple.target_python.py_version: return (False, 'Python version is incorrect') supports_python = _check_link_requires_python( - link, version_info=self._target_python.py_version_info, - ignore_requires_python=self._ignore_requires_python, + link, version_info=link_evaluator_tuple.target_python.py_version_info, + ignore_requires_python=link_evaluator_tuple.ignore_requires_python, ) if not supports_python: # Return None for the reason text to suppress calling @@ -230,6 +268,18 @@ def evaluate_link(self, link): return (True, version) + def evaluate_link(self, link): + # type: (Link) -> Tuple[bool, Optional[str]] + """ + Determine whether a link is a candidate for installation. + + :return: A tuple (is_candidate, result), where `result` is (1) a + version string if `is_candidate` is True, and (2) if + `is_candidate` is False, an optional string to log the reason + the link fails to qualify. + """ + return LinkEvaluator.evaluate_link_static(self.get_state_as_tuple(), link) + def filter_unallowed_hashes( candidates, # type: List[InstallationCandidate] @@ -555,6 +605,7 @@ def sort_best_candidate( if not candidates: return None best_candidate = max(candidates, key=self._sort_key) + return best_candidate def compute_best_candidate( @@ -719,84 +770,160 @@ def make_link_evaluator(self, project_name): ignore_requires_python=self._ignore_requires_python, ) - def _sort_links(self, links): - # type: (Iterable[Link]) -> List[Link] - """ - Returns elements of links in order, non-egg links first, egg links - second, while eliminating duplicates - """ - eggs, no_eggs = [], [] - seen = set() # type: Set[Link] - for link in links: - if link not in seen: - seen.add(link) - if link.egg_fragment: - eggs.append(link) - else: - no_eggs.append(link) - return no_eggs + eggs + def get_state_as_tuple(self, immutable=False): + # type: (bool) -> PackageFinderTuple - def _log_skipped_link(self, link, reason): - # type: (Link, str) -> None - if link not in self._logged_links: + if immutable: + return PackageFinderTuple( + logged_links=frozenset(self._logged_links), + link_collector=self._link_collector, + candidate_prefs=self._candidate_prefs, + target_python=self._target_python, + process_project_url=self.process_project_url + ) + + return PackageFinderTuple( + logged_links=self._logged_links, + link_collector=self._link_collector, + candidate_prefs=self._candidate_prefs, + target_python=self._target_python, + process_project_url=self.process_project_url + ) + + @staticmethod + def _log_skipped_link_static(package_finder_tuple, link, reason): + # type: (PackageFinderTuple, Link, str) -> None + if link not in package_finder_tuple.logged_links: # Put the link at the end so the reason is more visible and because # the link string is usually very long. logger.debug('Skipping link: %s: %s', reason, link) - self._logged_links.add(link) + package_finder_tuple.logged_links.add(link) - def get_install_candidate(self, link_evaluator, link): - # type: (LinkEvaluator, Link) -> Optional[InstallationCandidate] - """ - If the link is a candidate for install, convert it to an - InstallationCandidate and return it. Otherwise, return None. - """ - is_candidate, result = link_evaluator.evaluate_link(link) + def _log_skipped_link(self, link, reason): + # type: (Link, str) -> None + self._log_skipped_link_static(self.get_state_as_tuple(), link, reason) + + @staticmethod + def _get_install_candidate_static( + package_finder_tuple, # type: PackageFinderTuple + link_evaluator_tuple, # type: LinkEvaluatorTuple + link # type: Link + ): + # type: (...) -> Optional[InstallationCandidate] + is_candidate, result = LinkEvaluator.evaluate_link_static( + link_evaluator_tuple, + link + ) if not is_candidate: if result: - self._log_skipped_link(link, reason=result) + PackageFinder._log_skipped_link_static( + package_finder_tuple, + link, + reason=result + ) return None return InstallationCandidate( - name=link_evaluator.project_name, + name=link_evaluator_tuple.project_name, link=link, version=result, ) - def evaluate_links(self, link_evaluator, links): - # type: (LinkEvaluator, Iterable[Link]) -> List[InstallationCandidate] + def get_install_candidate(self, link_evaluator, link): + # type: (LinkEvaluator, Link) -> Optional[InstallationCandidate] """ - Convert links that are candidates to InstallationCandidate objects. + If the link is a candidate for install, convert it to an + InstallationCandidate and return it. Otherwise, return None. """ + return self._get_install_candidate_static( + self.get_state_as_tuple(), + link_evaluator.get_state_as_tuple(), + link + ) + + @staticmethod + def _evaluate_links_static( + package_finder_tuple, # type: PackageFinderTuple + link_evaluator_tuple, # type: LinkEvaluatorTuple + links # type: Iterable[Link] + ): + # type: (...) -> List[InstallationCandidate] candidates = [] - for link in self._sort_links(links): - candidate = self.get_install_candidate(link_evaluator, link) + + eggs, no_eggs = [], [] + seen = set() # type: Set[Link] + for link in links: + if link not in seen: + seen.add(link) + if link.egg_fragment: + eggs.append(link) + else: + no_eggs.append(link) + links = no_eggs + eggs + for link in links: + candidate = PackageFinder._get_install_candidate_static( + package_finder_tuple, + link_evaluator_tuple, + link + ) if candidate is not None: candidates.append(candidate) return candidates - def process_project_url(self, project_url, link_evaluator): - # type: (Link, LinkEvaluator) -> List[InstallationCandidate] + def evaluate_links(self, link_evaluator, links): + # type: (LinkEvaluator, Iterable[Link]) -> List[InstallationCandidate] + """ + Convert links that are candidates to InstallationCandidate objects. + """ + return self._evaluate_links_static( + self.get_state_as_tuple(), + link_evaluator.get_state_as_tuple(), + links + ) + + @staticmethod + def _process_project_url_static( + package_finder_tuple, # type: PackageFinderTuple + project_url, # type: Link + link_evaluator_tuple # type: LinkEvaluatorTuple + ): + # type: (...) -> List[InstallationCandidate] logger.debug( 'Fetching project page and analyzing links: %s', project_url, ) - html_page = self._link_collector.fetch_page(project_url) + html_page = package_finder_tuple.link_collector.fetch_page(project_url) if html_page is None: return [] page_links = list(parse_links(html_page)) with indent_log(): - package_links = self.evaluate_links( - link_evaluator, + package_links = PackageFinder._evaluate_links_static( + package_finder_tuple, + link_evaluator_tuple, links=page_links, ) return package_links + def process_project_url(self, project_url, link_evaluator): + # type: (Link, LinkEvaluator) -> List[InstallationCandidate] + return self._process_project_url_static( + self.get_state_as_tuple(), + project_url, + link_evaluator.get_state_as_tuple() + ) + + @staticmethod @functools.lru_cache(maxsize=None) - def find_all_candidates(self, project_name): - # type: (str) -> List[InstallationCandidate] + def _find_all_candidates_static( + package_finder_tuple, # type: PackageFinderTuple + link_evaluator_tuple, # type: LinkEvaluatorTuple + project_name, # type: str + candidates_from_page # type: Callable[..., List[InstallationCandidate]] + ): + # type: (...) -> Tuple[List[InstallationCandidate], Set[Link]] """Find all available InstallationCandidate for project_name This checks index_urls and find_links. @@ -805,14 +932,18 @@ def find_all_candidates(self, project_name): See LinkEvaluator.evaluate_link() for details on which files are accepted. """ - link_evaluator = self.make_link_evaluator(project_name) - collected_sources = self._link_collector.collect_sources( + package_finder_tuple = PackageFinderTuple( + logged_links=set(package_finder_tuple.logged_links), + link_collector=package_finder_tuple.link_collector, + target_python=package_finder_tuple.target_python, + candidate_prefs=package_finder_tuple.candidate_prefs, + process_project_url=package_finder_tuple.process_project_url + ) + + collected_sources = package_finder_tuple.link_collector.collect_sources( project_name=project_name, - candidates_from_page=functools.partial( - self.process_project_url, - link_evaluator=link_evaluator, - ), + candidates_from_page=candidates_from_page, ) page_candidates_it = itertools.chain.from_iterable( @@ -829,8 +960,9 @@ def find_all_candidates(self, project_name): for source in sources if source is not None ) - file_candidates = self.evaluate_links( - link_evaluator, + file_candidates = PackageFinder._evaluate_links_static( + package_finder_tuple, + link_evaluator_tuple, sorted(file_links_it, reverse=True), ) @@ -839,7 +971,57 @@ def find_all_candidates(self, project_name): logger.debug("Local files found: %s", ", ".join(paths)) # This is an intentional priority ordering - return file_candidates + page_candidates + return ( + file_candidates + page_candidates, + package_finder_tuple.logged_links + ) + + def find_all_candidates(self, project_name): + # type: (str) -> List[InstallationCandidate] + """Find all available InstallationCandidate for project_name + + This checks index_urls and find_links. + All versions found are returned as an InstallationCandidate list. + + See LinkEvaluator.evaluate_link() for details on which files + are accepted. + """ + link_evaluator = self.make_link_evaluator(project_name) + + package_finder_tuple = self.get_state_as_tuple(immutable=True) + link_evaluator_tuple = link_evaluator.get_state_as_tuple() + candidates_from_page = functools.partial( + package_finder_tuple.process_project_url, + link_evaluator=link_evaluator, + ) + (candidates, logged_links) = self._find_all_candidates_static( + package_finder_tuple, + link_evaluator_tuple, + project_name, + candidates_from_page + ) + + self._logged_links = logged_links + + return candidates + + @staticmethod + def _make_candidate_evaluator_static( + package_finder_tuple, # type: PackageFinderTuple + project_name, # type: str + specifier=None, # type: Optional[specifiers.BaseSpecifier] + hashes=None, # type: Optional[Hashes] + ): + # type: (...) -> CandidateEvaluator + candidate_prefs = package_finder_tuple.candidate_prefs + return CandidateEvaluator.create( + project_name=project_name, + target_python=package_finder_tuple.target_python, + prefer_binary=candidate_prefs.prefer_binary, + allow_all_prereleases=candidate_prefs.allow_all_prereleases, + specifier=specifier, + hashes=hashes, + ) def make_candidate_evaluator( self, @@ -850,17 +1032,41 @@ def make_candidate_evaluator( # type: (...) -> CandidateEvaluator """Create a CandidateEvaluator object to use. """ - candidate_prefs = self._candidate_prefs - return CandidateEvaluator.create( - project_name=project_name, - target_python=self._target_python, - prefer_binary=candidate_prefs.prefer_binary, - allow_all_prereleases=candidate_prefs.allow_all_prereleases, - specifier=specifier, - hashes=hashes, + package_finder_tuple = self.get_state_as_tuple() + return self._make_candidate_evaluator_static( + package_finder_tuple, + project_name, + specifier, + hashes ) + @staticmethod @functools.lru_cache(maxsize=None) + def _find_best_candidate_static( + package_finder_tuple, # type: PackageFinderTuple + link_evaluator_tuple, # type: LinkEvaluatorTuple + project_name, # type: str + candidates_from_page, # type: Callable[..., List[InstallationCandidate]] + specifier=None, # type: Optional[specifiers.BaseSpecifier] + hashes=None # type: Optional[Hashes] + ): + # type: (...) -> Tuple[BestCandidateResult, Set[Link]] + candidate_evaluator = PackageFinder._make_candidate_evaluator_static( + package_finder_tuple, + project_name, + specifier, + hashes + ) + + (candidates, logged_links) = PackageFinder._find_all_candidates_static( + package_finder_tuple, + link_evaluator_tuple, + project_name, + candidates_from_page + ) + + return (candidate_evaluator.compute_best_candidate(candidates), logged_links) + def find_best_candidate( self, project_name, # type: str @@ -874,15 +1080,30 @@ def find_best_candidate( (e.g. `packaging.specifiers.SpecifierSet`) to filter applicable versions. - :return: A `BestCandidateResult` instance. + :return: A `BestCandidateResult` instance, + called from a static function that caches function calls. """ - candidates = self.find_all_candidates(project_name) - candidate_evaluator = self.make_candidate_evaluator( - project_name=project_name, - specifier=specifier, - hashes=hashes, + link_evaluator = self.make_link_evaluator(project_name) + + package_finder_tuple = self.get_state_as_tuple(immutable=True) + link_evaluator_tuple = link_evaluator.get_state_as_tuple() + + candidates_from_page = functools.partial( + package_finder_tuple.process_project_url, + link_evaluator=link_evaluator, ) - return candidate_evaluator.compute_best_candidate(candidates) + (best_candidate, logged_links) = self._find_best_candidate_static( + package_finder_tuple, + link_evaluator_tuple, + project_name, + candidates_from_page, + specifier, + hashes + ) + + self._logged_links = logged_links + + return best_candidate def find_requirement(self, req, upgrade): # type: (InstallRequirement, bool) -> Optional[InstallationCandidate] diff --git a/tests/unit/test_resolution_legacy_resolver.py b/tests/unit/test_resolution_legacy_resolver.py old mode 100644 new mode 100755 index 236a4c624e7..60d555f0f26 --- a/tests/unit/test_resolution_legacy_resolver.py +++ b/tests/unit/test_resolution_legacy_resolver.py @@ -5,6 +5,7 @@ from pip._vendor import pkg_resources from pip._internal.exceptions import NoneMetadataError, UnsupportedPythonVersion +from pip._internal.index.package_finder import PackageFinder from pip._internal.req.constructors import install_req_from_line from pip._internal.resolution.legacy.resolver import ( Resolver, @@ -178,11 +179,20 @@ class TestYankedWarning: Test _populate_link() emits warning if one or more candidates are yanked. """ def _make_test_resolver(self, monkeypatch, mock_candidates): - def _find_candidates(project_name): - return mock_candidates + def _find_candidates( + package_finder, + link_evaluator, + project_name, + candidates_from_page + ): + return (mock_candidates, []) finder = make_test_finder() - monkeypatch.setattr(finder, "find_all_candidates", _find_candidates) + monkeypatch.setattr( + PackageFinder, + "_find_all_candidates_static", + _find_candidates + ) return Resolver( finder=finder,