Skip to content

nix-hash: support base-64 and SRI format#7690

Merged
Ericson2314 merged 3 commits intoNixOS:masterfrom
ShamrockLee:nix-hash-sri
Mar 15, 2023
Merged

nix-hash: support base-64 and SRI format#7690
Ericson2314 merged 3 commits intoNixOS:masterfrom
ShamrockLee:nix-hash-sri

Conversation

@ShamrockLee
Copy link
Contributor

@ShamrockLee ShamrockLee commented Jan 25, 2023

Motivation

This PR provides nix-hash with base-64 and SRI format support, equivalent to that of nix hash.

If applied, users with a nix-channel-based setup would be able to compute hashes in base-64 and SRI format without having to type nix --extra-experimental-features hash ...

$ nix-hash --type sha256 --base64 /nix/store/g2m8kfw7kpgpph05v2fxcx4d5an09hl3-hello-2.12.1
p7JaKoco7c2MM+1zHW1B5ebUUnt9rGVc3GaTsu1QYfE=

$ nix-hash --type sha256 --sri /nix/store/g2m8kfw7kpgpph05v2fxcx4d5an09hl3-hello-2.12.1
sha256-p7JaKoco7c2MM+1zHW1B5ebUUnt9rGVc3GaTsu1QYfE=

$ nix-hash --type sha1 --to-base64 e4fd8ba5f7bbeaea5ace89fe10255536cd60dab6
5P2Lpfe76upazon+ECVVNs1g2rY=

$ nix-hash --type sha1 --to-sri nvd61k9nalji1zl9rrdfmsmvyyjqpzg4
sha1-5P2Lpfe76upazon+ECVVNs1g2rY=

$ nix-hash --to-base16 sha1-5P2Lpfe76upazon+ECVVNs1g2rY=
e4fd8ba5f7bbeaea5ace89fe10255536cd60dab6

Context

Solves #7688

Checklist for maintainers

Maintainers: tick if completed or explain if not relevant

  • agreed on idea
  • agreed on implementation strategy
  • tests, as appropriate
    • functional tests - tests/**.sh
    • unit tests - src/*/tests
    • integration tests
  • documentation in the manual
  • code and comments are self-explanatory
  • commit message explains why the change was made
  • new feature or bug fix: updated release notes

@ShamrockLee ShamrockLee marked this pull request as ready for review January 25, 2023 15:18
@ShamrockLee ShamrockLee mentioned this pull request Jan 25, 2023
Closed
@edolstra
Copy link
Member

edolstra commented Jan 26, 2023

I don't think we should be extending nix-hash and the old CLI in general. nix hash works fine.

@vcunat
Copy link
Member

vcunat commented Jan 26, 2023

I think we're being a bit schizophrenic in (not) recommending the new nix CLI. It's hard to use by default (--extra-experimental-features nix-command) and we still say that it can change incompatibly.

For example what would you use in a script? A script-local alias that adds the extra enabler and additionally hope that you won't be hit by future incompatibility?

@fricklerhandwerk fricklerhandwerk added the cli The old and/or new command line interface label Feb 7, 2023
@nixos-discourse
Copy link

nixos-discourse commented Feb 20, 2023

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2023-02-17-nix-team-meeting-minutes-33/25624/1

@thufschmitt thufschmitt self-assigned this Mar 10, 2023
@Ericson2314 Ericson2314 added the idea approved The given proposal has been discussed and approved by the Nix team. An implementation is welcome. label Mar 10, 2023
@Ericson2314
Copy link
Member

Ericson2314 commented Mar 10, 2023 •
edited
Loading

Discussed in the Nix team meeting:

@nixos-discourse
Copy link

nixos-discourse commented Mar 13, 2023

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2023-03-10-nix-team-meeting-minutes-39/26279/1

@github-actions github-actions bot added the with-tests Issues related to testing. PRs with tests have some priority label Mar 13, 2023
@ShamrockLee
Copy link
Contributor Author

ShamrockLee commented Mar 13, 2023

  • @roberth: Needs tests and release notes

  • @thufschmitt: Let's ask author to do those. If they do those

Done.

@ShamrockLee
Copy link
Contributor Author

ShamrockLee commented Mar 15, 2023

Oops! I didn't read the hacking guide carefully enough.

Now the test should pass.

@ShamrockLee
tests/hash.sh: try: Use FORMAT_FLAG instead of EXTRA
2f32303 
Do not rely on the "multiple format flag specified" behavior.

Explicitly test without the format flag / with the --base16 flag.
@ShamrockLee
test/hash.sh: add to-base32 test for nix hash
08510c5
Ericson2314
Ericson2314 reviewed Mar 15, 2023
View reviewed changes
Ericson2314
Ericson2314 reviewed Mar 15, 2023
View reviewed changes
@ShamrockLee
nix-hash: support base-64 and SRI format
c27d358 
Add the --base64 and --sri flags for the Base64 and SRI format output.

Add the --base16 flag to explicitly specify the hexadecimal format.

Add the --to-base64 and --to-sri flag to convert a hash to the above
mentioned format.
@Ericson2314 Ericson2314 enabled auto-merge March 15, 2023 19:30
@Ericson2314
Copy link
Member

Ericson2314 commented Mar 15, 2023

Thanks @ShamrockLee. This is nicely done.

@Ericson2314 Ericson2314 merged commit 0a140a9 into NixOS:master Mar 15, 2023
@ShamrockLee ShamrockLee deleted the nix-hash-sri branch March 15, 2023 21:34
@drupol drupol mentioned this pull request Jun 7, 2023
Merged
12 tasks
@vcunat vcunat mentioned this pull request Sep 19, 2023
Draft
@fricklerhandwerk fricklerhandwerk added the feature Feature request or proposal label Jan 8, 2024
This was referenced Jun 30, 2024
Merged
Closed
@philipp-baumann philipp-baumann mentioned this pull request Jul 8, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Ericson2314 Ericson2314 Ericson2314 left review comments

@fricklerhandwerk fricklerhandwerk Awaiting requested review from fricklerhandwerk

@edolstra edolstra Awaiting requested review from edolstra edolstra is a code owner

@thufschmitt thufschmitt Awaiting requested review from thufschmitt

Assignees

@thufschmitt thufschmitt

Labels

cli The old and/or new command line interface feature Feature request or proposal idea approved The given proposal has been discussed and approved by the Nix team. An implementation is welcome. with-tests Issues related to testing. PRs with tests have some priority

Projects

Nix team
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@ShamrockLee @edolstra @vcunat @nixos-discourse @Ericson2314 @fricklerhandwerk @thufschmitt