Skip to content

treewide: sha512 → hash#255512

Merged
infinisil merged 3 commits intoNixOS:masterfrom
nicoonoclaste:sha512-to-hash
Sep 23, 2023
Merged

treewide: sha512 → hash#255512
infinisil merged 3 commits intoNixOS:masterfrom
nicoonoclaste:sha512-to-hash

Conversation

@nicoonoclaste
Copy link
Copy Markdown
Contributor

Description of changes

  • Improve sha-to-hash.py to

    • handle multiple hash functions (currently SHA-256 and SHA-512),
    • automatically skip autogenerated files, based on heuristics.
  • Replace sha512 with hash through nixpkgs, where appropriate.
    This does not include autogenerated files; the tools making those should be updated & rerun.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Fits CONTRIBUTING.md.

@github-actions github-actions bot added 6.topic: ocaml OCaml is a general-purpose, high-level, multi-paradigm programming language. 6.topic: nodejs Node.js is a free, open-source, cross-platform JavaScript runtime environment labels Sep 16, 2023
@nicoonoclaste
Copy link
Copy Markdown
Contributor Author

Perfect issue number for this <3

@nicoonoclaste nicoonoclaste force-pushed the sha512-to-hash branch 2 times, most recently from 3742392 to 4c5f69a Compare September 16, 2023 18:04
@nicoonoclaste
Copy link
Copy Markdown
Contributor Author

PS: Rebased to deal with two minor oopsies in the history:

@vcunat
Copy link
Copy Markdown
Member

vcunat commented Sep 16, 2023

For reference, lots of discussion around this happened at NixOS/rfcs#131

@nicoonoclaste
Copy link
Copy Markdown
Contributor Author

nicoonoclaste commented Sep 16, 2023

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 hash, though?
This PR only affects what little manually-written Nix code uses sha512 (the diff only lists 38 changed hashes).
In particular, I'm very much not trying to move autogenerated files over, or even (yet) list all nix-generating tools that would need updating.

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 sha256 and sha512 parameters, most likely because they were removed from the fetchers' docs (in favor of hash) but are still in widespread use.

@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Sep 16, 2023
@vcunat
Copy link
Copy Markdown
Member

vcunat commented Sep 16, 2023

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.

@vcunat
Copy link
Copy Markdown
Member

vcunat commented Sep 16, 2023

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).

@nicoonoclaste
Copy link
Copy Markdown
Contributor Author

From what you say I'd think it would be better to document them.

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.

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).

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.

@nicoonoclaste
Copy link
Copy Markdown
Contributor Author

nicoonoclaste commented Sep 16, 2023

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.

BTW, going by what's in nixpkgs, there doesn't seem to be that much of a split on how to represent sha512 hashes:

  • 3 are in “nix32” format;
  • 167 are hex-encoded... plus 10188 more in texlive/tlpdb.nix (which seems auto-generated)
  • 13660 are in base64.

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

@nicoonoclaste
Copy link
Copy Markdown
Contributor Author

Rebased to address merge conflict.

Copy link
Copy Markdown
Member

@ulrikstrid ulrikstrid left a 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).

@delroth delroth added the 12.approvals: 1 This PR was reviewed and approved by one person. label Sep 20, 2023
Add support for `sha512`, refactor to easily add hash functions in the future.

Also, skip autogenerated files.
Copy link
Copy Markdown
Member

@AndersonTorres AndersonTorres left a 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

@infinisil infinisil merged commit 390a424 into NixOS:master Sep 23, 2023
@nicoonoclaste nicoonoclaste deleted the sha512-to-hash branch October 17, 2023 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: nodejs Node.js is a free, open-source, cross-platform JavaScript runtime environment 6.topic: ocaml OCaml is a general-purpose, high-level, multi-paradigm programming language. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants