Enable usage of supported checksum algorithms in RECORDS (fixes #13198)
Description
In #13198 we identified that an issue still remains in conda's handling of PEP 376 where checksum algorithms shown as uppercase result in an AssertionError. This PR fixes the existing logic to handle all supported algorithms per that PEP in RECORD files. The latest packaging guidelines provide that this is everything in hashlib.algorithms_guaranteed except sha1 and md5.
One caveat-- while the PEP itself does not specify capitalization, it only specifies that the algorithm should be one of hashlib.algorithms_guaranteed-- those are all lowercase. In practice, however, some widely used packages seem to use uppercase. Conda probably shouldn't break in this case even if the algorithm name is technically non-conformant.
This PR also improves error messages significantly if a non-conforming package is encountered.
Checklist - did you ...
- [x] Add a file to the
newsdirectory (using the template) for the next release's release notes? - [x] Add / update necessary tests?
- [ ] Add / update outdated documentation?
We require contributors to sign our Contributor License Agreement and we don't have one on file for @iamthebot.
In order for us to review and merge your code, please e-sign the Contributor License Agreement PDF. We then need to manually verify your signature, merge the PR (https://github.com/conda/infrastructure/pull/835), and ping the bot to refresh the PR.
FYI I just signed the CLA but the CLA PR needs to be merged by a maintainer.
Updated this PR to reflect the latest python packaging guidelines where all algorithms in hashlib.algorithms_guaranteed are supported except sha1 and md5.
@conda-bot check
@iamthebot,
Thanks a lot for this pull request. I already left a couple minor suggestions for improvements (we prefer using f-strings for this project).
I am wondering if we could ditch the use of assertions. When these fail, they result in ugly trac eback errors like the one you initially encountered. Instead, you could raise any error which inherits from conda.exceptions.CondaError and a much nicer error message will be shown.
Let me know what you think.
@travishathaway went ahead and applied your PR suggestions. Also expanded on the relevant unit test a bit so we actually check the possible algorithms (this is very quick so shouldn't impact CI time too much). I also went ahead and removed unused code (_check_path_data is not referenced anywhere).
I didn't change the other assertions in this file just to keep the PR tightly scoped but we can do that in a followup PR.
Let me know what you think.
@iamthebot,
Thanks for the clarification. I'm okay with the letting the assert statements stay in for now. Please see my comments regarding the failing test though.
Thanks @travishathaway I've been a bit busy this week, will try to get to this tomorrow.
Hi there, thank you for your contribution!
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed automatically if no further activity occurs.
If you would like this pull request to remain open please:
- Rebase and verify the changes still work
- Leave a comment with the current status
NOTE: If this pull request was closed prematurely, please leave a comment.
Thanks!