From 74504fff6c688b2792c0f5748926ab4a30cd870f Mon Sep 17 00:00:00 2001 From: Chris Jerdonek Date: Thu, 11 Jul 2019 01:13:17 -0700 Subject: [PATCH] Prefer candidates with allowed hashes when sorting. --- news/5874.feature | 2 ++ src/pip/_internal/index.py | 16 ++++++++++++---- tests/unit/test_index.py | 32 ++++++++++++++++++++++++++------ 3 files changed, 40 insertions(+), 10 deletions(-) create mode 100644 news/5874.feature diff --git a/news/5874.feature b/news/5874.feature new file mode 100644 index 000000000..844e3790f --- /dev/null +++ b/news/5874.feature @@ -0,0 +1,2 @@ +When choosing candidates to install, prefer candidates with a hash matching +one of the user-provided hashes. diff --git a/src/pip/_internal/index.py b/src/pip/_internal/index.py index 267277f0b..ad634e1c2 100644 --- a/src/pip/_internal/index.py +++ b/src/pip/_internal/index.py @@ -54,7 +54,7 @@ if MYPY_CHECK_RUNNING: BuildTag = Tuple[Any, ...] # either empty tuple or Tuple[int, str] CandidateSortingKey = ( - Tuple[int, int, _BaseVersion, BuildTag, Optional[int]] + Tuple[int, int, int, _BaseVersion, BuildTag, Optional[int]] ) HTMLElement = xml.etree.ElementTree.Element SecureOrigin = Tuple[str, str, Optional[str]] @@ -624,8 +624,14 @@ class CandidateEvaluator(object): The preference is as follows: - First and foremost, yanked candidates (in the sense of PEP 592) are - always less preferred than candidates that haven't been yanked. Then: + First and foremost, candidates with allowed (matching) hashes are + always preferred over candidates without matching hashes. This is + because e.g. if the only candidate with an allowed hash is yanked, + we still want to use that candidate. + + Second, excepting hash considerations, candidates that have been + yanked (in the sense of PEP 592) are always less preferred than + candidates that haven't been yanked. Then: If not finding wheels, they are sorted by version only. If finding wheels, then the sort order is by version, then: @@ -660,9 +666,11 @@ class CandidateEvaluator(object): build_tag = (int(build_tag_groups[0]), build_tag_groups[1]) else: # sdist pri = -(support_num) + has_allowed_hash = int(link.is_hash_allowed(self._hashes)) yank_value = -1 * int(link.is_yanked) # -1 for yanked. return ( - yank_value, binary_preference, candidate.version, build_tag, pri, + has_allowed_hash, yank_value, binary_preference, candidate.version, + build_tag, pri, ) def get_best_candidate( diff --git a/tests/unit/test_index.py b/tests/unit/test_index.py index 82bebdd40..a8a63396c 100644 --- a/tests/unit/test_index.py +++ b/tests/unit/test_index.py @@ -302,6 +302,29 @@ class TestCandidateEvaluator: ] assert found_candidates._applicable_candidates == expected_applicable + @pytest.mark.parametrize('hex_digest, expected', [ + # Test a link with no hash. + (None, 0), + # Test a link with an allowed hash. + (64 * 'a', 1), + # Test a link with a hash that isn't allowed. + (64 * 'b', 0), + ]) + def test_sort_key__hash(self, hex_digest, expected): + """ + Test the effect of the link's hash on _sort_key()'s return value. + """ + candidate = make_mock_candidate('1.0', hex_digest=hex_digest) + hashes_data = { + 'sha256': [64 * 'a'], + } + hashes = Hashes(hashes_data) + evaluator = CandidateEvaluator.create(hashes=hashes) + sort_value = evaluator._sort_key(candidate) + # The hash is reflected in the first element of the tuple. + actual = sort_value[0] + assert actual == expected + @pytest.mark.parametrize('yanked_reason, expected', [ # Test a non-yanked file. (None, 0), @@ -312,14 +335,11 @@ class TestCandidateEvaluator: """ Test the effect of is_yanked on _sort_key()'s return value. """ - url = 'https://example.com/mypackage.tar.gz' - link = Link(url, yanked_reason=yanked_reason) - candidate = InstallationCandidate('mypackage', '1.0', link) - + candidate = make_mock_candidate('1.0', yanked_reason=yanked_reason) evaluator = CandidateEvaluator.create() sort_value = evaluator._sort_key(candidate) - # Yanked / non-yanked is reflected in the first element of the tuple. - actual = sort_value[0] + # Yanked / non-yanked is reflected in the second element of the tuple. + actual = sort_value[1] assert actual == expected def test_get_best_candidate__no_candidates(self):