Skip to content

Support SHA256 hashes in @EXPLICIT files #3866

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

Merged
merged 9 commits into from
Mar 21, 2025

Conversation

jaimergp
Copy link
Contributor

@jjerphan jjerphan added the release::bug_fixes For PRs fixing bugs label Mar 19, 2025
@Hind-M
Copy link
Member

Hind-M commented Mar 20, 2025

Thanks!

Maybe also add some tests in test_package_info.cpp:

  • Always check pkg.md5 and pkg.sha256 values
  • Add a section for a hash.size=64 (sha256 case)

Should we support sha256: in urls in this PR as well? (it can be done later otherwise since it's not mandatory).

@jaimergp
Copy link
Contributor Author

Should we support sha256: in urls in this PR as well? (it can be done later otherwise since it's not mandatory).

I'll try my best 😬

Copy link
Member

@jjerphan jjerphan left a comment

Choose a reason for hiding this comment

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

Thank you for this contribution, @jaimergp.

This LGTM modulo one discussion.

Copy link
Member

Choose a reason for hiding this comment

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

I think we might want to do something in else cases.

How about raising a warning when hashes are malformed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that sounds sensible. I didn't add them to respect the existing logic (silent ignore). I'm not sure how to add warnings in this codebase, so feel free to commit to the branch directly :)

Copy link
Member

Choose a reason for hiding this comment

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

You are right, and thinking again I am not sure we want to add warnings to spam users if this does not cause fatal behavior and if they cannot do anything about it.

@jjerphan jjerphan merged commit b3a48ce into mamba-org:main Mar 21, 2025
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release::bug_fixes For PRs fixing bugs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants