From 5bdca46117c701cd1cd47700c755367442537157 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Mon, 5 Jun 2023 12:35:03 +0200 Subject: [PATCH] fetchClosure: Split into three cases --- src/libexpr/primops/fetchClosure.cc | 177 ++++++++++++++++------------ 1 file changed, 101 insertions(+), 76 deletions(-) diff --git a/src/libexpr/primops/fetchClosure.cc b/src/libexpr/primops/fetchClosure.cc index 6a15c826f..e8c32de96 100644 --- a/src/libexpr/primops/fetchClosure.cc +++ b/src/libexpr/primops/fetchClosure.cc @@ -5,6 +5,101 @@ namespace nix { +/** + * Handler for the content addressed case. + * + * @param state The evaluator state and store to write to. + * @param fromStore The store containing the path to rewrite. + * @param fromPath The source path to be rewritten. + * @param toPathMaybe The path to write the rewritten path to. If empty, the error shows the actual path. + * @param v The return `Value` + */ +static void runFetchClosureWithRewrite(EvalState & state, const PosIdx pos, Store & fromStore, const StorePath & fromPath, const std::optional & toPathMaybe, Value &v) { + + // establish toPath or throw + + if (!toPathMaybe || !state.store->isValidPath(*toPathMaybe)) { + auto rewrittenPath = makeContentAddressed(fromStore, *state.store, fromPath); + if (toPathMaybe && *toPathMaybe != rewrittenPath) + throw Error({ + .msg = hintfmt("rewriting '%s' to content-addressed form yielded '%s', while '%s' was expected", + state.store->printStorePath(fromPath), + state.store->printStorePath(rewrittenPath), + state.store->printStorePath(*toPathMaybe)), + .errPos = state.positions[pos] + }); + if (!toPathMaybe) + throw Error({ + .msg = hintfmt( + "rewriting '%s' to content-addressed form yielded '%s'; " + "please set this in the 'toPath' attribute passed to 'fetchClosure'", + state.store->printStorePath(fromPath), + state.store->printStorePath(rewrittenPath)), + .errPos = state.positions[pos] + }); + } + + auto toPath = *toPathMaybe; + + // check and return + + auto resultInfo = state.store->queryPathInfo(toPath); + + if (!resultInfo->isContentAddressed(*state.store)) { + // 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. + throw Error({ + .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.", + state.store->printStorePath(toPath)), + .errPos = state.positions[pos] + }); + } + + state.mkStorePathString(toPath, v); +} + +/** + * Fetch the closure and make sure it's content addressed. + */ +static void runFetchClosureWithContentAddressedPath(EvalState & state, const PosIdx pos, Store & fromStore, const StorePath & fromPath, Value & v) { + + if (!state.store->isValidPath(fromPath)) + copyClosure(fromStore, *state.store, RealisedPath::Set { fromPath }); + + auto info = state.store->queryPathInfo(fromPath); + + if (!info->isContentAddressed(*state.store)) { + 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(fromPath, v); +} + +/** + * Fetch the closure and make sure it's input addressed. + */ +static void runFetchClosureWithInputAddressedPath(EvalState & state, const PosIdx pos, Store & fromStore, const StorePath & fromPath, Value & v) { + + if (!state.store->isValidPath(fromPath)) + copyClosure(fromStore, *state.store, RealisedPath::Set { fromPath }); + + auto info = state.store->queryPathInfo(fromPath); + + if (info->isContentAddressed(*state.store)) { + throw Error({ + .msg = hintfmt("The 'fetchClosure' result, '%s' is not input addressed, despite 'inputAddressed' being set to true. It is preferable to return a content addressed path, so remove the 'inputAddressed' attribute to ensure content addressing is used in the future", + state.store->printStorePath(fromPath)), + .errPos = state.positions[pos] + }); + } + + state.mkStorePathString(fromPath, v); +} + static void prim_fetchClosure(EvalState & state, const PosIdx pos, Value * * args, Value & v) { state.forceAttrs(*args[0], pos, "while evaluating the argument passed to builtins.fetchClosure"); @@ -92,82 +187,12 @@ static void prim_fetchClosure(EvalState & state, const PosIdx pos, Value * * arg auto fromStore = openStore(parsedURL.to_string()); - if (enableRewriting) { - if (!toPath || !state.store->isValidPath(*toPath)) { - auto rewrittenPath = makeContentAddressed(*fromStore, *state.store, *fromPath); - if (toPath && *toPath != rewrittenPath) - throw Error({ - .msg = hintfmt("rewriting '%s' to content-addressed form yielded '%s', while '%s' was expected", - state.store->printStorePath(*fromPath), - state.store->printStorePath(rewrittenPath), - state.store->printStorePath(*toPath)), - .errPos = state.positions[pos] - }); - if (!toPath) - throw Error({ - .msg = hintfmt( - "rewriting '%s' to content-addressed form yielded '%s'; " - "please set this in the 'toPath' attribute passed to 'fetchClosure'", - state.store->printStorePath(*fromPath), - state.store->printStorePath(rewrittenPath)), - .errPos = state.positions[pos] - }); - } - } else { - if (!state.store->isValidPath(*fromPath)) - copyClosure(*fromStore, *state.store, RealisedPath::Set { *fromPath }); - toPath = fromPath; - } - - auto info = state.store->queryPathInfo(*toPath); - - /* If inputAddressed is set, make sure the result is as expected. */ - 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 (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({ - .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.", - 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); + if (enableRewriting) + runFetchClosureWithRewrite(state, pos, *fromStore, *fromPath, toPath, v); + else if (inputAddressed) + runFetchClosureWithInputAddressedPath(state, pos, *fromStore, *fromPath, v); + else + runFetchClosureWithContentAddressedPath(state, pos, *fromStore, *fromPath, v); } static RegisterPrimOp primop_fetchClosure({