Skip to content

Commit

Permalink
Make outputHashAlgo accept "nar", stay in sync
Browse files Browse the repository at this point in the history
Now that we have a few things identifying content address methods by
name, we should be consistent about it.

Move up the `parseHashAlgoOpt` for tidiness too.

Discussed this change for consistency's sake as part of NixOS#8876
  • Loading branch information
Ericson2314 committed Feb 15, 2024
1 parent bc9471d commit 08d293a
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 14 deletions.
7 changes: 5 additions & 2 deletions doc/manual/src/language/advanced-attributes.md
Original file line number Diff line number Diff line change
Expand Up @@ -203,12 +203,15 @@ Derivations can declare some infrequently used optional attributes.

This is the default.

- `"recursive"`\
The hash is computed over the NAR archive dump of the output
- `"nar"` or `"recursive"`\
The hash is computed over the [NAR archive](@docroot@/glossary.md#gloss-nar) dump of the output
(i.e., the result of [`nix-store --dump`](@docroot@/command-ref/nix-store/dump.md)). In
this case, the output can be anything, including a directory
tree.

`"nar"` is more clear, and consistent with other parts of Nix (such as the CLI), however support for it is only added in Nix version 2.21.
`"recursive"` is the traditional way of indicating this and is supported since 2005 (virtually the entire history of Nix).

The `outputHash` attribute, finally, must be a string containing
the hash in either hexadecimal or base-32 notation. (See the
[`nix-hash` command](../command-ref/nix-hash.md) for information
Expand Down
25 changes: 14 additions & 11 deletions src/libexpr/primops.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1108,7 +1108,7 @@ drvName, Bindings * attrs, Value & v)
bool contentAddressed = false;
bool isImpure = false;
std::optional<std::string> outputHash;
std::string outputHashAlgo;
std::optional<HashAlgorithm> outputHashAlgo;
std::optional<ContentAddressMethod> ingestionMethod;

StringSet outputs;
Expand All @@ -1120,15 +1120,18 @@ drvName, Bindings * attrs, Value & v)
vomit("processing attribute '%1%'", key);

auto handleHashMode = [&](const std::string_view s) {
if (s == "recursive") ingestionMethod = FileIngestionMethod::Recursive;
else if (s == "flat") ingestionMethod = FileIngestionMethod::Flat;
else if (s == "text") {
experimentalFeatureSettings.require(Xp::DynamicDerivations);
ingestionMethod = TextIngestionMethod {};
} else
if (s == "recursive") {
// back compat, new name is "nar"
ingestionMethod = FileIngestionMethod::Recursive;
} else try {
ingestionMethod = ContentAddressMethod::parse(s);
} catch (UsageError &) {
state.error<EvalError>(
"invalid value '%s' for 'outputHashMode' attribute", s
).atPos(v).debugThrow();
}
if (ingestionMethod == TextIngestionMethod {})
experimentalFeatureSettings.require(Xp::DynamicDerivations);
};

auto handleOutputs = [&](const Strings & ss) {
Expand Down Expand Up @@ -1204,7 +1207,7 @@ drvName, Bindings * attrs, Value & v)
else if (i->name == state.sOutputHash)
outputHash = state.forceStringNoCtx(*i->value, pos, context_below);
else if (i->name == state.sOutputHashAlgo)
outputHashAlgo = state.forceStringNoCtx(*i->value, pos, context_below);
outputHashAlgo = parseHashAlgoOpt(state.forceStringNoCtx(*i->value, pos, context_below));
else if (i->name == state.sOutputHashMode)
handleHashMode(state.forceStringNoCtx(*i->value, pos, context_below));
else if (i->name == state.sOutputs) {
Expand All @@ -1222,7 +1225,7 @@ drvName, Bindings * attrs, Value & v)
if (i->name == state.sBuilder) drv.builder = std::move(s);
else if (i->name == state.sSystem) drv.platform = std::move(s);
else if (i->name == state.sOutputHash) outputHash = std::move(s);
else if (i->name == state.sOutputHashAlgo) outputHashAlgo = std::move(s);
else if (i->name == state.sOutputHashAlgo) outputHashAlgo = parseHashAlgoOpt(s);
else if (i->name == state.sOutputHashMode) handleHashMode(s);
else if (i->name == state.sOutputs)
handleOutputs(tokenizeString<Strings>(s));
Expand Down Expand Up @@ -1306,7 +1309,7 @@ drvName, Bindings * attrs, Value & v)
"multiple outputs are not supported in fixed-output derivations"
).atPos(v).debugThrow();

auto h = newHashAllowEmpty(*outputHash, parseHashAlgoOpt(outputHashAlgo));
auto h = newHashAllowEmpty(*outputHash, outputHashAlgo);

auto method = ingestionMethod.value_or(FileIngestionMethod::Flat);

Expand All @@ -1326,7 +1329,7 @@ drvName, Bindings * attrs, Value & v)
state.error<EvalError>("derivation cannot be both content-addressed and impure")
.atPos(v).debugThrow();

auto ha = parseHashAlgoOpt(outputHashAlgo).value_or(HashAlgorithm::SHA256);
auto ha = outputHashAlgo.value_or(HashAlgorithm::SHA256);
auto method = ingestionMethod.value_or(FileIngestionMethod::Recursive);

for (auto & i : outputs) {
Expand Down
4 changes: 3 additions & 1 deletion tests/functional/fixed.nix
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,13 @@ rec {
outputHashMode = "flat";
};

# Test for building two derivations in parallel that produce the
# Test for building derivations in parallel that produce the
# same output path because they're fixed-output derivations.
parallelSame = [
(f2 "foo" ./fixed.builder2.sh "recursive" "md5" "3670af73070fa14077ad74e0f5ea4e42")
(f2 "bar" ./fixed.builder2.sh "recursive" "md5" "3670af73070fa14077ad74e0f5ea4e42")
(f2 "foo" ./fixed.builder2.sh "nar" "md5" "3670af73070fa14077ad74e0f5ea4e42")
(f2 "bar" ./fixed.builder2.sh "nar" "md5" "3670af73070fa14077ad74e0f5ea4e42")
];

}

0 comments on commit 08d293a

Please sign in to comment.