Repository: pypa/pip

Issue:
pip 9 offers upgrades to prereleases
* Pip version: 9.0.3
* Python version: 2.7.14
* Operating system: Ubuntu 17.10

### Description:

Any pip command says

```
You are using pip version 9.0.3, however version 10.0.0b2 is available.
You should consider upgrading via the 'pip install --upgrade pip' command.
```

I'm not sure it's a good idea to offer upgrades to pre-release versions?  Anyway, the suggested command doesn't work (because it needs --pre to allow upgrades to pre-release versions):

```
Requirement already up-to-date: pip in /home/mg/.venv/lib/python2.7/site-packages
You are using pip version 9.0.3, however version 10.0.0b2 is available.
You should consider upgrading via the 'pip install --upgrade pip' command.
```


Discussion:
cjerdonek: Question / observation: to make this much more easily testable (and easier to understand, etc), it looks like you might be able to convert both of `find_requirement()` and `find_best_match()` into pure functions that accept a return value of `find_all_candidates()` (and some other simpler arguments). Then `find_requirement()` could be implemented as calling `find_all_candidates()`, followed by the one-line call to the pure function.

Does that look right to you?

uranusjr: I considered that approach as well, but opted for the least-effort one in the end. I’ll try it instead. The call in `pip_version_check` would look a little awkward, but that is probably because of the convoluted state of `PackageFinder`. Hopefully I can untangle things more with future refactoring…
cjerdonek: Coming back to this (I was busy with other things lately), I think your previous version might have been better. I don't know for sure because I can't see it any longer, but it might be because of the awkwardness you mentioned -- without a similar gain to offset it. My suggestion of passing the return value of `find_all_candidates()` was only if it would result in being able to factor out a pure function (which would be the gain). That's what I was asking about the possibility of. Looking at the code more closely myself after seeing your latest version, it looks like `PackageFinder._candidate_sort_key()` is preventing this from being done easily. Maybe you encountered that yourself when you tried.
uranusjr: Yes, indeed `_candidate_sort_key` is the most problematic part. I have the previous version locally, but looking at it now, I feel both versions have some better parts. I’ll need to spend sometime (maybe this weekend) merging the good parts together.
pradyunsg: Looking at the current version, I think it does what's needed well enough; and @uranusjr's suggested plan moving forward sounds fair to me.

I'll defer to @cjerdonek since he's been more involved here than I have. :)
uranusjr: Soft ping :)
pypa-bot: Hello!

I am an automated bot and I have noticed that this pull request is not currently able to be merged. If you are able to either merge the ``master`` branch into this pull request or rebase this pull request against ``master`` then it will eligible for code review and hopefully merging!
uranusjr: This looks even cleaner than I expected! Thanks so much @cjerdonek 😀 

(Also rebased to keep the bot happy.)
cjerdonek: I have some comments after the latest changes.
uranusjr: Are there things I could do to push this forward? Should I change `FoundCandidates`’s specifier init argument to optional? I feel it is quite unnecessary since the class is only intended to be instantiated by `PackageFinder`, and default values only makes things less obvious.
cjerdonek: Hi @uranusjr, sorry! I actually _just_ spent time going over this again an hour or so ago. I had been wanting to go over it again for a while but only just now got to it. I had some further minor thoughts, but I think it looks good enough. I was planning to merge it first thing tomorrow. Thanks for your thought and work on this and for your patience.
cjerdonek: PS - if there were further tweaks you wanted to do, you could always do that in a later PR. (And if it's a smaller / easier PR, I promise reviews can happen way faster.) It's only when there's more going on in a PR that it slows things down.

cjerdonek: Regarding `FoundCandidates`'s init method, I was actually realizing it would be simpler to pass the set of versions to filter by, instead of passing in `specifier` and `prereleases` and recomputing that set each time. For example, since the set is needed anyways, it seems like it shouldn't have to be computed more than once (including even just for formatting). But it would be better to discuss further improvements after merging what you already have IMO, as opposed to postponing further.
uranusjr: Ah, I see what you mean, good point. I’ll work on improving that after this gets merged, then.

Patch:
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,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)


Test patch:
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):