Filter out yanked links earlier when allow_yanked=False.

This commit is contained in:
Chris Jerdonek 2019-06-27 11:50:27 -07:00
parent 13d3de227d
commit 8af3f21a57
9 changed files with 116 additions and 78 deletions

View File

@ -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,

View File

@ -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

View File

@ -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 '<none given>'
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 '<none given>'
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 '<none given>'
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:

View File

@ -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)

View File

@ -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,

View File

@ -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:

View File

@ -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'),

View File

@ -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: <none given>')),
('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(),
)

View File

@ -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