Skip to content

Commit

Permalink
Prefer candidates with allowed hashes when sorting.
Browse files Browse the repository at this point in the history
  • Loading branch information
cjerdonek committed Jul 14, 2019
1 parent 9ddc89a commit 74504ff
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 10 deletions.
2 changes: 2 additions & 0 deletions news/5874.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
When choosing candidates to install, prefer candidates with a hash matching
one of the user-provided hashes.
16 changes: 12 additions & 4 deletions src/pip/_internal/index.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@

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]]
Expand Down Expand Up @@ -624,8 +624,14 @@ def _sort_key(self, candidate):
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:
Expand Down Expand Up @@ -660,9 +666,11 @@ def _sort_key(self, candidate):
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(
Expand Down
32 changes: 26 additions & 6 deletions tests/unit/test_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,29 @@ def test_make_found_candidates(self):
]
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),
Expand All @@ -312,14 +335,11 @@ def test_sort_key__is_yanked(self, yanked_reason, expected):
"""
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):
Expand Down

0 comments on commit 74504ff

Please sign in to comment.