Skip to content

Commit

Permalink
fetchClosure: Allow input addressed paths in pure mode
Browse files Browse the repository at this point in the history
When explicitly requested by the caller, as suggested in the meeting
(NixOS#8090 (comment))

> @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.
  • Loading branch information
roberth committed Jun 5, 2023
1 parent b3b78aa commit 8205e0a
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 13 deletions.
51 changes: 40 additions & 11 deletions src/libexpr/primops/fetchClosure.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ static void prim_fetchClosure(EvalState & state, const PosIdx pos, Value * * arg
std::optional<StorePath> fromPath;
bool enableRewriting = false;
std::optional<StorePath> toPath;
bool inputAddressed = false;

for (auto & attr : *args[0]->attrs) {
const auto & attrName = state.symbols[attr.name];
Expand All @@ -38,6 +39,9 @@ static void prim_fetchClosure(EvalState & state, const PosIdx pos, Value * * arg
fromStoreUrl = state.forceStringNoCtx(*attr.value, attr.pos,
attrHint());

else if (attrName == "inputAddressed")
inputAddressed = state.forceBool(*attr.value, attr.pos, attrHint());

else
throw Error({
.msg = hintfmt("attribute '%s' isn't supported in call to 'fetchClosure'", attrName),
Expand All @@ -51,6 +55,18 @@ static void prim_fetchClosure(EvalState & state, const PosIdx pos, Value * * arg
.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)
throw Error({
.msg = hintfmt("attribute '%s' is missing in call to 'fetchClosure'", "fromStore"),
Expand Down Expand Up @@ -104,15 +120,28 @@ static void prim_fetchClosure(EvalState & state, const PosIdx pos, Value * * arg
toPath = fromPath;
}

/* In pure mode, require a CA path. */
if (evalSettings.pureEval) {
/* We want input addressing to be explicit, to inform readers and to give
expression authors an opportunity to improve their user experience. */
if (!inputAddressed) {
auto info = state.store->queryPathInfo(*toPath);
if (!info->isContentAddressed(*state.store))
throw Error({
.msg = hintfmt("in pure mode, 'fetchClosure' requires a content-addressed path, which '%s' isn't",
state.store->printStorePath(*toPath)),
.errPos = state.positions[pos]
});
if (!info->isContentAddressed(*state.store)) {
if (enableRewriting) {
throw Error({
// Ideally we'd compute the path for them, but this error message is unlikely to occur in practice, so we keep it simple.
.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);
Expand All @@ -138,7 +167,7 @@ static RegisterPrimOp primop_fetchClosure({
`/nix/store/ldbh...`.
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.
To find out the correct value for `toPath` given a `fromPath`,
Expand All @@ -153,8 +182,8 @@ static RegisterPrimOp primop_fetchClosure({
allows you to use a previously built store path in a Nix
expression. However, it is more reproducible because it requires
specifying a binary cache from which the path can be fetched.
Also, requiring a content-addressed final store path avoids the
need for users to configure binary cache public keys.
Also, the default requirement of a content-addressed final store path
avoids the need for users to configure binary cache public keys.
This function is only available if you enable the experimental
feature `fetch-closure`.
Expand Down
16 changes: 14 additions & 2 deletions tests/fetchClosure.sh
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,28 @@ clearStore

if [[ "$NIX_REMOTE" != "daemon" ]]; then

# In impure mode, we can use non-CA paths.
[[ $(nix eval --raw --no-require-sigs --impure --expr "
# We can use non-CA paths when we ask explicitly.
[[ $(nix eval --raw --no-require-sigs --expr "
builtins.fetchClosure {
fromStore = \"file://$cacheDir\";
fromPath = $nonCaPath;
inputAddressed = true;
}
") = $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

# 'toPath' set to empty string should fail but print the expected path.
Expand Down

0 comments on commit 8205e0a

Please sign in to comment.