fetchClosure: Allow input addressed paths in pure mode

When explicitly requested by the caller, as suggested in the meeting
(https://github.com/NixOS/nix/pull/8090#issuecomment-1531139324)

> @edolstra: { toPath } vs { fromPath } is too implicit

I've opted for the `inputAddressed = true` requirement, because it
we did not agree on renaming the path attributes.

> @roberth: more explicit
> @edolstra: except for the direction; not immediately clear in which direction the rewriting happens

This is in fact the most explicit syntax and a bit redundant, which is
good, because that redundancy lets us deliver an error message that
reminds expression authors that CA provides a better experience to
their users.
This commit is contained in:
Robert Hensing 2023-05-19 10:48:53 +02:00
parent 7e5b6d2c45
commit ea30f152b7
2 changed files with 57 additions and 13 deletions

View file

@ -13,6 +13,7 @@ static void prim_fetchClosure(EvalState & state, const PosIdx pos, Value * * arg
std::optional<StorePath> fromPath; std::optional<StorePath> fromPath;
bool enableRewriting = false; bool enableRewriting = false;
std::optional<StorePath> toPath; std::optional<StorePath> toPath;
bool inputAddressed = false;
for (auto & attr : *args[0]->attrs) { for (auto & attr : *args[0]->attrs) {
const auto & attrName = state.symbols[attr.name]; const auto & attrName = state.symbols[attr.name];
@ -38,6 +39,9 @@ static void prim_fetchClosure(EvalState & state, const PosIdx pos, Value * * arg
fromStoreUrl = state.forceStringNoCtx(*attr.value, attr.pos, fromStoreUrl = state.forceStringNoCtx(*attr.value, attr.pos,
attrHint()); attrHint());
else if (attrName == "inputAddressed")
inputAddressed = state.forceBool(*attr.value, attr.pos, attrHint());
else else
throw Error({ throw Error({
.msg = hintfmt("attribute '%s' isn't supported in call to 'fetchClosure'", attrName), .msg = hintfmt("attribute '%s' isn't supported in call to 'fetchClosure'", attrName),
@ -51,6 +55,18 @@ static void prim_fetchClosure(EvalState & state, const PosIdx pos, Value * * arg
.errPos = state.positions[pos] .errPos = state.positions[pos]
}); });
if (inputAddressed) {
if (toPath && toPath != fromPath)
throw Error({
.msg = hintfmt("attribute '%s' is set to true, but 'toPath' does not match 'fromPath'. 'toPath' should be equal, or should be omitted. Instead 'toPath' was '%s' and 'fromPath' was '%s'",
"inputAddressed",
state.store->printStorePath(*toPath),
state.store->printStorePath(*fromPath)),
.errPos = state.positions[pos]
});
assert(!enableRewriting);
}
if (!fromStoreUrl) if (!fromStoreUrl)
throw Error({ throw Error({
.msg = hintfmt("attribute '%s' is missing in call to 'fetchClosure'", "fromStore"), .msg = hintfmt("attribute '%s' is missing in call to 'fetchClosure'", "fromStore"),
@ -104,15 +120,28 @@ static void prim_fetchClosure(EvalState & state, const PosIdx pos, Value * * arg
toPath = fromPath; toPath = fromPath;
} }
/* In pure mode, require a CA path. */ /* We want input addressing to be explicit, to inform readers and to give
if (evalSettings.pureEval) { expression authors an opportunity to improve their user experience. */
if (!inputAddressed) {
auto info = state.store->queryPathInfo(*toPath); auto info = state.store->queryPathInfo(*toPath);
if (!info->isContentAddressed(*state.store)) if (!info->isContentAddressed(*state.store)) {
throw Error({ if (enableRewriting) {
.msg = hintfmt("in pure mode, 'fetchClosure' requires a content-addressed path, which '%s' isn't", throw Error({
state.store->printStorePath(*toPath)), // Ideally we'd compute the path for them, but this error message is unlikely to occur in practice, so we keep it simple.
.errPos = state.positions[pos] .msg = hintfmt("Rewriting was requested, but 'toPath' is not content addressed. This is impossible. Please change 'toPath' to the correct path, or to a non-existing path, and try again",
}); state.store->printStorePath(*toPath)),
.errPos = state.positions[pos]
});
} else {
// We just checked toPath, but we report fromPath, because that's what the user certainly passed.
assert (toPath == fromPath);
throw Error({
.msg = hintfmt("The 'fromPath' value '%s' is input addressed, but input addressing was not requested. If you do intend to return an input addressed store path, add 'inputAddressed = true;' to the 'fetchClosure' arguments. Note that content addressing does not require users to configure a trusted binary cache public key on their systems, and is therefore preferred.",
state.store->printStorePath(*fromPath)),
.errPos = state.positions[pos]
});
}
}
} }
state.mkStorePathString(*toPath, v); state.mkStorePathString(*toPath, v);
@ -138,7 +167,7 @@ static RegisterPrimOp primop_fetchClosure({
`/nix/store/ldbh...`. `/nix/store/ldbh...`.
If `fromPath` is already content-addressed, or if you are If `fromPath` is already content-addressed, or if you are
allowing impure evaluation (`--impure`), then `toPath` may be allowing input addressing (`inputAddressed = true;`), then `toPath` may be
omitted. omitted.
To find out the correct value for `toPath` given a `fromPath`, To find out the correct value for `toPath` given a `fromPath`,
@ -153,8 +182,11 @@ static RegisterPrimOp primop_fetchClosure({
allows you to use a previously built store path in a Nix allows you to use a previously built store path in a Nix
expression. However, it is more reproducible because it requires expression. However, it is more reproducible because it requires
specifying a binary cache from which the path can be fetched. specifying a binary cache from which the path can be fetched.
Also, requiring a content-addressed final store path avoids the Also, the default requirement of a content-addressed final store path
need for users to configure binary cache public keys. avoids the need for users to configure [`trusted-public-keys`](@docroot@/command-ref/conf-file.md#conf-trusted-public-keys).
This function is only available if you enable the experimental
feature `fetch-closure`.
)", )",
.fun = prim_fetchClosure, .fun = prim_fetchClosure,
.experimentalFeature = Xp::FetchClosure, .experimentalFeature = Xp::FetchClosure,

View file

@ -35,16 +35,28 @@ clearStore
if [[ "$NIX_REMOTE" != "daemon" ]]; then if [[ "$NIX_REMOTE" != "daemon" ]]; then
# In impure mode, we can use non-CA paths. # We can use non-CA paths when we ask explicitly.
[[ $(nix eval --raw --no-require-sigs --impure --expr " [[ $(nix eval --raw --no-require-sigs --expr "
builtins.fetchClosure { builtins.fetchClosure {
fromStore = \"file://$cacheDir\"; fromStore = \"file://$cacheDir\";
fromPath = $nonCaPath; fromPath = $nonCaPath;
inputAddressed = true;
} }
") = $nonCaPath ]] ") = $nonCaPath ]]
[ -e $nonCaPath ] [ -e $nonCaPath ]
# .. but only if we ask explicitly.
expectStderr 1 nix eval --raw --no-require-sigs --expr "
builtins.fetchClosure {
fromStore = \"file://$cacheDir\";
fromPath = $nonCaPath;
}
" | grepQuiet -E "The .fromPath. value .* is input addressed, but input addressing was not requested. If you do intend to return an input addressed store path, add .inputAddressed = true;. to the .fetchClosure. arguments."
[ -e $nonCaPath ]
fi fi
# 'toPath' set to empty string should fail but print the expected path. # 'toPath' set to empty string should fail but print the expected path.