Fix ca/eval-store.sh test

The refactor in the last commit fixed the bug it was supposed to fix,
but introduced a new bug in that sometimes we tried to write a resolved
derivation to a store before all its `inputSrcs` were in that store.

The solution is to defer writing the derivation until inside
`DerivationBuildingGoal`, just before we do an actual build. At this
point, we are sure that all inputs in are the store.

This does have the side effect of meaning we don't write down the
resolved derivation in the substituting case, only the building case,
but I think that is actually fine. The store that actually does the
building should make a record of what it built by storing the resolved
derivation. Other stores that just substitute from that store don't
necessary want that derivation however. They can trust the substituter
to keep the record around, or baring that, they can attempt to re
resolve everything, if they need to be audited.

Author: Ericson2314

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

@@ -26,8 +26,8 @@
 namespace nix {
 
 DerivationBuildingGoal::DerivationBuildingGoal(
-    const StorePath & drvPath, const Derivation & drv_, Worker & worker, BuildMode buildMode)
-    : Goal(worker, gaveUpOnSubstitution())
+    const StorePath & drvPath, const Derivation & drv_, Worker & worker, BuildMode buildMode, bool storeDerivation)
+    : Goal(worker, gaveUpOnSubstitution(storeDerivation))
     , drvPath(drvPath)
     , buildMode(buildMode)
 {
@@ -124,7 +124,7 @@ static void runPostBuildHook(
 
 /* At least one of the output paths could not be
    produced using a substitute.  So we have to build instead. */
-Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution()
+Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution(bool storeDerivation)
 {
     Goals waitees;
 
@@ -172,6 +172,13 @@ Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution()
 
     /* Determine the full set of input paths. */
 
+    if (storeDerivation) {
+        assert(drv->inputDrvs.map.empty());
+        /* Store the resolved derivation, as part of the record of
+           what we're actually building */
+        writeDerivation(worker.store, *drv);
+    }
+
     {
         /* If we get this far, we know no dynamic drvs inputs */
 

src/libstore/build/derivation-goal.cc

@@ -30,8 +30,9 @@ DerivationGoal::DerivationGoal(
     const Derivation & drv,
     const OutputName & wantedOutput,
     Worker & worker,
-    BuildMode buildMode)
-    : Goal(worker, haveDerivation())
+    BuildMode buildMode,
+    bool storeDerivation)
+    : Goal(worker, haveDerivation(storeDerivation))
     , drvPath(drvPath)
     , wantedOutput(wantedOutput)
     , outputHash{[&] {
@@ -65,7 +66,7 @@ std::string DerivationGoal::key()
     }.to_string(worker.store);
 }
 
-Goal::Co DerivationGoal::haveDerivation()
+Goal::Co DerivationGoal::haveDerivation(bool storeDerivation)
 {
     trace("have derivation");
 
@@ -159,11 +160,8 @@ Goal::Co DerivationGoal::haveDerivation()
     if (resolutionGoal->resolvedDrv) {
         auto & [pathResolved, drvResolved] = *resolutionGoal->resolvedDrv;
 
-        /* Store the resolved derivation, as part of the record of
-           what we're actually building */
-        writeDerivation(worker.store, drvResolved);
-
-        auto resolvedDrvGoal = worker.makeDerivationGoal(pathResolved, drvResolved, wantedOutput, buildMode);
+        auto resolvedDrvGoal =
+            worker.makeDerivationGoal(pathResolved, drvResolved, wantedOutput, buildMode, /*storeDerivation=*/true);
         {
             Goals waitees{resolvedDrvGoal};
             co_await await(std::move(waitees));
@@ -239,7 +237,7 @@ Goal::Co DerivationGoal::haveDerivation()
 
     /* Give up on substitution for the output we want, actually build this derivation */
 
-    auto g = worker.makeDerivationBuildingGoal(drvPath, *drv, buildMode);
+    auto g = worker.makeDerivationBuildingGoal(drvPath, *drv, buildMode, storeDerivation);
 
     /* We will finish with it ourselves, as if we were the derivational goal. */
     g->preserveException = true;

src/libstore/build/worker.cc

@@ -76,9 +76,14 @@ std::shared_ptr<DerivationTrampolineGoal> Worker::makeDerivationTrampolineGoal(
 }
 
 std::shared_ptr<DerivationGoal> Worker::makeDerivationGoal(
-    const StorePath & drvPath, const Derivation & drv, const OutputName & wantedOutput, BuildMode buildMode)
+    const StorePath & drvPath,
+    const Derivation & drv,
+    const OutputName & wantedOutput,
+    BuildMode buildMode,
+    bool storeDerivation)
 {
-    return initGoalIfNeeded(derivationGoals[drvPath][wantedOutput], drvPath, drv, wantedOutput, *this, buildMode);
+    return initGoalIfNeeded(
+        derivationGoals[drvPath][wantedOutput], drvPath, drv, wantedOutput, *this, buildMode, storeDerivation);
 }
 
 std::shared_ptr<DerivationResolutionGoal>
@@ -87,10 +92,10 @@ Worker::makeDerivationResolutionGoal(const StorePath & drvPath, const Derivation
     return initGoalIfNeeded(derivationResolutionGoals[drvPath], drvPath, drv, *this, buildMode);
 }
 
-std::shared_ptr<DerivationBuildingGoal>
-Worker::makeDerivationBuildingGoal(const StorePath & drvPath, const Derivation & drv, BuildMode buildMode)
+std::shared_ptr<DerivationBuildingGoal> Worker::makeDerivationBuildingGoal(
+    const StorePath & drvPath, const Derivation & drv, BuildMode buildMode, bool storeDerivation)
 {
-    return initGoalIfNeeded(derivationBuildingGoals[drvPath], drvPath, drv, *this, buildMode);
+    return initGoalIfNeeded(derivationBuildingGoals[drvPath], drvPath, drv, *this, buildMode, storeDerivation);
 }
 
 std::shared_ptr<PathSubstitutionGoal>

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

@@ -29,8 +29,21 @@ typedef enum { rpAccept, rpDecline, rpPostpone } HookReply;
  */
 struct DerivationBuildingGoal : public Goal
 {
+    /**
+     * @param storeDerivation Whether to store the derivation in
+     * `worker.store`. This is useful for newly-resolved derivations. In this
+     * case, the derivation was not created a priori, e.g. purely (or close
+     * enough) from evaluation of the Nix language, but also depends on the
+     * exact content produced by upstream builds. It is strongly advised to
+     * have a permanent record of such a resolved derivation in order to
+     * faithfully reconstruct the build history.
+     */
     DerivationBuildingGoal(
-        const StorePath & drvPath, const Derivation & drv, Worker & worker, BuildMode buildMode = bmNormal);
+        const StorePath & drvPath,
+        const Derivation & drv,
+        Worker & worker,
+        BuildMode buildMode = bmNormal,
+        bool storeDerivation = false);
     ~DerivationBuildingGoal();
 
 private:
@@ -100,7 +113,7 @@ private:
     /**
      * The states.
      */
-    Co gaveUpOnSubstitution();
+    Co gaveUpOnSubstitution(bool storeDerivation);
     Co tryToBuild();
 
     /**

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

@@ -40,12 +40,16 @@ struct DerivationGoal : public Goal
      */
     OutputName wantedOutput;
 
+    /**
+     * @param storeDerivation See `DerivationBuildingGoal`. This is just passed along.
+     */
     DerivationGoal(
         const StorePath & drvPath,
         const Derivation & drv,
         const OutputName & wantedOutput,
         Worker & worker,
-        BuildMode buildMode = bmNormal);
+        BuildMode buildMode = bmNormal,
+        bool storeDerivation = false);
     ~DerivationGoal() = default;
 
     void timedOut(Error && ex) override
@@ -80,7 +84,7 @@ private:
     /**
      * The states.
      */
-    Co haveDerivation();
+    Co haveDerivation(bool storeDerivation);
 
     /**
      * Return `std::nullopt` if the output is unknown, e.g. un unbuilt

src/libstore/include/nix/store/build/worker.hh

@@ -223,7 +223,8 @@ public:
         const StorePath & drvPath,
         const Derivation & drv,
         const OutputName & wantedOutput,
-        BuildMode buildMode = bmNormal);
+        BuildMode buildMode = bmNormal,
+        bool storeDerivation = false);
 
     /**
      * @ref DerivationResolutionGoal "derivation resolution goal"
@@ -234,8 +235,11 @@ public:
     /**
      * @ref DerivationBuildingGoal "derivation building goal"
      */
-    std::shared_ptr<DerivationBuildingGoal>
-    makeDerivationBuildingGoal(const StorePath & drvPath, const Derivation & drv, BuildMode buildMode = bmNormal);
+    std::shared_ptr<DerivationBuildingGoal> makeDerivationBuildingGoal(
+        const StorePath & drvPath,
+        const Derivation & drv,
+        BuildMode buildMode = bmNormal,
+        bool storeDerivation = false);
 
     /**
      * @ref PathSubstitutionGoal "substitution goal"