From ea1d5ac484cc635abf1f4dd3d5cb0ae6e1ae8f4e Mon Sep 17 00:00:00 2001 From: Tzu-ping Chung Date: Wed, 31 Oct 2018 05:56:40 +0900 Subject: [PATCH] Isolate, reuse PackageFinder best candidate logic 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 | 123 ++++++++++++++++++++-------- src/pip/_internal/utils/outdated.py | 9 +- tests/unit/test_unit_outdated.py | 7 +- 4 files changed, 99 insertions(+), 41 deletions(-) create mode 100644 news/5175.bugfix diff --git a/news/5175.bugfix b/news/5175.bugfix new file mode 100644 index 00000000000..1ab90f4f35d --- /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 f251c4b58b6..173d90c3a8f 100644 --- a/src/pip/_internal/index.py +++ b/src/pip/_internal/index.py @@ -55,7 +55,8 @@ 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,63 @@ 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 + + @property + def all(self): + # type: () -> Iterable[InstallationCandidate] + return iter(self._candidates) + + @property + def applicable(self): + # type: () -> Iterable[InstallationCandidate] + # 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) + + @property + def best(self): + # type: () -> Optional[InstallationCandidate] + try: + return max(self.applicable, key=self._sort_key) + except ValueError: # Raised by max() if self.applicable is empty. + return None + + class PackageFinder(object): """This finds packages. @@ -628,6 +686,27 @@ def find_all_candidates(self, project_name): # This is an intentional priority ordering return file_versions + find_links_versions + page_versions + def find_candidates(self, project_name, specifier): + """Find matches for the given project and specifier. + + `specifier` should implement `filter` to allow version filtering (e.g. + ``packaging.specifiers.SpecifierSet``). + + Returns a FoundCandidates instance that contains the following + properties: + + * `all`: All candidates matching `project_name` found on the index. + * `applicable`: Candidates matching `project_name` and `specifier`. + * `best`: The best candidate available. This may be None if no + applicable candidates are found. + """ + 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,35 +715,8 @@ def find_requirement(self, req, upgrade): 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.best if req.satisfied_by is not None: installed_version = parse_version(req.satisfied_by.version) @@ -678,7 +730,7 @@ def find_requirement(self, req, upgrade): req, ', '.join( sorted( - {str(c.version) for c in all_candidates}, + {str(c.version) for c in candidates.all}, key=parse_version, ) ) @@ -710,21 +762,24 @@ def find_requirement(self, req, upgrade): ) return None + compatible_version_display = ", ".join( + str(v) for v in sorted({c.version for c in candidates.applicable}) + ) or "none" + if best_installed: # We have an existing version, and its the best version logger.debug( 'Installed version (%s) is most up-to-date (past versions: ' '%s)', installed_version, - ', '.join(sorted(compatible_versions, key=parse_version)) or - "none", + compatible_version_display, ) raise BestVersionAlreadyInstalled logger.debug( 'Using version %s (newest of versions: %s)', best_candidate.version, - ', '.join(sorted(compatible_versions, key=parse_version)) + compatible_version_display, ) return best_candidate.location diff --git a/src/pip/_internal/utils/outdated.py b/src/pip/_internal/utils/outdated.py index f7692761186..7d7d50cfdef 100644 --- a/src/pip/_internal/utils/outdated.py +++ b/src/pip/_internal/utils/outdated.py @@ -8,6 +8,7 @@ from pip._vendor import lockfile, pkg_resources from pip._vendor.packaging import version as packaging_version +from pip._vendor.packaging.specifiers import SpecifierSet from pip._internal.index import PackageFinder from pip._internal.utils.compat import WINDOWS @@ -129,12 +130,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", SpecifierSet()).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 31b5a7fa384..0696b71c7a4 100644 --- a/tests/unit/test_unit_outdated.py +++ b/tests/unit/test_unit_outdated.py @@ -1,3 +1,4 @@ +import collections import datetime import os import sys @@ -28,8 +29,10 @@ 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, specifier): + return collections.namedtuple("FoundCandidates", "best")( + best=self.INSTALLATION_CANDIDATES[0], + ) class MockDistribution(object):