primops: add builtins.convertHash #7708

ShamrockLee · 2023-01-29T07:16:41Z

Motivation

This PR adds a builtin function builtins.convertHash that converts a hash string to another format.

Context

Examples:

# This returns "sha256-47DEQpj8HBSa+/TImW+5JCeuQeRkm5NMpJWZG3hSuFU="
builtins.convertHash {
  hash = "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855";
  toHashFormat = "sri";
  hashAlgo = "sha256";
}

# This returns "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"
builtins.convertHash {
  hash = "sha256-47DEQpj8HBSa+/TImW+5JCeuQeRkm5NMpJWZG3hSuFU=";
  toHashFormat = "base16";
}

# This returns "sha256-47DEQpj8HBSa+/TImW+5JCeuQeRkm5NMpJWZG3hSuFU="
builtins.convertHash {
  hash = "sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855";
  toHashFormat = "sri";
}

Type:

builtins.convertHash :: { hash :: String, hashAlgo :: String, toHashFormat :: String } -> String
builtins.convertHash :: { hash :: String, toHashFormat :: String } -> String

To implement builtins.convertHash, the hash algorithm name (if specified) is parsed with parseHashType, and the hash format name with parseHashFormat. The original hash and the algorithm then feed to Hash::parseAny, and produce the result through its method to_string with the parsed hash format taken.

The attribute names are inspired by the outputHash, and outputHashAlgo used to specify FOD, and nix-hash --to-base*|--to-sri used to convert hash formats in the command line.

This PR also renames tree-wide from "hash base" to "hash format" to avoid confusion. This includes:

Rename enum Base to enum HashFormat.
Rename members of classes.
- From CmdHashFormat::base to CmdHashFormat::hashFormat
- From CmdToBase::base to CmdToBase::hashFormat
Rename temporary variables from hashBase or base to hashFormat.

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 - tests/nixos/*
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

edolstra · 2023-01-30T11:13:59Z

This should probably be convertHash, since it's not very obvious what it means to "rebase" a hash.

"base" should probably be "encoding", since "sri" is not a base.

The type argument should not be mandatory, because SRI hashes already contain the type of the hash.

ShamrockLee · 2023-01-30T12:12:41Z

Thanks for reviewing! convertHash sounds neet!

The type argument should not be mandatory, because SRI hashes already contain the type of the hash.

Regarding this, we could define convertSriHash that has the effect of convertHash type but without the type argument.

This could be done as a builtin (using either parseSRI or parseAnyPrefix in libutil/hash.cc) or inside Nixpkgs lib/strings.nix (by lib.splitString). Which one is preferred?

BTW, we would need to support colon-seperated hash prefix also, since "sha256:0000000000000000000000000000000000000000000000000000" is used by nix-prefetch.

ShamrockLee · 2023-01-30T16:22:28Z

Just rename to convertHash, and add a new builtin convertSRIHash.

aameen-tulip · 2023-02-17T00:55:05Z

You're my hero.

ShamrockLee · 2023-03-16T00:05:31Z

Bump onto master
Add release note
Add tests

ShamrockLee · 2023-03-16T00:06:38Z

Do you guys think I should rename convertSRIHash to convertSriHash or not?

roberth · 2023-06-09T11:19:10Z

builtins.convertHash :: String -> String -> String -> String
builtins.convertSRIHash :: String -> String -> String

I think this should be changed to a single function where you can specify the output format, and perhaps optionally the input format. It should take an attribute set instead of positional arguments, to make clear which string is what.

nixos-discourse · 2023-06-16T11:42:49Z

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

https://discourse.nixos.org/t/2023-06-09-nix-team-meeting-minutes-61/29163/1

nixos-discourse · 2023-09-05T12:52:57Z

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

https://discourse.nixos.org/t/string-to-base-64/32624/2

ShamrockLee · 2023-09-05T16:10:47Z

Sorry for late reply. I need some time to mourn for the catastrophic data loss on my laptop (~300 GiB disappeared after reboot) and restore the environment.

It should take an attribute set instead of positional arguments, to make clear which string is what.

What is the proper way to read in an attribute set for a built-in function? There seems to be a variety of implementation inside primops.cc.

fricklerhandwerk

Hey @ShamrockLee, I'll be available for getting this merged. I suggest to start with examples and usage notes for the docs, so we're sure the interface is smooth. Implementation is already largely there and should be straightforward.

src/libexpr/primops.cc

doc/manual/src/release-notes/rl-next.md

fricklerhandwerk · 2023-10-06T19:27:57Z

@ShamrockLee do you still have time to pursue this further? I'm really sorry that PRs here have such long turnaround times. We're working on improving that, step by step.

ShamrockLee · 2023-10-09T06:55:20Z

Another force push to clean up temporary files accidentally committed or remainings across rebase.

fricklerhandwerk

Ran the tests and played with it, it works. A few superficial items on documentation/presentation.

src/libexpr/primops.cc

src/libutil/hash.hh

src/libexpr/primops.cc

src/libutil/hash.hh

Ericson2314 · 2023-10-11T17:04:59Z

src/libutil/hash.hh

+/**
+ * Parse a string representing a hash format.
+ */
+HashFormat parseHashFormat(std::string_view hashFormatName);
+
+/**
+ * std::optional version of parseHashFormat that doesn't throw error.
+ */
+std::optional<HashFormat> parseHashFormatOpt(std::string_view hashFormatName);
+
+/**
+ * The reverse of parseHashFormat.
+ */
+std::string_view printHashFormat(HashFormat hashFormat);
+


These should have unit tests that they round-trip. Ideally also a rapidcheck property test too.

Round-trip test added.

I'm not familiar with rapidcheck property check. Are there examples of such tests? I'm still trying to figure out what https://github.com/emil-e/rapidcheck/blob/master/doc/properties.md would like to explain.

There are examples of such tests, look at the store path tests.

src/libexpr/primops.cc

Remove redundant "else" after "return". Use std::nullopt to increase readability.

List the allowed hash formats

hashBase is ambiguous, since it's not about the digital bases, but about the format of hashes. Base16, Base32 and Base64 are all character maps for binary encoding. Rename the enum Base to HashFormat. Rename variables of type HashFormat from [hash]Base to hashFormat, including CmdHashBase::hashFormat and CmdToBase::hashFormat.

Base* -> HashFormat::Base*

Add hash format analogy of parseHashTypeOpt, parseHashType, and printHashType. Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>

Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>

ShamrockLee · 2023-10-18T17:00:51Z

(Rebase to resolve merge conflict.)

fricklerhandwerk · 2023-10-19T09:38:41Z

@ShamrockLee thank you very much for pulling through with this!

ShamrockLee marked this pull request as ready for review January 29, 2023 14:18

ShamrockLee requested review from edolstra and thufschmitt as code owners January 29, 2023 14:18

ShamrockLee force-pushed the primop-rebasehash branch from 3cf1f49 to e29457c Compare January 30, 2023 16:10

ShamrockLee changed the title ~~primops: add builtins.rebaseHash~~ primops: add builtins.convertHash and builtins.convertSRIHash Jan 30, 2023

nlewo mentioned this pull request Feb 26, 2023

Update add-sri.py script to work with current nixpkgs and Nix nix-community/nixpkgs-swh#7

Merged

ShamrockLee force-pushed the primop-rebasehash branch from e29457c to b81b954 Compare March 16, 2023 00:02

ShamrockLee requested a review from fricklerhandwerk as a code owner March 16, 2023 00:02

github-actions bot added the with-tests Issues related to testing. PRs with tests have some priority label Mar 16, 2023

fricklerhandwerk added feature Feature request or proposal language The Nix expression language; parser, interpreter, primops, evaluation, etc labels Mar 17, 2023

ShamrockLee force-pushed the primop-rebasehash branch 2 times, most recently from 8324e57 to 3114296 Compare May 25, 2023 15:19

thufschmitt assigned fricklerhandwerk Jun 9, 2023

thufschmitt added the idea approved The given proposal has been discussed and approved by the Nix team. An implementation is welcome. label Jun 9, 2023

ShamrockLee mentioned this pull request Aug 2, 2023

[WIP] vscode-extensions: support multiple extension registries NixOS/nixpkgs#121583

Draft

10 tasks

roberth mentioned this pull request Sep 5, 2023

Stabilise nix hash and subcommands #8876

Closed

2 tasks

fricklerhandwerk reviewed Sep 6, 2023

View reviewed changes

src/libexpr/primops.cc Outdated Show resolved Hide resolved

src/libexpr/primops.cc Outdated Show resolved Hide resolved

doc/manual/src/release-notes/rl-next.md Outdated Show resolved Hide resolved

doc/manual/src/release-notes/rl-next.md Outdated Show resolved Hide resolved

roberth mentioned this pull request Oct 6, 2023

From-source nix-built artifacts without the "nix" Extra Attribute natural-transformation/sbtix#31

Merged

ShamrockLee force-pushed the primop-rebasehash branch from fae78e3 to 7cf889e Compare October 9, 2023 06:53

github-actions bot removed the documentation label Oct 9, 2023

fricklerhandwerk approved these changes Oct 10, 2023

View reviewed changes

src/libexpr/primops.cc Outdated Show resolved Hide resolved

src/libutil/hash.hh Outdated Show resolved Hide resolved

src/libexpr/primops.cc Outdated Show resolved Hide resolved

src/libexpr/primops.cc Outdated Show resolved Hide resolved

src/libexpr/primops.cc Outdated Show resolved Hide resolved

ShamrockLee force-pushed the primop-rebasehash branch from c7a0009 to 84f150a Compare October 11, 2023 08:46

github-actions bot added new-cli Relating to the "nix" command store Issues and pull requests concerning the Nix store labels Oct 11, 2023

ShamrockLee force-pushed the primop-rebasehash branch from 84f150a to 2d37f33 Compare October 11, 2023 09:11

ShamrockLee requested a review from fricklerhandwerk October 11, 2023 09:57

fricklerhandwerk approved these changes Oct 11, 2023

View reviewed changes

src/libexpr/primops.cc Outdated Show resolved Hide resolved

src/libexpr/primops.cc Outdated Show resolved Hide resolved

Ericson2314 reviewed Oct 11, 2023

View reviewed changes

src/libutil/hash.hh Outdated Show resolved Hide resolved

Ericson2314 reviewed Oct 11, 2023

View reviewed changes

src/libexpr/primops.cc Show resolved Hide resolved

ShamrockLee force-pushed the primop-rebasehash branch from 4c21b46 to 947e60b Compare October 18, 2023 16:27

github-actions bot added the fetching Networking with the outside (non-Nix) world label Oct 18, 2023

ShamrockLee and others added 8 commits October 19, 2023 00:38

Simplify parseHashTypeOpt

e9ddf0b

Remove redundant "else" after "return". Use std::nullopt to increase readability.

Elaborate the "unknown hash algorithm" error

aff177d

List the allowed hash formats

Document HashFormat

5043e6c

treewide: Reference HashFormat members with scope

e026f3e

Base* -> HashFormat::Base*

Migrate HashFormat to scoped enumeration (enum struct)

231b0fc

Add helper function parseHashFormat[Opt] printHashFormat

6b47635

Add hash format analogy of parseHashTypeOpt, parseHashType, and printHashType. Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>

primops: add builtins.convertHash

5088e65

Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>

ShamrockLee force-pushed the primop-rebasehash branch from 947e60b to 5088e65 Compare October 18, 2023 16:59

ShamrockLee requested a review from Ericson2314 October 18, 2023 18:23

fricklerhandwerk merged commit 8b48fb1 into NixOS:master Oct 19, 2023
8 checks passed

amarshall mentioned this pull request Jan 12, 2024

Add the builtin hashTo #3151

Closed

ShamrockLee deleted the primop-rebasehash branch February 25, 2024 17:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

primops: add builtins.convertHash #7708

primops: add builtins.convertHash #7708

ShamrockLee commented Jan 29, 2023 •

edited

edolstra commented Jan 30, 2023

ShamrockLee commented Jan 30, 2023

ShamrockLee commented Jan 30, 2023

aameen-tulip commented Feb 17, 2023

ShamrockLee commented Mar 16, 2023

ShamrockLee commented Mar 16, 2023

roberth commented Jun 9, 2023

nixos-discourse commented Jun 16, 2023

nixos-discourse commented Sep 5, 2023

ShamrockLee commented Sep 5, 2023 •

edited

fricklerhandwerk left a comment

fricklerhandwerk commented Oct 6, 2023

ShamrockLee commented Oct 9, 2023 •

edited

fricklerhandwerk left a comment

Ericson2314 Oct 11, 2023

ShamrockLee Oct 18, 2023

Ericson2314 Oct 19, 2023

ShamrockLee commented Oct 18, 2023

fricklerhandwerk commented Oct 19, 2023

primops: add builtins.convertHash #7708

primops: add builtins.convertHash #7708

Conversation

ShamrockLee commented Jan 29, 2023 • edited

Motivation

Context

Checklist for maintainers

edolstra commented Jan 30, 2023

ShamrockLee commented Jan 30, 2023

ShamrockLee commented Jan 30, 2023

aameen-tulip commented Feb 17, 2023

ShamrockLee commented Mar 16, 2023

ShamrockLee commented Mar 16, 2023

roberth commented Jun 9, 2023

nixos-discourse commented Jun 16, 2023

nixos-discourse commented Sep 5, 2023

ShamrockLee commented Sep 5, 2023 • edited

fricklerhandwerk left a comment

Choose a reason for hiding this comment

fricklerhandwerk commented Oct 6, 2023

ShamrockLee commented Oct 9, 2023 • edited

fricklerhandwerk left a comment

Choose a reason for hiding this comment

Ericson2314 Oct 11, 2023

Choose a reason for hiding this comment

ShamrockLee Oct 18, 2023

Choose a reason for hiding this comment

Ericson2314 Oct 19, 2023

Choose a reason for hiding this comment

ShamrockLee commented Oct 18, 2023

fricklerhandwerk commented Oct 19, 2023

ShamrockLee commented Jan 29, 2023 •

edited

ShamrockLee commented Sep 5, 2023 •

edited

ShamrockLee commented Oct 9, 2023 •

edited