Move _link_package_versions() to CandidateEvaluator

src/pip/_internal/index.py

@@ -258,20 +258,123 @@ def _get_html_page(link, session=None):
 
 class CandidateEvaluator(object):
 
+    """
+    Responsible for filtering and sorting candidates for installation based
+    on what tags are valid.
+    """
+
     def __init__(
         self,
         valid_tags,          # type: List[Pep425Tag]
         prefer_binary=False  # type: bool
-
     ):
         # type: (...) -> None
         self._prefer_binary = prefer_binary
         self._valid_tags = valid_tags
 
-    def is_wheel_supported(self, wheel):
+        # We compile the regex here instead of as a class attribute so as
+        # not to not impact pip start-up time.  This is also okay because
+        # CandidateEvaluator is generally instantiated only once per pip
+        # invocation (when PackageFinder is instantiated).
+        self._py_version_re = re.compile(r'-py([123]\.?[0-9]?)$')
+        # These are boring links that have already been logged somehow.
+        self._logged_links = set()  # type: Set[Link]
+
+    def _log_skipped_link(self, link, reason):
+        # type: (Link, str) -> None
+        if link not in self._logged_links:
+            logger.debug('Skipping link %s; %s', link, reason)
+            self._logged_links.add(link)
+
+    def _is_wheel_supported(self, wheel):
         # type: (Wheel) -> bool
         return wheel.supported(self._valid_tags)
 
+    def evaluate_link(self, link, search):
+        # type: (Link, Search) -> Optional[InstallationCandidate]
+        """
+        Determine whether a link is a candidate for installation.
+
+        Returns an InstallationCandidate if so, otherwise None.
+        """
+        version = None
+        if link.egg_fragment:
+            egg_info = link.egg_fragment
+            ext = link.ext
+        else:
+            egg_info, ext = link.splitext()
+            if not ext:
+                self._log_skipped_link(link, 'not a file')
+                return None
+            if ext not in SUPPORTED_EXTENSIONS:
+                self._log_skipped_link(
+                    link, 'unsupported archive format: %s' % ext,
+                )
+                return None
+            if "binary" not in search.formats and ext == WHEEL_EXTENSION:
+                self._log_skipped_link(
+                    link, 'No binaries permitted for %s' % search.supplied,
+                )
+                return None
+            if "macosx10" in link.path and ext == '.zip':
+                self._log_skipped_link(link, 'macosx10 one')
+                return None
+            if ext == WHEEL_EXTENSION:
+                try:
+                    wheel = Wheel(link.filename)
+                except InvalidWheelFilename:
+                    self._log_skipped_link(link, 'invalid wheel filename')
+                    return None
+                if canonicalize_name(wheel.name) != search.canonical:
+                    self._log_skipped_link(
+                        link, 'wrong project name (not %s)' % search.supplied)
+                    return None
+
+                if not self._is_wheel_supported(wheel):
+                    self._log_skipped_link(
+                        link, 'it is not compatible with this Python')
+                    return None
+
+                version = wheel.version
+
+        # This should be up by the search.ok_binary check, but see issue 2700.
+        if "source" not in search.formats and ext != WHEEL_EXTENSION:
+            self._log_skipped_link(
+                link, 'No sources permitted for %s' % search.supplied,
+            )
+            return None
+
+        if not version:
+            version = _egg_info_matches(egg_info, search.canonical)
+        if not version:
+            self._log_skipped_link(
+                link, 'Missing project version for %s' % search.supplied)
+            return None
+
+        match = self._py_version_re.search(version)
+        if match:
+            version = version[:match.start()]
+            py_version = match.group(1)
+            if py_version != sys.version[:3]:
+                self._log_skipped_link(
+                    link, 'Python version is incorrect')
+                return None
+        try:
+            support_this_python = check_requires_python(link.requires_python)
+        except specifiers.InvalidSpecifier:
+            logger.debug("Package %s has an invalid Requires-Python entry: %s",
+                         link.filename, link.requires_python)
+            support_this_python = True
+
+        if not support_this_python:
+            logger.debug("The package %s is incompatible with the python "
+                         "version in use. Acceptable python versions are: %s",
+                         link, link.requires_python)
+            return None
+        logger.debug('Found link %s, version: %s', link, version)
+
+        return InstallationCandidate(search.supplied, version, link)
+
     def _sort_key(self, candidate):
         # type: (InstallationCandidate) -> CandidateSortingKey
         """
@@ -293,7 +396,7 @@ def _sort_key(self, candidate):
         if candidate.location.is_wheel:
             # can raise InvalidWheelFilename
             wheel = Wheel(candidate.location.filename)
-            if not wheel.supported(self._valid_tags):
+            if not self._is_wheel_supported(wheel):
                 raise UnsupportedWheel(
                     "%s is not a supported wheel for this platform. It "
                     "can't be sorted." % wheel.filename
@@ -457,9 +560,6 @@ def __init__(
 
         self.index_urls = index_urls
 
-        # These are boring links that have already been logged somehow:
-        self.logged_links = set()  # type: Set[Link]
-
         self.format_control = format_control or FormatControl(set(), set())
 
         # Domains that we won't emit warnings for when not using HTTPS
@@ -668,13 +768,13 @@ def mkurl_pypi_url(url):
         return [mkurl_pypi_url(url) for url in self.index_urls]
 
     def find_all_candidates(self, project_name):
-        # type: (str) -> List[Optional[InstallationCandidate]]
+        # 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 _link_package_versions for details on which files are accepted
+        See evaluate_link() for details on which files are accepted
         """
         index_locations = self._get_index_urls_locations(project_name)
         index_file_loc, index_url_loc = self._sort_locations(index_locations)
@@ -849,8 +949,6 @@ def _get_pages(self, locations, project_name):
 
             yield page
 
-    _py_version_re = re.compile(r'-py([123]\.?[0-9]?)$')
-
     def _sort_links(self, links):
         # type: (Iterable[Link]) -> List[Link]
         """
@@ -873,101 +971,14 @@ def _package_versions(
         links,  # type: Iterable[Link]
         search  # type: Search
     ):
-        # type: (...) -> List[Optional[InstallationCandidate]]
+        # type: (...) -> List[InstallationCandidate]
         result = []
         for link in self._sort_links(links):
-            v = self._link_package_versions(link, search)
-            if v is not None:
-                result.append(v)
+            candidate = self.candidate_evaluator.evaluate_link(link, search)
+            if candidate is not None:
+                result.append(candidate)
         return result
 
-    def _log_skipped_link(self, link, reason):
-        # type: (Link, str) -> None
-        if link not in self.logged_links:
-            logger.debug('Skipping link %s; %s', link, reason)
-            self.logged_links.add(link)
-
-    def _link_package_versions(self, link, search):
-        # type: (Link, Search) -> Optional[InstallationCandidate]
-        """Return an InstallationCandidate or None"""
-        version = None
-        if link.egg_fragment:
-            egg_info = link.egg_fragment
-            ext = link.ext
-        else:
-            egg_info, ext = link.splitext()
-            if not ext:
-                self._log_skipped_link(link, 'not a file')
-                return None
-            if ext not in SUPPORTED_EXTENSIONS:
-                self._log_skipped_link(
-                    link, 'unsupported archive format: %s' % ext,
-                )
-                return None
-            if "binary" not in search.formats and ext == WHEEL_EXTENSION:
-                self._log_skipped_link(
-                    link, 'No binaries permitted for %s' % search.supplied,
-                )
-                return None
-            if "macosx10" in link.path and ext == '.zip':
-                self._log_skipped_link(link, 'macosx10 one')
-                return None
-            if ext == WHEEL_EXTENSION:
-                try:
-                    wheel = Wheel(link.filename)
-                except InvalidWheelFilename:
-                    self._log_skipped_link(link, 'invalid wheel filename')
-                    return None
-                if canonicalize_name(wheel.name) != search.canonical:
-                    self._log_skipped_link(
-                        link, 'wrong project name (not %s)' % search.supplied)
-                    return None
-
-                if not self.candidate_evaluator.is_wheel_supported(wheel):
-                    self._log_skipped_link(
-                        link, 'it is not compatible with this Python')
-                    return None
-
-                version = wheel.version
-
-        # This should be up by the search.ok_binary check, but see issue 2700.
-        if "source" not in search.formats and ext != WHEEL_EXTENSION:
-            self._log_skipped_link(
-                link, 'No sources permitted for %s' % search.supplied,
-            )
-            return None
-
-        if not version:
-            version = _egg_info_matches(egg_info, search.canonical)
-        if not version:
-            self._log_skipped_link(
-                link, 'Missing project version for %s' % search.supplied)
-            return None
-
-        match = self._py_version_re.search(version)
-        if match:
-            version = version[:match.start()]
-            py_version = match.group(1)
-            if py_version != sys.version[:3]:
-                self._log_skipped_link(
-                    link, 'Python version is incorrect')
-                return None
-        try:
-            support_this_python = check_requires_python(link.requires_python)
-        except specifiers.InvalidSpecifier:
-            logger.debug("Package %s has an invalid Requires-Python entry: %s",
-                         link.filename, link.requires_python)
-            support_this_python = True
-
-        if not support_this_python:
-            logger.debug("The package %s is incompatible with the python "
-                         "version in use. Acceptable python versions are: %s",
-                         link, link.requires_python)
-            return None
-        logger.debug('Found link %s, version: %s', link, version)
-
-        return InstallationCandidate(search.supplied, version, link)
-
 
 def _find_name_version_sep(egg_info, canonical_name):
     # type: (str, str) -> int

tests/unit/test_finder.py

@@ -460,7 +460,7 @@ def test_finder_installs_pre_releases_with_version_spec():
         assert link.url == "https://foo/bar-2.0b1.tar.gz"
 
 
-class TestLinkPackageVersions(object):
+class TestCandidateEvaluator(object):
 
     # patch this for travis which has distribute in its base env for now
     @patch(
@@ -471,11 +471,8 @@ def setup(self):
         self.version = '1.0'
         self.search_name = 'pytest'
         self.canonical_name = 'pytest'
-        self.finder = PackageFinder(
-            [],
-            [],
-            session=PipSession(),
-        )
+        valid_tags = pip._internal.pep425tags.get_supported()
+        self.evaluator = CandidateEvaluator(valid_tags=valid_tags)
 
     @pytest.mark.parametrize(
         'url',
@@ -484,15 +481,15 @@ def setup(self):
             'http:/yo/pytest-1.0-py2.py3-none-any.whl',
         ],
     )
-    def test_link_package_versions_match(self, url):
+    def test_evaluate_link__match(self, url):
         """Test that 'pytest' archives match for 'pytest'"""
         link = Link(url)
         search = Search(
             supplied=self.search_name,
             canonical=self.canonical_name,
             formats=['source', 'binary'],
         )
-        result = self.finder._link_package_versions(link, search)
+        result = self.evaluator.evaluate_link(link, search)
         expected = InstallationCandidate(self.search_name, self.version, link)
         assert result == expected, result
 
@@ -505,15 +502,15 @@ def test_link_package_versions_match(self, url):
             'http:/yo/pytest_xdist-1.0-py2.py3-none-any.whl',
         ],
     )
-    def test_link_package_versions_substring_fails(self, url):
+    def test_evaluate_link__substring_fails(self, url):
         """Test that 'pytest<something> archives won't match for 'pytest'."""
         link = Link(url)
         search = Search(
             supplied=self.search_name,
             canonical=self.canonical_name,
             formats=['source', 'binary'],
         )
-        result = self.finder._link_package_versions(link, search)
+        result = self.evaluator.evaluate_link(link, search)
         assert result is None, result