Skip to content
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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 3 commits into from
Mar 15, 2023
Merged

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

merged 3 commits into from
Mar 15, 2023

Conversation

ShamrockLee
Copy link
Contributor

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

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

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

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

  • @roberth: Needs tests and release notes

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

Done.

@ShamrockLee
Copy link
Contributor Author

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
tests/hash.sh Outdated Show resolved Hide resolved
@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
Copy link
Member

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 fricklerhandwerk is a code owner

@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