From 018d6462def78e6f1b940d497ffaa3828831af03 Mon Sep 17 00:00:00 2001
From: John Ericson <John.Ericson@Obsidian.Systems>
Date: Tue, 14 Oct 2025 18:10:27 -0400
Subject: [PATCH] `DrvOutputSubstitutionGoal`: Don't actually fetch any store
 objects

We now have a nice separation of concerns: `DrvOutputSubstitutionGoal`
is *just* for getting realisations, and `PathSubstitutionGoal` is just
for fetching store objects.

The fetching of store objects that this used to do is now moved to the
caller.
---
 src/libstore/build/derivation-goal.cc         | 22 +++++++++++++++---
 .../build/drv-output-substitution-goal.cc     | 23 +------------------
 .../build/drv-output-substitution-goal.hh     | 19 +++++++++++----
 3 files changed, 34 insertions(+), 30 deletions(-)

diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc
index 13ffe51f3..bdbca5f00 100644
--- a/src/libstore/build/derivation-goal.cc
+++ b/src/libstore/build/derivation-goal.cc
@@ -1,4 +1,5 @@
 #include "nix/store/build/derivation-goal.hh"
+#include "nix/store/build/drv-output-substitution-goal.hh"
 #include "nix/store/build/derivation-building-goal.hh"
 #include "nix/store/build/derivation-resolution-goal.hh"
 #ifndef _WIN32 // TODO enable build hook on Windows
@@ -100,9 +101,24 @@ Goal::Co DerivationGoal::haveDerivation(bool storeDerivation)
            through substitutes.  If that doesn't work, we'll build
            them. */
         if (settings.useSubstitutes && drvOptions.substitutesAllowed()) {
-            if (!checkResult)
-                waitees.insert(upcast_goal(worker.makeDrvOutputSubstitutionGoal(DrvOutput{outputHash, wantedOutput})));
-            else {
+            if (!checkResult) {
+                DrvOutput id{outputHash, wantedOutput};
+                auto g = worker.makeDrvOutputSubstitutionGoal(id);
+                waitees.insert(g);
+                co_await await(std::move(waitees));
+
+                if (nrFailed == 0) {
+                    waitees.insert(upcast_goal(worker.makePathSubstitutionGoal(g->outputInfo->outPath)));
+                    co_await await(std::move(waitees));
+
+                    trace("output path substituted");
+
+                    if (nrFailed == 0)
+                        worker.store.registerDrvOutput({*g->outputInfo, id});
+                    else
+                        debug("The output path of the derivation output '%s' could not be substituted", id.to_string());
+                }
+            } else {
                 auto * cap = getDerivationCA(*drv);
                 waitees.insert(upcast_goal(worker.makePathSubstitutionGoal(
                     checkResult->first.outPath,
diff --git a/src/libstore/build/drv-output-substitution-goal.cc b/src/libstore/build/drv-output-substitution-goal.cc
index 59a08b8a6..71f0ae1e7 100644
--- a/src/libstore/build/drv-output-substitution-goal.cc
+++ b/src/libstore/build/drv-output-substitution-goal.cc
@@ -19,7 +19,7 @@ Goal::Co DrvOutputSubstitutionGoal::init()
     trace("init");
 
     /* If the derivation already exists, we’re done */
-    if (worker.store.queryRealisation(id)) {
+    if ((outputInfo = worker.store.queryRealisation(id))) {
         co_return amDone(ecSuccess);
     }
 
@@ -77,12 +77,6 @@ Goal::Co DrvOutputSubstitutionGoal::init()
 
         worker.childTerminated(this);
 
-        /*
-         * The realisation corresponding to the given output id.
-         * Will be filled once we can get it.
-         */
-        std::shared_ptr<const UnkeyedRealisation> outputInfo;
-
         try {
             outputInfo = promise->get_future().get();
         } catch (std::exception & e) {
@@ -93,21 +87,6 @@ Goal::Co DrvOutputSubstitutionGoal::init()
         if (!outputInfo)
             continue;
 
-        Goals waitees;
-
-        waitees.insert(worker.makePathSubstitutionGoal(outputInfo->outPath));
-
-        co_await await(std::move(waitees));
-
-        trace("output path substituted");
-
-        if (nrFailed > 0) {
-            debug("The output path of the derivation output '%s' could not be substituted", id.to_string());
-            co_return amDone(nrNoSubstituters > 0 ? ecNoSubstituters : ecFailed);
-        }
-
-        worker.store.registerDrvOutput({*outputInfo, id});
-
         trace("finished");
         co_return amDone(ecSuccess);
     }
diff --git a/src/libstore/include/nix/store/build/drv-output-substitution-goal.hh b/src/libstore/include/nix/store/build/drv-output-substitution-goal.hh
index 2d42169dd..5f36bbf06 100644
--- a/src/libstore/include/nix/store/build/drv-output-substitution-goal.hh
+++ b/src/libstore/include/nix/store/build/drv-output-substitution-goal.hh
@@ -14,11 +14,14 @@ namespace nix {
 class Worker;
 
 /**
- * Substitution of a derivation output.
- * This is done in three steps:
- * 1. Fetch the output info from a substituter
- * 2. Substitute the corresponding output path
- * 3. Register the output info
+ * Fetch a `Realisation` (drv ⨯ output name -> output path) from a
+ * substituter.
+ *
+ * If the output store object itself should also be substituted, that is
+ * the responsibility of the caller to do so.
+ *
+ * @todo rename this `BuidlTraceEntryGoal`, which will make sense
+ * especially once `Realisation` is renamed to `BuildTraceEntry`.
  */
 class DrvOutputSubstitutionGoal : public Goal
 {
@@ -31,6 +34,12 @@ class DrvOutputSubstitutionGoal : public Goal
 public:
     DrvOutputSubstitutionGoal(const DrvOutput & id, Worker & worker);
 
+    /**
+     * The realisation corresponding to the given output id.
+     * Will be filled once we can get it.
+     */
+    std::shared_ptr<const UnkeyedRealisation> outputInfo;
+
     Co init();
 
     std::string key() override;