fetchClosure: Split into three cases

1 files 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<StorePath> & 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({