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

Make outputHashAlgo accept "nar", stay in sync #10021

Merged
merged 1 commit into from
Apr 9, 2024
Merged

Make outputHashAlgo accept "nar", stay in sync #10021

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions doc/manual/src/language/advanced-attributes.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,12 +207,17 @@ 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
- `"recursive"` or `"nar"`\
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.

`"recursive"` is the traditional way of indicating this,
and is supported since 2005 (virtually the entire history of Nix).
`"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.

- [`__contentAddressed`]{#adv-attr-__contentAddressed}
> **Warning**
> This attribute is part of an [experimental feature](@docroot@/contributing/experimental-features.md).
Expand Down
20 changes: 11 additions & 9 deletions src/libexpr/primops.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1139,18 +1139,20 @@ 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 == "git") {
experimentalFeatureSettings.require(Xp::GitHashing);
ingestionMethod = FileIngestionMethod::Git;
} else if (s == "text") {
experimentalFeatureSettings.require(Xp::DynamicDerivations);
ingestionMethod = TextIngestionMethod {};
} else
if (s == "recursive") {
// back compat, new name is "nar"
ingestionMethod = FileIngestionMethod::Recursive;
Comment on lines +1142 to +1144
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we instead want to accept `recursive as a file ingestion method for consistency?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure what you mean, I think change ContentAddressMethod::parse so this one is accepted everywhere (e.g. the new CLI too)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's what I meant

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I forget whether we discussed this before, but I do want to over time steer people to the new name (which is clearer) in the new CLI.

Copy link
Member Author

@Ericson2314 Ericson2314 Feb 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Also with git support, which I hope to merge very soon, NAR isn't the only "recursive" format anyways.)

} 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);
if (ingestionMethod == FileIngestionMethod::Git)
experimentalFeatureSettings.require(Xp::GitHashing);
};

auto handleOutputs = [&](const Strings & ss) {
Expand Down
2 changes: 2 additions & 0 deletions tests/functional/fixed.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,4 +64,6 @@ rec {
(f2 "bar" ./fixed.builder2.sh "recursive" "md5" "3670af73070fa14077ad74e0f5ea4e42")
];

# Can use "nar" instead of "recursive" now.
nar-not-recursive = f2 "foo" ./fixed.builder2.sh "nar" "md5" "3670af73070fa14077ad74e0f5ea4e42";
}
4 changes: 4 additions & 0 deletions tests/functional/fixed.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,3 +61,7 @@ out3=$(nix-store --add-fixed --recursive sha256 $TEST_ROOT/fixed)

out4=$(nix-store --print-fixed-path --recursive sha256 "1ixr6yd3297ciyp9im522dfxpqbkhcw0pylkb2aab915278fqaik" fixed)
[ "$out" = "$out4" ]

# Can use `outputHashMode = "nar";` instead of `"recursive"` now.
clearStore
nix-build fixed.nix -A nar-not-recursive --no-out-link
Loading