From 14cb4f4fb6e28ebca6bd64378a44f994bf9e4d22 Mon Sep 17 00:00:00 2001 From: Tzu-ping Chung Date: Wed, 17 Apr 2019 07:46:25 +0800 Subject: [PATCH] Isolate, reuse PackageFinder best candidate logic (#5971) Split out how PackageFinder finds the best candidate, and reuse it in the self version check, to avoid the latter duplicating (and incorrectly implementing) the same logic. --- news/5175.bugfix | 1 + src/pip/_internal/index.py | 138 +++++++++++++++++++--------- src/pip/_internal/utils/outdated.py | 8 +- tests/unit/test_unit_outdated.py | 12 ++- 4 files changed, 111 insertions(+), 48 deletions(-) create mode 100644 news/5175.bugfix diff --git a/news/5175.bugfix b/news/5175.bugfix new file mode 100644 index 000000000..1ab90f4f3 --- /dev/null +++ b/news/5175.bugfix @@ -0,0 +1 @@ +Make pip's self version check avoid recommending upgrades to prereleases if the currently-installed version is stable. diff --git a/src/pip/_internal/index.py b/src/pip/_internal/index.py index a1ef4b6d5..1f251448b 100644 --- a/src/pip/_internal/index.py +++ b/src/pip/_internal/index.py @@ -55,7 +55,8 @@ if MYPY_CHECK_RUNNING: BuildTag = Tuple[Any, ...] # either empty tuple or Tuple[int, str] CandidateSortingKey = Tuple[int, _BaseVersion, BuildTag, Optional[int]] -__all__ = ['FormatControl', 'PackageFinder'] + +__all__ = ['FormatControl', 'FoundCandidates', 'PackageFinder'] SECURE_ORIGINS = [ @@ -254,6 +255,67 @@ def _get_html_page(link, session=None): return None +class FoundCandidates(object): + """A collection of candidates, returned by `PackageFinder.find_candidates`. + + Arguments: + + * `candidates`: A sequence of all available candidates found. + * `specifier`: Specifier to filter applicable versions. + * `prereleases`: Whether prereleases should be accounted. Pass None to + infer from the specifier. + * `sort_key`: A callable used as the key function when choosing the best + candidate. + """ + def __init__( + self, + candidates, # type: List[InstallationCandidate] + specifier, # type: specifiers.BaseSpecifier + prereleases, # type: Optional[bool] + sort_key, # type: Callable[[InstallationCandidate], Any] + ): + # type: (...) -> None + self._candidates = candidates + self._specifier = specifier + self._prereleases = prereleases + self._sort_key = sort_key + + def iter_all(self): + # type: () -> Iterable[InstallationCandidate] + """Iterate through all candidates. + """ + return iter(self._candidates) + + def iter_applicable(self): + # type: () -> Iterable[InstallationCandidate] + """Iterate through candidates matching the given specifier. + """ + # Filter out anything which doesn't match our specifier. + versions = set(self._specifier.filter( + # We turn the version object into a str here because otherwise + # when we're debundled but setuptools isn't, Python will see + # packaging.version.Version and + # pkg_resources._vendor.packaging.version.Version as different + # types. This way we'll use a str as a common data interchange + # format. If we stop using the pkg_resources provided specifier + # and start using our own, we can drop the cast to str(). + [str(c.version) for c in self._candidates], + prereleases=self._prereleases, + )) + # Again, converting to str to deal with debundling. + return (c for c in self._candidates if str(c.version) in versions) + + def get_best(self): + # type: () -> Optional[InstallationCandidate] + """Return the best candidate available, or None if no applicable + candidates are found. + """ + candidates = list(self.iter_applicable()) + if not candidates: + return None + return max(candidates, key=self._sort_key) + + class PackageFinder(object): """This finds packages. @@ -628,6 +690,25 @@ class PackageFinder(object): # This is an intentional priority ordering return file_versions + find_links_versions + page_versions + def find_candidates( + self, + project_name, # type: str + specifier=specifiers.SpecifierSet(), # type: specifiers.BaseSpecifier + ): + """Find matches for the given project and specifier. + + If given, `specifier` should implement `filter` to allow version + filtering (e.g. ``packaging.specifiers.SpecifierSet``). + + Returns a `FoundCandidates` instance. + """ + return FoundCandidates( + self.find_all_candidates(project_name), + specifier=specifier, + prereleases=(self.allow_all_prereleases or None), + sort_key=self._candidate_sort_key, + ) + def find_requirement(self, req, upgrade): # type: (InstallRequirement, bool) -> Optional[Link] """Try to find a Link matching req @@ -636,52 +717,28 @@ class PackageFinder(object): Returns a Link if found, Raises DistributionNotFound or BestVersionAlreadyInstalled otherwise """ - all_candidates = self.find_all_candidates(req.name) - - # Filter out anything which doesn't match our specifier - compatible_versions = set( - req.specifier.filter( - # We turn the version object into a str here because otherwise - # when we're debundled but setuptools isn't, Python will see - # packaging.version.Version and - # pkg_resources._vendor.packaging.version.Version as different - # types. This way we'll use a str as a common data interchange - # format. If we stop using the pkg_resources provided specifier - # and start using our own, we can drop the cast to str(). - [str(c.version) for c in all_candidates], - prereleases=( - self.allow_all_prereleases - if self.allow_all_prereleases else None - ), - ) - ) - applicable_candidates = [ - # Again, converting to str to deal with debundling. - c for c in all_candidates if str(c.version) in compatible_versions - ] - - if applicable_candidates: - best_candidate = max(applicable_candidates, - key=self._candidate_sort_key) - else: - best_candidate = None + candidates = self.find_candidates(req.name, req.specifier) + best_candidate = candidates.get_best() + installed_version = None # type: Optional[_BaseVersion] if req.satisfied_by is not None: installed_version = parse_version(req.satisfied_by.version) - else: - installed_version = None + + def _format_versions(cand_iter): + # This repeated parse_version and str() conversion is needed to + # handle different vendoring sources from pip and pkg_resources. + # If we stop using the pkg_resources provided specifier. + return ", ".join(sorted( + {str(c.version) for c in cand_iter}, + key=parse_version, + )) or "none" if installed_version is None and best_candidate is None: logger.critical( 'Could not find a version that satisfies the requirement %s ' '(from versions: %s)', req, - ', '.join( - sorted( - {str(c.version) for c in all_candidates}, - key=parse_version, - ) - ) + _format_versions(candidates.iter_all()), ) raise DistributionNotFound( @@ -716,15 +773,14 @@ class PackageFinder(object): 'Installed version (%s) is most up-to-date (past versions: ' '%s)', installed_version, - ', '.join(sorted(compatible_versions, key=parse_version)) or - "none", + _format_versions(candidates.iter_applicable()), ) raise BestVersionAlreadyInstalled logger.debug( 'Using version %s (newest of versions: %s)', best_candidate.version, - ', '.join(sorted(compatible_versions, key=parse_version)) + _format_versions(candidates.iter_applicable()), ) return best_candidate.location diff --git a/src/pip/_internal/utils/outdated.py b/src/pip/_internal/utils/outdated.py index f76927611..3b58cd5e2 100644 --- a/src/pip/_internal/utils/outdated.py +++ b/src/pip/_internal/utils/outdated.py @@ -129,12 +129,10 @@ def pip_version_check(session, options): trusted_hosts=options.trusted_hosts, session=session, ) - all_candidates = finder.find_all_candidates("pip") - if not all_candidates: + candidate = finder.find_candidates("pip").get_best() + if candidate is None: return - pypi_version = str( - max(all_candidates, key=lambda c: c.version).version - ) + pypi_version = str(candidate.version) # save that we've performed a check state.save(pypi_version, current_time) diff --git a/tests/unit/test_unit_outdated.py b/tests/unit/test_unit_outdated.py index 31b5a7fa3..9d46ac41c 100644 --- a/tests/unit/test_unit_outdated.py +++ b/tests/unit/test_unit_outdated.py @@ -12,6 +12,14 @@ from pip._internal.index import InstallationCandidate from pip._internal.utils import outdated +class MockFoundCandidates(object): + def __init__(self, best): + self._best = best + + def get_best(self): + return self._best + + class MockPackageFinder(object): BASE_URL = 'https://pypi.org/simple/pip-{0}.tar.gz' @@ -28,8 +36,8 @@ class MockPackageFinder(object): def __init__(self, *args, **kwargs): pass - def find_all_candidates(self, project_name): - return self.INSTALLATION_CANDIDATES + def find_candidates(self, project_name): + return MockFoundCandidates(self.INSTALLATION_CANDIDATES[0]) class MockDistribution(object):