From 58ee598b03723c9a4907e52c13a22bbf9fb06040 Mon Sep 17 00:00:00 2001 From: Chris Jerdonek Date: Mon, 22 Apr 2019 04:17:33 -0700 Subject: [PATCH 1/3] Refine return type of _package_versions() and find_all_candidates(). --- src/pip/_internal/index.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pip/_internal/index.py b/src/pip/_internal/index.py index ff614b346c0..7c6cc6bd153 100644 --- a/src/pip/_internal/index.py +++ b/src/pip/_internal/index.py @@ -668,7 +668,7 @@ 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. @@ -873,7 +873,7 @@ 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) From 651d6fe70598aee315d796c4af57fe11c5cf5162 Mon Sep 17 00:00:00 2001 From: Chris Jerdonek Date: Mon, 22 Apr 2019 04:28:44 -0700 Subject: [PATCH 2/3] Move _link_package_versions() to CandidateEvaluator. --- src/pip/_internal/index.py | 187 ++++++++++++++++++------------------- tests/unit/test_finder.py | 11 +-- 2 files changed, 97 insertions(+), 101 deletions(-) diff --git a/src/pip/_internal/index.py b/src/pip/_internal/index.py index 7c6cc6bd153..a4ab1b75fae 100644 --- a/src/pip/_internal/index.py +++ b/src/pip/_internal/index.py @@ -258,20 +258,111 @@ def _get_html_page(link, session=None): class CandidateEvaluator(object): + _py_version_re = re.compile(r'-py([123]\.?[0-9]?)$') + 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 + # 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 _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.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 """ @@ -457,9 +548,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 @@ -849,8 +937,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] """ @@ -876,98 +962,11 @@ def _package_versions( # type: (...) -> List[InstallationCandidate] result = [] for link in self._sort_links(links): - v = self._link_package_versions(link, search) + v = self.candidate_evaluator._link_package_versions(link, search) if v is not None: result.append(v) 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 diff --git a/tests/unit/test_finder.py b/tests/unit/test_finder.py index 632769c4f65..8e6038655a0 100644 --- a/tests/unit/test_finder.py +++ b/tests/unit/test_finder.py @@ -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', @@ -492,7 +489,7 @@ def test_link_package_versions_match(self, url): canonical=self.canonical_name, formats=['source', 'binary'], ) - result = self.finder._link_package_versions(link, search) + result = self.evaluator._link_package_versions(link, search) expected = InstallationCandidate(self.search_name, self.version, link) assert result == expected, result @@ -513,7 +510,7 @@ def test_link_package_versions_substring_fails(self, url): canonical=self.canonical_name, formats=['source', 'binary'], ) - result = self.finder._link_package_versions(link, search) + result = self.evaluator._link_package_versions(link, search) assert result is None, result From 10c68e674b689673126475734dbed5e43e0177ad Mon Sep 17 00:00:00 2001 From: Chris Jerdonek Date: Mon, 22 Apr 2019 19:23:36 -0700 Subject: [PATCH 3/3] Rename _link_package_versions() to evaluate_link(). --- src/pip/_internal/index.py | 32 ++++++++++++++++++++++---------- tests/unit/test_finder.py | 10 +++++----- 2 files changed, 27 insertions(+), 15 deletions(-) diff --git a/src/pip/_internal/index.py b/src/pip/_internal/index.py index a4ab1b75fae..746d5e7cd10 100644 --- a/src/pip/_internal/index.py +++ b/src/pip/_internal/index.py @@ -258,7 +258,10 @@ def _get_html_page(link, session=None): class CandidateEvaluator(object): - _py_version_re = re.compile(r'-py([123]\.?[0-9]?)$') + """ + Responsible for filtering and sorting candidates for installation based + on what tags are valid. + """ def __init__( self, @@ -269,6 +272,11 @@ def __init__( self._prefer_binary = prefer_binary self._valid_tags = valid_tags + # 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] @@ -278,13 +286,17 @@ def _log_skipped_link(self, link, reason): logger.debug('Skipping link %s; %s', link, reason) self._logged_links.add(link) - def is_wheel_supported(self, wheel): + def _is_wheel_supported(self, wheel): # type: (Wheel) -> bool return wheel.supported(self._valid_tags) - def _link_package_versions(self, link, search): + def evaluate_link(self, link, search): # type: (Link, Search) -> Optional[InstallationCandidate] - """Return an InstallationCandidate or None""" + """ + 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 @@ -318,7 +330,7 @@ def _link_package_versions(self, link, search): link, 'wrong project name (not %s)' % search.supplied) return None - if not self.is_wheel_supported(wheel): + if not self._is_wheel_supported(wheel): self._log_skipped_link( link, 'it is not compatible with this Python') return None @@ -384,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 @@ -762,7 +774,7 @@ def find_all_candidates(self, 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) @@ -962,9 +974,9 @@ def _package_versions( # type: (...) -> List[InstallationCandidate] result = [] for link in self._sort_links(links): - v = self.candidate_evaluator._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 diff --git a/tests/unit/test_finder.py b/tests/unit/test_finder.py index 8e6038655a0..68e985fd79c 100644 --- a/tests/unit/test_finder.py +++ b/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( @@ -481,7 +481,7 @@ 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( @@ -489,7 +489,7 @@ def test_link_package_versions_match(self, url): canonical=self.canonical_name, formats=['source', 'binary'], ) - result = self.evaluator._link_package_versions(link, search) + result = self.evaluator.evaluate_link(link, search) expected = InstallationCandidate(self.search_name, self.version, link) assert result == expected, result @@ -502,7 +502,7 @@ 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 archives won't match for 'pytest'.""" link = Link(url) search = Search( @@ -510,7 +510,7 @@ def test_link_package_versions_substring_fails(self, url): canonical=self.canonical_name, formats=['source', 'binary'], ) - result = self.evaluator._link_package_versions(link, search) + result = self.evaluator.evaluate_link(link, search) assert result is None, result