Desugar structured attrs, "export reference graph" outside DerivationBuilder

I think this is a better separation of concerns. `DerivationBuilder`
doesn't need to to the final, query-heavy details about how these things
are constructed. It just operates on the level of "simple, stupid" files
and environment variables.

src/libstore/build/derivation-building-goal.cc

@@ -677,6 +677,42 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
             auto * localStoreP = dynamic_cast<LocalStore *>(&worker.store);
             assert(localStoreP);
 
+            decltype(DerivationBuilderParams::extraEnv) extraEnv;
+            decltype(DerivationBuilderParams::extraFiles) extraFiles;
+
+            try {
+                if (drv->structuredAttrs) {
+                    auto json = drv->structuredAttrs->prepareStructuredAttrs(
+                        worker.store, *drvOptions, inputPaths, drv->outputs);
+
+                    extraEnv.insert_or_assign(
+                        "NIX_ATTRS_SH_FILE",
+                        DerivationBuilderParams::EnvEntry{
+                            .nameOfPassAsFile = ".attrs.sh",
+                            .value = StructuredAttrs::writeShell(json),
+                        });
+                    extraEnv.insert_or_assign(
+                        "NIX_ATTRS_JSON_FILE",
+                        DerivationBuilderParams::EnvEntry{
+                            .nameOfPassAsFile = ".attrs.json",
+                            .value = json.dump(),
+                        });
+                } else {
+                    /* Handle exportReferencesGraph(), if set. */
+                    for (auto & [fileName, storePaths] : drvOptions->getParsedExportReferencesGraph(worker.store)) {
+                        /* Write closure info to <fileName>. */
+                        extraFiles.insert_or_assign(
+                            fileName,
+                            worker.store.makeValidityRegistration(
+                                worker.store.exportReferences(storePaths, inputPaths), false, false));
+                    }
+                }
+            } catch (BuildError & e) {
+                outputLocks.unlock();
+                worker.permanentFailure = true;
+                co_return done(BuildResult::InputRejected, {}, std::move(e));
+            }
+
             /* If we have to wait and retry (see below), then `builder` will
                already be created, so we don't need to create it again. */
             builder = makeDerivationBuilder(
@@ -690,6 +726,8 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
                     *drvOptions,
                     inputPaths,
                     initialOutputs,
+                    std::move(extraEnv),
+                    std::move(extraFiles),
                 });
         }
 

src/libstore/include/nix/store/build/derivation-builder.hh

@@ -59,6 +59,35 @@ struct DerivationBuilderParams
 
     const BuildMode & buildMode;
 
+    struct EnvEntry
+    {
+        /**
+         * Actually, this should be passed as a file, but with a custom
+         * name (rather than hash-derived name for usual "pass as file").
+         */
+        std::optional<std::string> nameOfPassAsFile;
+
+        /**
+         * String value of env var, or contents of the file
+         */
+        std::string value;
+    };
+
+    /**
+     * Extra environment variables to additionally set, possibly
+     * indirectly via a file.
+     *
+     * This is used by the caller to desugar the "structured attrs"
+     * mechanism, so `DerivationBuilder` doesn't need to know about it.
+     */
+    std::map<std::string, EnvEntry, std::less<>> extraEnv;
+
+    /**
+     * Inserted in the temp dir, but no file names placed in env, unlike
+     * `EnvEntry::nameOfPassAsFile` above.
+     */
+    StringMap extraFiles;
+
     DerivationBuilderParams(
         const StorePath & drvPath,
         const BuildMode & buildMode,
@@ -66,7 +95,9 @@ struct DerivationBuilderParams
         const Derivation & drv,
         const DerivationOptions & drvOptions,
         const StorePathSet & inputPaths,
-        std::map<std::string, InitialOutput> & initialOutputs)
+        std::map<std::string, InitialOutput> & initialOutputs,
+        std::map<std::string, EnvEntry, std::less<>> extraEnv,
+        StringMap extraFiles)
         : drvPath{drvPath}
         , buildResult{buildResult}
         , drv{drv}
@@ -74,6 +105,8 @@ struct DerivationBuilderParams
         , inputPaths{inputPaths}
         , initialOutputs{initialOutputs}
         , buildMode{buildMode}
+        , extraEnv{std::move(extraEnv)}
+        , extraFiles{std::move(extraFiles)}
     {
     }
 

src/libstore/unix/build/derivation-builder.cc

@@ -273,11 +273,6 @@ protected:
 
 private:
 
-    /**
-     * Write a JSON file containing the derivation attributes.
-     */
-    void writeStructuredAttrs();
-
     /**
      * Start an in-process nix daemon thread for recursive-nix.
      */
@@ -781,18 +776,6 @@ void DerivationBuilderImpl::startBuilder()
     /* Construct the environment passed to the builder. */
     initEnv();
 
-    writeStructuredAttrs();
-
-    /* Handle exportReferencesGraph(), if set. */
-    if (!drv.structuredAttrs) {
-        for (auto & [fileName, storePaths] : drvOptions.getParsedExportReferencesGraph(store)) {
-            /* Write closure info to <fileName>. */
-            writeFile(
-                tmpDir + "/" + fileName,
-                store.makeValidityRegistration(store.exportReferences(storePaths, inputPaths), false, false));
-        }
-    }
-
     prepareSandbox();
 
     if (needsHashRewrite() && pathExists(homeDir))
@@ -1033,6 +1016,11 @@ void DerivationBuilderImpl::initEnv()
     /* The maximum number of cores to utilize for parallel building. */
     env["NIX_BUILD_CORES"] = fmt("%d", settings.buildCores ? settings.buildCores : settings.getDefaultCores());
 
+    auto writeEnv = [&](const std::string & envVarName, const std::string & fileName, const std::string & value) {
+        writeBuilderFile(fileName, rewriteStrings(value, inputRewrites));
+        env[envVarName] = tmpDirInSandbox() + "/" + fileName;
+    };
+
     /* In non-structured mode, set all bindings either directory in the
        environment or via a file, as specified by
        `DerivationOptions::passAsFile`. */
@@ -1043,12 +1031,26 @@ void DerivationBuilderImpl::initEnv()
             } else {
                 auto hash = hashString(HashAlgorithm::SHA256, i.first);
                 std::string fn = ".attr-" + hash.to_string(HashFormat::Nix32, false);
-                writeBuilderFile(fn, rewriteStrings(i.second, inputRewrites));
-                env[i.first + "Path"] = tmpDirInSandbox() + "/" + fn;
+                writeEnv(i.first + "Path", fn, i.second);
             }
         }
     }
 
+    /* Do this with or without structured attrs --- actually, this is
+       used to desugar structured attrs. */
+    for (const auto & [name, info] : extraEnv) {
+        if (info.nameOfPassAsFile) {
+            writeEnv(name, *info.nameOfPassAsFile, info.value);
+        } else {
+            env[name] = info.value;
+        }
+    }
+
+    /* Add extra files, analogous to `extraEnv` */
+    for (const auto & [fileName, value] : extraFiles) {
+        writeBuilderFile(fileName, value);
+    }
+
     /* For convenience, set an environment pointing to the top build
        directory. */
     env["NIX_BUILD_TOP"] = tmpDirInSandbox();
@@ -1102,20 +1104,6 @@ void DerivationBuilderImpl::initEnv()
     env["TERM"] = "xterm-256color";
 }
 
-void DerivationBuilderImpl::writeStructuredAttrs()
-{
-    if (drv.structuredAttrs) {
-        auto json = drv.structuredAttrs->prepareStructuredAttrs(store, drvOptions, inputPaths, drv.outputs);
-
-        auto jsonSh = StructuredAttrs::writeShell(json);
-
-        writeBuilderFile(".attrs.sh", rewriteStrings(jsonSh, inputRewrites));
-        env["NIX_ATTRS_SH_FILE"] = tmpDirInSandbox() + "/.attrs.sh";
-        writeBuilderFile(".attrs.json", rewriteStrings(json.dump(), inputRewrites));
-        env["NIX_ATTRS_JSON_FILE"] = tmpDirInSandbox() + "/.attrs.json";
-    }
-}
-
 void DerivationBuilderImpl::startDaemon()
 {
     experimentalFeatureSettings.require(Xp::RecursiveNix);