lix-project/lix
20
55
Fork 31
Code Issues 344 Code Review (Gerrit) Projects 3 Releases Wiki Activity

fetchClosure: Always check that inputAddressed matches the result

Browse source
This commit is contained in:
Robert Hensing 2023-05-19 16:25:23 +02:00
parent ea30f152b7
commit 508aa58e67
1 changed files with 34 additions and 8 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

42
src/libexpr/primops/fetchClosure.cc
View file

@ -13,7 +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; std::optional<bool> inputAddressedMaybe;
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];
@ -40,7 +40,7 @@ static void prim_fetchClosure(EvalState & state, const PosIdx pos, Value * * arg
attrHint()); attrHint());
else if (attrName == "inputAddressed") else if (attrName == "inputAddressed")
inputAddressed = state.forceBool(*attr.value, attr.pos, attrHint()); inputAddressedMaybe = state.forceBool(*attr.value, attr.pos, attrHint());
else else
throw Error({ throw Error({
@ -55,6 +55,8 @@ static void prim_fetchClosure(EvalState & state, const PosIdx pos, Value * * arg
.errPos = state.positions[pos] .errPos = state.positions[pos]
}); });
bool inputAddressed = inputAddressedMaybe.value_or(false);
if (inputAddressed) { if (inputAddressed) {
if (toPath && toPath != fromPath) if (toPath && toPath != fromPath)
throw Error({ throw Error({
@ -120,15 +122,39 @@ static void prim_fetchClosure(EvalState & state, const PosIdx pos, Value * * arg
toPath = fromPath; toPath = fromPath;
} }
/* We want input addressing to be explicit, to inform readers and to give auto info = state.store->queryPathInfo(*toPath);
expression authors an opportunity to improve their user experience. */
if (!inputAddressed) { /* If inputAddressed is set, make sure the result is as expected. */
auto info = state.store->queryPathInfo(*toPath); if (inputAddressedMaybe) {
bool expectCA = !inputAddressed;
if (info->isContentAddressed(*state.store) != expectCA) {
if (inputAddressed)
throw Error({
.msg = hintfmt("The 'fetchClosure' result, '%s' is not input addressed, despite 'inputAddressed' being set to true. It is ok or even preferable to return a content addressed path, so you probably want to remove the 'inputAddressed' attribute",
state.store->printStorePath(*toPath)),
.errPos = state.positions[pos]
});
else
throw Error({
.msg = hintfmt("The 'fetchClosure' result, '%s' is input addressed, despite 'inputAddressed' being set to false. Please make sure you passed the correct path(s) or set 'inputAddressed' to true if you intended to return an input addressed path",
state.store->printStorePath(*toPath)),
.errPos = state.positions[pos]
});
}
} else {
/*
While it's fine to omit the inputAddressed attribute for CA paths,
we want input addressing to be explicit, to inform readers and to give
expression authors an opportunity to improve their user experience;
see message below.
*/
if (!info->isContentAddressed(*state.store)) { if (!info->isContentAddressed(*state.store)) {
if (enableRewriting) { if (enableRewriting) {
// We don't perform the rewriting when outPath already exists, as an optimisation.
// However, we can quickly detect a mistake if the toPath is input addressed.
// Ideally we'd compute the path for them here, but this error message is unlikely to occur in practice, so we keep it simple.
throw Error({ 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("The 'toPath' value '%s' is input addressed, so it can't possibly be the result of rewriting. You may set 'toPath' to an empty string to figure out the correct path.",
.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)), state.store->printStorePath(*toPath)),
.errPos = state.positions[pos] .errPos = state.positions[pos]
}); });