Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fall back to distributions without hashes in resolver #2949

Merged
merged 1 commit into from
Apr 10, 2024

Conversation

charliermarsh
Copy link
Member

@charliermarsh charliermarsh commented Apr 9, 2024

Summary

This represents a change to --require-hashes in the event that we don't find a matching hash from the registry. The behavior in this PR is closer to pip's.

Prior to this PR, if a distribution had no reported hash, or only mismatched hashes, we would mark it as incompatible. Now, we mark it as compatible, but we use the hash-agreement as part of the ordering, such that we prefer any distribution with a matching hash, then any distribution with no hash, then any distribution with a mismatched hash.

As a result, if an index reports incorrect hashes, but the user provides the correct one, resolution now succeeds, where it would've failed.

Similarly, if an index omits hashes altogether, but the user provides the correct one, resolution now succeeds, where it would've failed.

If we end up picking a distribution whose hash ultimately doesn't match, we'll reject it later, after resolution.

@charliermarsh charliermarsh added the enhancement New feature or request label Apr 9, 2024
@charliermarsh charliermarsh marked this pull request as ready for review April 9, 2024 23:12
sha256:123

Computed:
sha256:5d69f0b590514103234f0c3526563856f04d044d8d0ea1073a843ae429b3187e
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These aren't as nice-looking, but keep in mind that for --require-hashes, the user has to pin all dependencies upfront anyway. There's no backtracking or anything.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They're pretty ugly but 🤷‍♂️ we could address later if we want.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolutely DESTROYING my error messages

Copy link
Member

@zanieb zanieb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this UX seems much better.

@charliermarsh charliermarsh force-pushed the charlie/check-hashes-iii branch 2 times, most recently from 669384d to e3f5242 Compare April 10, 2024 18:55
Base automatically changed from charlie/check-hashes-iii to main April 10, 2024 19:09
@charliermarsh charliermarsh enabled auto-merge (squash) April 10, 2024 19:14
@charliermarsh charliermarsh merged commit c18551f into main Apr 10, 2024
37 checks passed
@charliermarsh charliermarsh deleted the charlie/check-hashes-v branch April 10, 2024 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants