-
-
Notifications
You must be signed in to change notification settings - Fork 14.8k
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
treewide: sha512 → hash #255512
treewide: sha512 → hash #255512
Conversation
Perfect issue number for this <3 |
3742392
to
4c5f69a
Compare
PS: Rebased to deal with two minor oopsies in the history:
|
4c5f69a
to
bbd4ae8
Compare
For reference, lots of discussion around this happened at NixOS/rfcs#131 |
Thanks for the pointer! It looks like the RFC was about automatically enforcing the use of Overall, what motivated me to try and migrate most of nixpkgs over, is that I noticed some nix users seem to be confused by the |
That sounds like it stems from a split in the community where to move on this topic (whether to prefer SRI and how strongly), which seems very much like that RFC. |
From what you say I'd think it would be better to document them. Even if they would be considered deprecated for new packages or something, in which case you just express that in those docs (though I'm not aware of such a decision so far). |
Presumably, they were previously documented and were eventually removed. IDK which PR removed it, and for what reason, so I'm certainly not going to blindly revert that.
IDK how normative that is, but Eelco Dolstra was already stating in 2020 that “nowadays it's better to use SRI hashes, which is what the new CLI defaults to.” In general, I agree it makes sense to have a hash format that is preferred and tooling defaults to; especially if that format is an actual standard, so we interop with other software. |
BTW, going by what's in nixpkgs, there doesn't seem to be that much of a split on how to represent sha512 hashes:
You can reproduce that with set -A formats "hex" "[0-9A-Fa-f]{128}" "nix32" "[0-9a-fg-np-z]{103}" "base64" "(sha512-)?[A-Za-z0-9+/]{86}={2}"
for format regex in "${(@kv)formats}"; do
echo -n "$format: "
rg -c -g "*.nix" "['\"]${regex}['\"]" | cut -d: -f2 | paste -sd+ | bc
done |
bbd4ae8
to
f278def
Compare
Rebased to address merge conflict. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From the point of view of OCaml we have already been moving this way so this is fine. I have not reviewed the script(s).
Add support for `sha512`, refactor to easily add hash functions in the future. Also, skip autogenerated files.
f278def
to
fab52fc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know about pretty printers in Python.
Besides, LGTM
Description of changes
Improve
sha-to-hash.py
toReplace
sha512
withhash
through nixpkgs, where appropriate.This does not include autogenerated files; the tools making those should be updated & rerun.
Things done
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage