Merge pull request #13750 from obsidiansystems/simplify-derivation-goal-0

Simplify `DerivationGoal` in two ways

Author: xokdvium

src/libstore/build/derivation-goal.cc

@@ -241,7 +241,17 @@ Goal::Co DerivationGoal::repairClosure()
        that produced those outputs. */
 
     /* Get the output closure. */
-    auto outputs = queryDerivationOutputMap();
+    auto outputs = [&] {
+        for (auto * drvStore : {&worker.evalStore, &worker.store})
+            if (drvStore->isValidPath(drvPath))
+                return worker.store.queryDerivationOutputMap(drvPath, drvStore);
+
+        OutputPathMap res;
+        for (auto & [name, output] : drv->outputsAndOptPaths(worker.store))
+            res.insert_or_assign(name, *output.second);
+        return res;
+    }();
+
     StorePathSet outputClosure;
     if (auto mPath = get(outputs, wantedOutput)) {
         worker.store.computeFSClosure(*mPath, outputClosure);
@@ -304,37 +314,6 @@ Goal::Co DerivationGoal::repairClosure()
     co_return done(BuildResult::AlreadyValid, assertPathValidity());
 }
 
-std::map<std::string, std::optional<StorePath>> DerivationGoal::queryPartialDerivationOutputMap()
-{
-    assert(!drv->type().isImpure());
-
-    for (auto * drvStore : {&worker.evalStore, &worker.store})
-        if (drvStore->isValidPath(drvPath))
-            return worker.store.queryPartialDerivationOutputMap(drvPath, drvStore);
-
-    /* In-memory derivation will naturally fall back on this case, where
-       we do best-effort with static information. */
-    std::map<std::string, std::optional<StorePath>> res;
-    for (auto & [name, output] : drv->outputs)
-        res.insert_or_assign(name, output.path(worker.store, drv->name, name));
-    return res;
-}
-
-OutputPathMap DerivationGoal::queryDerivationOutputMap()
-{
-    assert(!drv->type().isImpure());
-
-    for (auto * drvStore : {&worker.evalStore, &worker.store})
-        if (drvStore->isValidPath(drvPath))
-            return worker.store.queryDerivationOutputMap(drvPath, drvStore);
-
-    // See comment in `DerivationGoal::queryPartialDerivationOutputMap`.
-    OutputPathMap res;
-    for (auto & [name, output] : drv->outputsAndOptPaths(worker.store))
-        res.insert_or_assign(name, *output.second);
-    return res;
-}
-
 std::pair<bool, SingleDrvOutputs> DerivationGoal::checkPathValidity()
 {
     if (drv->type().isImpure())
@@ -344,7 +323,20 @@ std::pair<bool, SingleDrvOutputs> DerivationGoal::checkPathValidity()
     StringSet wantedOutputsLeft{wantedOutput};
     SingleDrvOutputs validOutputs;
 
-    for (auto & i : queryPartialDerivationOutputMap()) {
+    auto partialDerivationOutputMap = [&] {
+        for (auto * drvStore : {&worker.evalStore, &worker.store})
+            if (drvStore->isValidPath(drvPath))
+                return worker.store.queryPartialDerivationOutputMap(drvPath, drvStore);
+
+        /* In-memory derivation will naturally fall back on this case, where
+           we do best-effort with static information. */
+        std::map<std::string, std::optional<StorePath>> res;
+        for (auto & [name, output] : drv->outputs)
+            res.insert_or_assign(name, output.path(worker.store, drv->name, name));
+        return res;
+    }();
+
+    for (auto & i : partialDerivationOutputMap) {
         auto initialOutput = get(initialOutputs, i.first);
         if (!initialOutput)
             // this is an invalid output, gets caught with (!wantedOutputsLeft.empty())

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

@@ -43,21 +43,6 @@ struct DerivationGoal : public Goal
      */
     OutputName wantedOutput;
 
-    /**
-     * The derivation stored at drvPath.
-     */
-    std::unique_ptr<Derivation> drv;
-
-    /**
-     * The remainder is state held during the build.
-     */
-
-    std::map<std::string, InitialOutput> initialOutputs;
-
-    BuildMode buildMode;
-
-    std::unique_ptr<MaintainCount<uint64_t>> mcExpectedBuilds;
-
     DerivationGoal(
         const StorePath & drvPath,
         const Derivation & drv,
@@ -73,18 +58,32 @@ struct DerivationGoal : public Goal
 
     std::string key() override;
 
+    JobCategory jobCategory() const override
+    {
+        return JobCategory::Administration;
+    };
+
+private:
+
     /**
-     * The states.
+     * The derivation stored at drvPath.
      */
-    Co haveDerivation();
+    std::unique_ptr<Derivation> drv;
 
     /**
-     * Wrappers around the corresponding Store methods that first consult the
-     * derivation.  This is currently needed because when there is no drv file
-     * there also is no DB entry.
+     * The remainder is state held during the build.
      */
-    std::map<std::string, std::optional<StorePath>> queryPartialDerivationOutputMap();
-    OutputPathMap queryDerivationOutputMap();
+
+    std::map<std::string, InitialOutput> initialOutputs;
+
+    BuildMode buildMode;
+
+    std::unique_ptr<MaintainCount<uint64_t>> mcExpectedBuilds;
+
+    /**
+     * The states.
+     */
+    Co haveDerivation();
 
     /**
      * Update 'initialOutputs' to determine the current status of the
@@ -103,11 +102,6 @@ struct DerivationGoal : public Goal
     Co repairClosure();
 
     Done done(BuildResult::Status status, SingleDrvOutputs builtOutputs = {}, std::optional<Error> ex = {});
-
-    JobCategory jobCategory() const override
-    {
-        return JobCategory::Administration;
-    };
 };
 
 } // namespace nix