diff --git a/src/pip/_internal/cli/base_command.py b/src/pip/_internal/cli/base_command.py index 78a4e1160..37dc17188 100644 --- a/src/pip/_internal/cli/base_command.py +++ b/src/pip/_internal/cli/base_command.py @@ -347,6 +347,7 @@ class RequirementCommand(Command): return PackageFinder.create( search_scope=search_scope, + allow_yanked=True, format_control=options.format_control, trusted_hosts=options.trusted_hosts, allow_all_prereleases=options.pre, diff --git a/src/pip/_internal/commands/list.py b/src/pip/_internal/commands/list.py index 67abee0fb..a9ff88eb1 100644 --- a/src/pip/_internal/commands/list.py +++ b/src/pip/_internal/commands/list.py @@ -116,8 +116,10 @@ class ListCommand(Command): """ search_scope = make_search_scope(options) + # Pass allow_yanked=False to ignore yanked versions. return PackageFinder.create( search_scope=search_scope, + allow_yanked=False, allow_all_prereleases=options.pre, trusted_hosts=options.trusted_hosts, session=session, @@ -183,10 +185,7 @@ class ListCommand(Command): if not candidate.version.is_prerelease] evaluator = finder.candidate_evaluator - # Pass allow_yanked=False to ignore yanked versions. - best_candidate = evaluator.get_best_candidate( - all_candidates, allow_yanked=False, - ) + best_candidate = evaluator.get_best_candidate(all_candidates) if best_candidate is None: continue diff --git a/src/pip/_internal/index.py b/src/pip/_internal/index.py index 2acdd74b4..ce39a83a1 100644 --- a/src/pip/_internal/index.py +++ b/src/pip/_internal/index.py @@ -307,8 +307,13 @@ class CandidateEvaluator(object): on what tags are valid. """ + # Don't include an allow_yanked default value to make sure each call + # site considers whether yanked releases are allowed. This also causes + # that decision to be made explicit in the calling code, which helps + # people when reading the code. def __init__( self, + allow_yanked, # type: bool target_python=None, # type: Optional[TargetPython] prefer_binary=False, # type: bool allow_all_prereleases=False, # type: bool @@ -316,6 +321,8 @@ class CandidateEvaluator(object): ): # type: (...) -> None """ + :param allow_yanked: Whether files marked as yanked (in the sense + of PEP 592) are permitted to be candidates for install. :param target_python: The target Python interpreter to use to check both the Python version embedded in the filename and the package's "Requires-Python" metadata. If None (the default), then a @@ -329,6 +336,7 @@ class CandidateEvaluator(object): if ignore_requires_python is None: ignore_requires_python = False + self._allow_yanked = allow_yanked self._ignore_requires_python = ignore_requires_python self._prefer_binary = prefer_binary self._target_python = target_python @@ -357,6 +365,10 @@ class CandidateEvaluator(object): the link fails to qualify. """ version = None + if link.is_yanked and not self._allow_yanked: + reason = link.yanked_reason or '' + return (False, 'yanked for reason: {}'.format(reason)) + if link.egg_fragment: egg_info = link.egg_fragment ext = link.ext @@ -507,23 +519,14 @@ class CandidateEvaluator(object): yank_value, binary_preference, candidate.version, build_tag, pri, ) - # Don't include an allow_yanked default value to make sure each call - # site considers whether yanked releases are allowed. This also causes - # that decision to be made explicit in the calling code, which helps - # people when reading the code. def get_best_candidate( self, candidates, # type: List[InstallationCandidate] - allow_yanked, # type: bool ): # type: (...) -> Optional[InstallationCandidate] """ Return the best candidate per the instance's sort order, or None if no candidate is acceptable. - - :param allow_yanked: Whether to permit returning a yanked candidate - in the sense of PEP 592. If true, a yanked candidate will be - returned only if all candidates have been yanked. """ if not candidates: return None @@ -532,20 +535,14 @@ class CandidateEvaluator(object): # Log a warning per PEP 592 if necessary before returning. link = best_candidate.location - if not link.is_yanked: - return best_candidate - - # Otherwise, all the candidates were yanked. - if not allow_yanked: - return None - - reason = link.yanked_reason or '' - msg = ( - 'The candidate selected for download or install is a ' - 'yanked version: {candidate}\n' - 'Reason for being yanked: {reason}' - ).format(candidate=best_candidate, reason=reason) - logger.warning(msg) + if link.is_yanked: + reason = link.yanked_reason or '' + msg = ( + 'The candidate selected for download or install is a ' + 'yanked version: {candidate}\n' + 'Reason for being yanked: {reason}' + ).format(candidate=best_candidate, reason=reason) + logger.warning(msg) return best_candidate @@ -589,23 +586,13 @@ class FoundCandidates(object): # Again, converting version to str to deal with debundling. return (c for c in self.iter_all() if str(c.version) in self._versions) - # Don't include an allow_yanked default value to make sure each call - # site considers whether yanked releases are allowed. This also causes - # that decision to be made explicit in the calling code, which helps - # people when reading the code. - def get_best(self, allow_yanked): - # type: (bool) -> Optional[InstallationCandidate] + def get_best(self): + # type: () -> Optional[InstallationCandidate] """Return the best candidate available, or None if no applicable candidates are found. - - :param allow_yanked: Whether to permit returning a yanked candidate - in the sense of PEP 592. If true, a yanked candidate will be - returned only if all candidates have been yanked. """ candidates = list(self.iter_applicable()) - return self._evaluator.get_best_candidate( - candidates, allow_yanked=allow_yanked, - ) + return self._evaluator.get_best_candidate(candidates) class PackageFinder(object): @@ -648,10 +635,15 @@ class PackageFinder(object): # These are boring links that have already been logged somehow. self._logged_links = set() # type: Set[Link] + # Don't include an allow_yanked default value to make sure each call + # site considers whether yanked releases are allowed. This also causes + # that decision to be made explicit in the calling code, which helps + # people when reading the code. @classmethod def create( cls, search_scope, # type: SearchScope + allow_yanked, # type: bool allow_all_prereleases=False, # type: bool trusted_hosts=None, # type: Optional[List[str]] session=None, # type: Optional[PipSession] @@ -663,6 +655,8 @@ class PackageFinder(object): # type: (...) -> PackageFinder """Create a PackageFinder. + :param allow_yanked: Whether files marked as yanked (in the sense + of PEP 592) are permitted to be candidates for install. :param trusted_hosts: Domains not to emit warnings for when not using HTTPS. :param session: The Session to use to make requests. @@ -682,6 +676,7 @@ class PackageFinder(object): ) candidate_evaluator = CandidateEvaluator( + allow_yanked=allow_yanked, target_python=target_python, prefer_binary=prefer_binary, allow_all_prereleases=allow_all_prereleases, @@ -969,7 +964,7 @@ class PackageFinder(object): Raises DistributionNotFound or BestVersionAlreadyInstalled otherwise """ candidates = self.find_candidates(req.name, req.specifier) - best_candidate = candidates.get_best(allow_yanked=True) + best_candidate = candidates.get_best() installed_version = None # type: Optional[_BaseVersion] if req.satisfied_by is not None: diff --git a/src/pip/_internal/utils/outdated.py b/src/pip/_internal/utils/outdated.py index 8ebb54d6b..8088cda78 100644 --- a/src/pip/_internal/utils/outdated.py +++ b/src/pip/_internal/utils/outdated.py @@ -125,17 +125,16 @@ def pip_version_check(session, options): # Lets use PackageFinder to see what the latest pip version is search_scope = make_search_scope(options, suppress_no_index=True) + # Pass allow_yanked=False so we don't suggest upgrading to a + # yanked version. finder = PackageFinder.create( search_scope=search_scope, + allow_yanked=False, allow_all_prereleases=False, # Explicitly set to False trusted_hosts=options.trusted_hosts, session=session, ) - # Pass allow_yanked=False so we don't suggest upgrading to a - # yanked version. - candidate = finder.find_candidates("pip").get_best( - allow_yanked=False, - ) + candidate = finder.find_candidates("pip").get_best() if candidate is None: return pypi_version = str(candidate.version) diff --git a/tests/lib/__init__.py b/tests/lib/__init__.py index ac706b4af..0e9cd9c92 100644 --- a/tests/lib/__init__.py +++ b/tests/lib/__init__.py @@ -104,6 +104,7 @@ def make_test_finder( return PackageFinder.create( search_scope=search_scope, + allow_yanked=True, allow_all_prereleases=allow_all_prereleases, trusted_hosts=trusted_hosts, session=session, diff --git a/tests/unit/test_build_env.py b/tests/unit/test_build_env.py index 4f279ff8f..dc1bb187d 100644 --- a/tests/unit/test_build_env.py +++ b/tests/unit/test_build_env.py @@ -27,7 +27,11 @@ def run_with_build_env(script, setup_script_contents, from pip._internal.models.search_scope import SearchScope search_scope = SearchScope.create([%r], []) - finder = PackageFinder.create(search_scope, session=PipSession()) + finder = PackageFinder.create( + search_scope, + allow_yanked=True, + session=PipSession(), + ) build_env = BuildEnvironment() try: diff --git a/tests/unit/test_finder.py b/tests/unit/test_finder.py index e5c0f29d4..7a79fb51f 100644 --- a/tests/unit/test_finder.py +++ b/tests/unit/test_finder.py @@ -218,7 +218,10 @@ class TestWheel: ] target_python = TargetPython() target_python._valid_tags = valid_tags - evaluator = CandidateEvaluator(target_python=target_python) + evaluator = CandidateEvaluator( + allow_yanked=True, + target_python=target_python, + ) sort_key = evaluator._sort_key results = sorted(links, key=sort_key, reverse=True) results2 = sorted(reversed(links), key=sort_key, reverse=True) @@ -426,7 +429,7 @@ class TestCandidateEvaluator(object): def setup(self): self.search_name = 'pytest' self.canonical_name = 'pytest' - self.evaluator = CandidateEvaluator() + self.evaluator = CandidateEvaluator(allow_yanked=True) @pytest.mark.parametrize('url, expected_version', [ ('http:/yo/pytest-1.0.tar.gz', '1.0'), diff --git a/tests/unit/test_index.py b/tests/unit/test_index.py index 26bc9e914..81fc8a11d 100644 --- a/tests/unit/test_index.py +++ b/tests/unit/test_index.py @@ -90,7 +90,10 @@ class TestCandidateEvaluator: Test the target_python argument. """ target_python = TargetPython(py_version_info=(3, 7, 3)) - evaluator = CandidateEvaluator(target_python=target_python) + evaluator = CandidateEvaluator( + allow_yanked=True, + target_python=target_python, + ) # The target_python attribute should be set as is. assert evaluator._target_python is target_python @@ -98,7 +101,10 @@ class TestCandidateEvaluator: """ Test passing None for the target_python argument. """ - evaluator = CandidateEvaluator(target_python=None) + evaluator = CandidateEvaluator( + allow_yanked=True, + target_python=None, + ) # Spot-check the default TargetPython object. actual_target_python = evaluator._target_python assert actual_target_python._given_py_version_info is None @@ -118,6 +124,7 @@ class TestCandidateEvaluator: ): target_python = TargetPython(py_version_info=py_version_info) evaluator = CandidateEvaluator( + allow_yanked=True, target_python=target_python, ignore_requires_python=ignore_requires_python, ) @@ -131,6 +138,29 @@ class TestCandidateEvaluator: actual = evaluator.evaluate_link(link, search=search) assert actual == expected + @pytest.mark.parametrize('yanked_reason, allow_yanked, expected', [ + (None, True, (True, '1.12')), + (None, False, (True, '1.12')), + ('', True, (True, '1.12')), + ('', False, (False, 'yanked for reason: ')), + ('bad metadata', True, (True, '1.12')), + ('bad metadata', False, + (False, 'yanked for reason: bad metadata')), + ]) + def test_evaluate_link__allow_yanked( + self, yanked_reason, allow_yanked, expected, + ): + evaluator = CandidateEvaluator(allow_yanked=allow_yanked) + link = Link( + 'https://example.com/#egg=twine-1.12', + yanked_reason=yanked_reason, + ) + search = Search( + supplied='twine', canonical='twine', formats=['source'], + ) + actual = evaluator.evaluate_link(link, search=search) + assert actual == expected + def test_evaluate_link__incompatible_wheel(self): """ Test an incompatible wheel. @@ -138,7 +168,10 @@ class TestCandidateEvaluator: target_python = TargetPython(py_version_info=(3, 6, 4)) # Set the valid tags to an empty list to make sure nothing matches. target_python._valid_tags = [] - evaluator = CandidateEvaluator(target_python=target_python) + evaluator = CandidateEvaluator( + allow_yanked=True, + target_python=target_python, + ) link = Link('https://example.com/sample-1.0-py2.py3-none-any.whl') search = Search( supplied='sample', canonical='sample', formats=['binary'], @@ -163,7 +196,7 @@ class TestCandidateEvaluator: link = Link(url, yanked_reason=yanked_reason) candidate = InstallationCandidate('mypackage', '1.0', link) - evaluator = CandidateEvaluator() + evaluator = CandidateEvaluator(allow_yanked=True) sort_value = evaluator._sort_key(candidate) # Yanked / non-yanked is reflected in the first element of the tuple. actual = sort_value[0] @@ -176,30 +209,17 @@ class TestCandidateEvaluator: return candidate - @pytest.mark.parametrize('allow_yanked', [True, False]) - def test_get_best_candidate__no_candidates(self, allow_yanked): + def test_get_best_candidate__no_candidates(self): """ Test passing an empty list. """ - evaluator = CandidateEvaluator() - actual = evaluator.get_best_candidate([], allow_yanked=allow_yanked) + evaluator = CandidateEvaluator(allow_yanked=True) + actual = evaluator.get_best_candidate([]) assert actual is None - def test_get_best_candidate__all_yanked__allow_yanked_false(self): + def test_get_best_candidate__all_yanked(self, caplog): """ - Test all candidates yanked with allow_yanked=False. - """ - candidates = [ - self.make_mock_candidate('1.0', yanked_reason=''), - self.make_mock_candidate('2.0', yanked_reason=''), - ] - evaluator = CandidateEvaluator() - actual = evaluator.get_best_candidate(candidates, allow_yanked=False) - assert actual is None - - def test_get_best_candidate__all_yanked__allow_yanked_true(self, caplog): - """ - Test all candidates yanked with allow_yanked=True. + Test all candidates yanked. """ candidates = [ self.make_mock_candidate('1.0', yanked_reason='bad metadata #1'), @@ -208,8 +228,8 @@ class TestCandidateEvaluator: self.make_mock_candidate('2.0', yanked_reason='bad metadata #2'), ] expected_best = candidates[1] - evaluator = CandidateEvaluator() - actual = evaluator.get_best_candidate(candidates, allow_yanked=True) + evaluator = CandidateEvaluator(allow_yanked=True) + actual = evaluator.get_best_candidate(candidates) assert actual is expected_best assert str(actual.version) == '3.0' @@ -231,8 +251,8 @@ class TestCandidateEvaluator: candidates = [ self.make_mock_candidate('1.0', yanked_reason=''), ] - evaluator = CandidateEvaluator() - actual = evaluator.get_best_candidate(candidates, allow_yanked=True) + evaluator = CandidateEvaluator(allow_yanked=True) + actual = evaluator.get_best_candidate(candidates) assert str(actual.version) == '1.0' assert len(caplog.records) == 1 @@ -257,8 +277,8 @@ class TestCandidateEvaluator: self.make_mock_candidate('1.0'), ] expected_best = candidates[1] - evaluator = CandidateEvaluator() - actual = evaluator.get_best_candidate(candidates, allow_yanked=True) + evaluator = CandidateEvaluator(allow_yanked=True) + actual = evaluator.get_best_candidate(candidates) assert actual is expected_best assert str(actual.version) == '2.0' @@ -268,6 +288,20 @@ class TestCandidateEvaluator: class TestPackageFinder: + @pytest.mark.parametrize('allow_yanked', [False, True]) + def test_create__allow_yanked(self, allow_yanked): + """ + Test that allow_yanked is passed to CandidateEvaluator. + """ + search_scope = SearchScope([], []) + finder = PackageFinder.create( + search_scope=search_scope, + allow_yanked=allow_yanked, + session=object(), + ) + evaluator = finder.candidate_evaluator + assert evaluator._allow_yanked == allow_yanked + def test_create__target_python(self): """ Test that target_python is passed to CandidateEvaluator as is. @@ -276,6 +310,7 @@ class TestPackageFinder: target_python = TargetPython(py_version_info=(3, 7, 3)) finder = PackageFinder.create( search_scope=search_scope, + allow_yanked=True, session=object(), target_python=target_python, ) @@ -361,6 +396,7 @@ class TestPackageFinder: search_scope = SearchScope([], []) finder = PackageFinder.create( search_scope=search_scope, + allow_yanked=True, trusted_hosts=None, session=object(), ) diff --git a/tests/unit/test_unit_outdated.py b/tests/unit/test_unit_outdated.py index f2b343f9f..5a8eb5c1e 100644 --- a/tests/unit/test_unit_outdated.py +++ b/tests/unit/test_unit_outdated.py @@ -16,7 +16,7 @@ class MockFoundCandidates(object): def __init__(self, best): self._best = best - def get_best(self, allow_yanked): + def get_best(self): return self._best