Limit ParsedDerivation just to the derivation's environment

This moves us towards getting rid of `ParsedDerivation` and just having `DerivationOptions`. Co-Authored-By: HaeNoe <[email protected]>
NixOS · Feb 3, 2025 · 38b981a · 38b981a
1 parent f9a50b2
commit 38b981a
Show file tree

Hide file tree

Showing 8 changed files with 44 additions and 30 deletions.
diff --git a/src/libstore-tests/derivation-advanced-attrs.cc b/src/libstore-tests/derivation-advanced-attrs.cc
@@ -81,7 +81,7 @@ TEST_F(DerivationAdvancedAttrsTest, Derivation_advancedAttributes_defaults)
 
         auto drvPath = writeDerivation(*store, got, NoRepair, true);
 
-        ParsedDerivation parsedDrv(got);
+        ParsedDerivation parsedDrv(got.env);
         DerivationOptions options = DerivationOptions::fromParsedDerivation(*store, parsedDrv);
 
         EXPECT_TRUE(!parsedDrv.hasStructuredAttrs());
@@ -116,7 +116,7 @@ TEST_F(DerivationAdvancedAttrsTest, Derivation_advancedAttributes)
 
         auto drvPath = writeDerivation(*store, got, NoRepair, true);
 
-        ParsedDerivation parsedDrv(got);
+        ParsedDerivation parsedDrv(got.env);
         DerivationOptions options = DerivationOptions::fromParsedDerivation(*store, parsedDrv);
 
         StringSet systemFeatures{"rainbow", "uid-range"};
@@ -157,7 +157,7 @@ TEST_F(DerivationAdvancedAttrsTest, Derivation_advancedAttributes_structuredAttr
 
         auto drvPath = writeDerivation(*store, got, NoRepair, true);
 
-        ParsedDerivation parsedDrv(got);
+        ParsedDerivation parsedDrv(got.env);
         DerivationOptions options = DerivationOptions::fromParsedDerivation(*store, parsedDrv);
 
         EXPECT_TRUE(parsedDrv.hasStructuredAttrs());
@@ -191,7 +191,7 @@ TEST_F(DerivationAdvancedAttrsTest, Derivation_advancedAttributes_structuredAttr
 
         auto drvPath = writeDerivation(*store, got, NoRepair, true);
 
-        ParsedDerivation parsedDrv(got);
+        ParsedDerivation parsedDrv(got.env);
         DerivationOptions options = DerivationOptions::fromParsedDerivation(*store, parsedDrv);
 
         StringSet systemFeatures{"rainbow", "uid-range"};

diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc
@@ -194,7 +194,7 @@ Goal::Co DerivationGoal::haveDerivation()
 {
     trace("have derivation");
 
-    parsedDrv = std::make_unique<ParsedDerivation>(*drv);
+    parsedDrv = std::make_unique<ParsedDerivation>(drv->env);
     try {
         drvOptions = std::make_unique<DerivationOptions>(
             DerivationOptions::fromParsedDerivation(worker.store, *parsedDrv));

diff --git a/src/libstore/derivation-options.cc b/src/libstore/derivation-options.cc
@@ -117,7 +117,7 @@ DerivationOptions::fromParsedDerivation(const StoreDirConfig & store, const Pars
         .passAsFile =
             [&] {
                 StringSet res;
-                if (auto * passAsFileString = get(parsed.drv.env, "passAsFile")) {
+                if (auto * passAsFileString = get(parsed.env, "passAsFile")) {
                     if (parsed.hasStructuredAttrs()) {
                         if (shouldWarn) {
                             warn(
@@ -144,7 +144,7 @@ DerivationOptions::fromParsedDerivation(const StoreDirConfig & store, const Pars
                         ret.insert_or_assign(key, std::move(storePaths));
                     }
                 } else {
-                    auto s = getOr(parsed.drv.env, "exportReferencesGraph", "");
+                    auto s = getOr(parsed.env, "exportReferencesGraph", "");
                     Strings ss = tokenizeString<Strings>(s);
                     if (ss.size() % 2 != 0)
                         throw BuildError("odd number of tokens in 'exportReferencesGraph': '%1%'", s);

diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc
@@ -222,7 +222,7 @@ void Store::queryMissing(const std::vector<DerivedPath> & targets,
             if (knownOutputPaths && invalid.empty()) return;
 
             auto drv = make_ref<Derivation>(derivationFromPath(drvPath));
-            ParsedDerivation parsedDrv(*drv);
+            ParsedDerivation parsedDrv(drv->env);
             DerivationOptions drvOptions;
             try {
                 drvOptions = DerivationOptions::fromParsedDerivation(*this, parsedDrv);

diff --git a/src/libstore/parsed-derivations.cc b/src/libstore/parsed-derivations.cc
@@ -6,12 +6,12 @@
 
 namespace nix {
 
-ParsedDerivation::ParsedDerivation(BasicDerivation & drv)
-    : drv(drv)
+ParsedDerivation::ParsedDerivation(const StringPairs & env)
+    : env(env)
 {
     /* Parse the __json attribute, if any. */
-    auto jsonAttr = drv.env.find("__json");
-    if (jsonAttr != drv.env.end()) {
+    auto jsonAttr = env.find("__json");
+    if (jsonAttr != env.end()) {
         try {
             structuredAttrs = std::make_unique<nlohmann::json>(nlohmann::json::parse(jsonAttr->second));
         } catch (std::exception & e) {
@@ -34,8 +34,8 @@ std::optional<std::string> ParsedDerivation::getStringAttr(const std::string & n
             return i->get<std::string>();
         }
     } else {
-        auto i = drv.env.find(name);
-        if (i == drv.env.end())
+        auto i = env.find(name);
+        if (i == env.end())
             return {};
         else
             return i->second;
@@ -54,8 +54,8 @@ bool ParsedDerivation::getBoolAttr(const std::string & name, bool def) const
             return i->get<bool>();
         }
     } else {
-        auto i = drv.env.find(name);
-        if (i == drv.env.end())
+        auto i = env.find(name);
+        if (i == env.end())
             return def;
         else
             return i->second == "1";
@@ -80,8 +80,8 @@ std::optional<Strings> ParsedDerivation::getStringsAttr(const std::string & name
             return res;
         }
     } else {
-        auto i = drv.env.find(name);
-        if (i == drv.env.end())
+        auto i = env.find(name);
+        if (i == env.end())
             return {};
         else
             return tokenizeString<Strings>(i->second);
@@ -155,17 +155,18 @@ static nlohmann::json pathInfoToJSON(
 std::optional<nlohmann::json> ParsedDerivation::prepareStructuredAttrs(
     Store & store,
     const DerivationOptions & drvOptions,
-    const StorePathSet & inputPaths)
+    const StorePathSet & inputPaths,
+    const DerivationOutputs & outputs)
 {
     if (!structuredAttrs) return std::nullopt;
 
     auto json = *structuredAttrs;
 
     /* Add an "outputs" object containing the output paths. */
-    nlohmann::json outputs;
-    for (auto & i : drv.outputs)
-        outputs[i.first] = hashPlaceholder(i.first);
-    json["outputs"] = outputs;
+    nlohmann::json outputsJson;
+    for (auto & i : outputs)
+        outputsJson[i.first] = hashPlaceholder(i.first);
+    json["outputs"] = std::move(outputsJson);
 
     /* Handle exportReferencesGraph. */
     for (auto & [key, storePaths] : drvOptions.exportReferencesGraph) {

diff --git a/src/libstore/parsed-derivations.hh b/src/libstore/parsed-derivations.hh
@@ -12,7 +12,7 @@ struct DerivationOptions;
 
 class ParsedDerivation
 {
-    BasicDerivation & drv;
+    const StringPairs & env;
     std::unique_ptr<nlohmann::json> structuredAttrs;
 
     std::optional<std::string> getStringAttr(const std::string & name) const;
@@ -33,7 +33,7 @@ class ParsedDerivation
 
 public:
 
-    ParsedDerivation(BasicDerivation & drv);
+    ParsedDerivation(const StringPairs & env);
 
     ~ParsedDerivation();
 
@@ -42,8 +42,11 @@ public:
         return static_cast<bool>(structuredAttrs);
     }
 
-    std::optional<nlohmann::json>
-    prepareStructuredAttrs(Store & store, const DerivationOptions & drvOptions, const StorePathSet & inputPaths);
+    std::optional<nlohmann::json> prepareStructuredAttrs(
+        Store & store,
+        const DerivationOptions & drvOptions,
+        const StorePathSet & inputPaths,
+        const DerivationOutputs & outputs);
 };
 
 std::string writeStructuredAttrsShell(const nlohmann::json & json);

diff --git a/src/libstore/unix/build/local-derivation-goal.cc b/src/libstore/unix/build/local-derivation-goal.cc
@@ -1218,7 +1218,12 @@ void LocalDerivationGoal::initEnv()
 
 void LocalDerivationGoal::writeStructuredAttrs()
 {
-    if (auto structAttrsJson = parsedDrv->prepareStructuredAttrs(worker.store, *drvOptions, inputPaths)) {
+    if (auto structAttrsJson = parsedDrv->prepareStructuredAttrs(
+            worker.store,
+            *drvOptions,
+            inputPaths,
+            drv->outputs))
+    {
         auto json = structAttrsJson.value();
         nlohmann::json rewritten;
         for (auto & [i, v] : json["outputs"].get<nlohmann::json::object_t>()) {

diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc
@@ -543,7 +543,7 @@ static void main_nix_build(int argc, char * * argv)
         env["NIX_STORE"] = store->storeDir;
         env["NIX_BUILD_CORES"] = std::to_string(settings.buildCores);
 
-        ParsedDerivation parsedDrv(drv);
+        ParsedDerivation parsedDrv(drv.env);
         DerivationOptions drvOptions;
         try {
             drvOptions = DerivationOptions::fromParsedDerivation(*store, parsedDrv);
@@ -583,7 +583,12 @@ static void main_nix_build(int argc, char * * argv)
             for (const auto & [inputDrv, inputNode] : drv.inputDrvs.map)
                 accumInputClosure(inputDrv, inputNode);
 
-            if (auto structAttrs = parsedDrv.prepareStructuredAttrs(*store, drvOptions, inputs)) {
+            if (auto structAttrs = parsedDrv.prepareStructuredAttrs(
+                    *store,
+                    drvOptions,
+                    inputs,
+                    drv.outputs))
+            {
                 auto json = structAttrs.value();
                 structuredAttrsRC = writeStructuredAttrsShell(json);