Revert "Revert "Adapt scheduler to work with dynamic derivations""

This fixes dynamic derivations, reverting #9081.

I believe that this time around, #9052 is fixed. When I first rebased
this, tests were failing (which wasn't the case before). The cause of
those test failures were due to the crude job in which the outer goal
tried to exit with the inner goal's status.

Now, that error handling has been reworked to be more faithful. The exit
exit status and exception of the inner goal is returned by the outer
goal. The exception was what was causing the test failures, but I
believe it was not having the right error code (there is more than one
for failure) that caused #9081.

The only cost of doing things the "right way" was that I had to
introduce a hacky `preserveException` boolean. I don't like this, but,
then again, none of us like anything about how the scheduler works.
Issue #11927 is still there to clean everything up, subsuming the need
for any `preserveException` because I doubt we will be fishing
information out of state machines like this at all.

This reverts commit 8440afb.

src/libstore/build/create-derivation-and-realise-goal.cc

@@ -0,0 +1,126 @@
+#include "create-derivation-and-realise-goal.hh"
+#include "worker.hh"
+
+namespace nix {
+
+CreateDerivationAndRealiseGoal::CreateDerivationAndRealiseGoal(
+    ref<SingleDerivedPath> drvReq, const OutputsSpec & wantedOutputs, Worker & worker, BuildMode buildMode)
+    : Goal(worker, DerivedPath::Built{.drvPath = drvReq, .outputs = wantedOutputs})
+    , drvReq(drvReq)
+    , wantedOutputs(wantedOutputs)
+    , buildMode(buildMode)
+{
+    name =
+        fmt("outer obtaining drv from '%s' and then building outputs %s",
+            drvReq->to_string(worker.store),
+            std::visit(
+                overloaded{
+                    [&](const OutputsSpec::All) -> std::string { return "* (all of them)"; },
+                    [&](const OutputsSpec::Names os) { return concatStringsSep(", ", quoteStrings(os)); },
+                },
+                wantedOutputs.raw));
+    trace("created outer");
+
+    worker.updateProgress();
+}
+
+CreateDerivationAndRealiseGoal::~CreateDerivationAndRealiseGoal() {}
+
+static StorePath pathPartOfReq(const SingleDerivedPath & req)
+{
+    return std::visit(
+        overloaded{
+            [&](const SingleDerivedPath::Opaque & bo) { return bo.path; },
+            [&](const SingleDerivedPath::Built & bfd) { return pathPartOfReq(*bfd.drvPath); },
+        },
+        req.raw());
+}
+
+std::string CreateDerivationAndRealiseGoal::key()
+{
+    /* Ensure that derivations get built in order of their name,
+       i.e. a derivation named "aardvark" always comes before "baboon". And
+       substitution goals and inner derivation goals always happen before
+       derivation goals (due to "b$"). */
+    return "c$" + std::string(pathPartOfReq(*drvReq).name()) + "$" + drvReq->to_string(worker.store);
+}
+
+void CreateDerivationAndRealiseGoal::timedOut(Error && ex) {}
+
+void CreateDerivationAndRealiseGoal::addWantedOutputs(const OutputsSpec & outputs)
+{
+    /* If we already want all outputs, there is nothing to do. */
+    auto newWanted = wantedOutputs.union_(outputs);
+    bool needRestart = !newWanted.isSubsetOf(wantedOutputs);
+    wantedOutputs = newWanted;
+
+    if (!needRestart)
+        return;
+
+    if (!optDrvPath)
+        // haven't started steps where the outputs matter yet
+        return;
+    worker.makeDerivationGoal(*optDrvPath, outputs, buildMode);
+}
+
+Goal::Co CreateDerivationAndRealiseGoal::init()
+{
+    trace("outer init");
+
+    /* The first thing to do is to make sure that the derivation
+       exists.  If it doesn't, it may be created through a
+       substitute. */
+    if (auto optDrvPath = [this]() -> std::optional<StorePath> {
+            if (buildMode != bmNormal)
+                return std::nullopt;
+
+            auto drvPath = StorePath::dummy;
+            try {
+                drvPath = resolveDerivedPath(worker.store, *drvReq);
+            } catch (MissingRealisation &) {
+                return std::nullopt;
+            }
+            auto cond = worker.evalStore.isValidPath(drvPath) || worker.store.isValidPath(drvPath);
+            return cond ? std::optional{drvPath} : std::nullopt;
+        }()) {
+        trace(
+            fmt("already have drv '%s' for '%s', can go straight to building",
+                worker.store.printStorePath(*optDrvPath),
+                drvReq->to_string(worker.store)));
+    } else {
+        trace("need to obtain drv we want to build");
+        addWaitee(worker.makeGoal(DerivedPath::fromSingle(*drvReq)));
+        co_await Suspend{};
+    }
+
+    trace("outer load and build derivation");
+
+    if (nrFailed != 0) {
+        co_return amDone(ecFailed, Error("cannot build missing derivation '%s'", drvReq->to_string(worker.store)));
+    }
+
+    StorePath drvPath = resolveDerivedPath(worker.store, *drvReq);
+    /* Build this step! */
+    concreteDrvGoal = worker.makeDerivationGoal(drvPath, wantedOutputs, buildMode);
+    {
+        auto g = upcast_goal(concreteDrvGoal);
+        /* We will finish with it ourselves, as if we were the derivational goal. */
+        g->preserveException = true;
+    }
+    optDrvPath = std::move(drvPath);
+    addWaitee(upcast_goal(concreteDrvGoal));
+    co_await Suspend{};
+
+    trace("outer build done");
+
+    buildResult = upcast_goal(concreteDrvGoal)
+                      ->getBuildResult(DerivedPath::Built{
+                          .drvPath = drvReq,
+                          .outputs = wantedOutputs,
+                      });
+
+    auto g = upcast_goal(concreteDrvGoal);
+    co_return amDone(g->exitCode, g->ex);
+}
+
+}

src/libstore/build/create-derivation-and-realise-goal.hh

@@ -0,0 +1,89 @@
+#pragma once
+
+#include "parsed-derivations.hh"
+#include "user-lock.hh"
+#include "store-api.hh"
+#include "pathlocks.hh"
+#include "goal.hh"
+
+namespace nix {
+
+struct DerivationGoal;
+
+/**
+ * This goal type is essentially the serial composition (like function
+ * composition) of a goal for getting a derivation, and then a
+ * `DerivationGoal` using the newly-obtained derivation.
+ *
+ * In the (currently experimental) general inductive case of derivations
+ * that are themselves build outputs, that first goal will be *another*
+ * `CreateDerivationAndRealiseGoal`. In the (much more common) base-case
+ * where the derivation has no provence and is just referred to by
+ * (content-addressed) store path, that first goal is a
+ * `SubstitutionGoal`.
+ *
+ * If we already have the derivation (e.g. if the evalutator has created
+ * the derivation locally and then instructured the store to build it),
+ * we can skip the first goal entirely as a small optimization.
+ */
+struct CreateDerivationAndRealiseGoal : public Goal
+{
+    /**
+     * How to obtain a store path of the derivation to build.
+     */
+    ref<SingleDerivedPath> drvReq;
+
+    /**
+     * The path of the derivation, once obtained.
+     **/
+    std::optional<StorePath> optDrvPath;
+
+    /**
+     * The goal for the corresponding concrete derivation.
+     **/
+    std::shared_ptr<DerivationGoal> concreteDrvGoal;
+
+    /**
+     * The specific outputs that we need to build.
+     */
+    OutputsSpec wantedOutputs;
+
+    /**
+     * The final output paths of the build.
+     *
+     * - For input-addressed derivations, always the precomputed paths
+     *
+     * - For content-addressed derivations, calcuated from whatever the
+     *   hash ends up being. (Note that fixed outputs derivations that
+     *   produce the "wrong" output still install that data under its
+     *   true content-address.)
+     */
+    OutputPathMap finalOutputs;
+
+    BuildMode buildMode;
+
+    CreateDerivationAndRealiseGoal(
+        ref<SingleDerivedPath> drvReq,
+        const OutputsSpec & wantedOutputs,
+        Worker & worker,
+        BuildMode buildMode = bmNormal);
+    virtual ~CreateDerivationAndRealiseGoal();
+
+    void timedOut(Error && ex) override;
+
+    std::string key() override;
+
+    /**
+     * Add wanted outputs to an already existing derivation goal.
+     */
+    void addWantedOutputs(const OutputsSpec & outputs);
+
+    Co init() override;
+
+    JobCategory jobCategory() const override
+    {
+        return JobCategory::Administration;
+    };
+};
+
+}

src/libstore/build/derivation-goal.cc

@@ -137,21 +137,8 @@ Goal::Co DerivationGoal::init() {
     trace("init");
 
     if (useDerivation) {
-        /* The first thing to do is to make sure that the derivation
-           exists.  If it doesn't, it may be created through a
-           substitute. */
-
-        if (buildMode != bmNormal || !worker.evalStore.isValidPath(drvPath)) {
-            addWaitee(upcast_goal(worker.makePathSubstitutionGoal(drvPath)));
-            co_await Suspend{};
-        }
-
         trace("loading derivation");
 
-        if (nrFailed != 0) {
-            co_return done(BuildResult::MiscFailure, {}, Error("cannot build missing derivation '%s'", worker.store.printStorePath(drvPath)));
-        }
-
         /* `drvPath' should already be a root, but let's be on the safe
            side: if the user forgot to make it a root, we wouldn't want
            things being garbage collected while we're busy. */
@@ -1549,23 +1536,24 @@ void DerivationGoal::waiteeDone(GoalPtr waitee, ExitCode result)
     if (!useDerivation || !drv) return;
     auto & fullDrv = *dynamic_cast<Derivation *>(drv.get());
 
-    auto * dg = dynamic_cast<DerivationGoal *>(&*waitee);
-    if (!dg) return;
+    std::optional info = tryGetConcreteDrvGoal(waitee);
+    if (!info) return;
+    const auto & [dg, drvReq] = *info;
 
-    auto * nodeP = fullDrv.inputDrvs.findSlot(DerivedPath::Opaque { .path = dg->drvPath });
+    auto * nodeP = fullDrv.inputDrvs.findSlot(drvReq.get());
     if (!nodeP) return;
     auto & outputs = nodeP->value;
 
     for (auto & outputName : outputs) {
-        auto buildResult = dg->getBuildResult(DerivedPath::Built {
-            .drvPath = makeConstantStorePathRef(dg->drvPath),
+        auto buildResult = dg.get().getBuildResult(DerivedPath::Built {
+            .drvPath = makeConstantStorePathRef(dg.get().drvPath),
             .outputs = OutputsSpec::Names { outputName },
         });
         if (buildResult.success()) {
             auto i = buildResult.builtOutputs.find(outputName);
             if (i != buildResult.builtOutputs.end())
                 inputDrvOutputs.insert_or_assign(
-                    { dg->drvPath, outputName },
+                    { dg.get().drvPath, outputName },
                     i->second.outPath);
         }
     }

src/libstore/build/derivation-goal.hh

@@ -56,6 +56,10 @@ struct InitialOutput {
 
 /**
  * A goal for building some or all of the outputs of a derivation.
+ *
+ * The derivation must already be present, either in the store in a drv
+ * or in memory. If the derivation itself needs to be gotten first, a
+ * `CreateDerivationAndRealiseGoal` goal must be used instead.
  */
 struct DerivationGoal : public Goal
 {

src/libstore/build/entry-points.cc

@@ -1,6 +1,7 @@
 #include "worker.hh"
 #include "substitution-goal.hh"
 #ifndef _WIN32 // TODO Enable building on Windows
+#  include "create-derivation-and-realise-goal.hh"
 #  include "derivation-goal.hh"
 #endif
 #include "local-store.hh"
@@ -29,8 +30,8 @@ void Store::buildPaths(const std::vector<DerivedPath> & reqs, BuildMode buildMod
         }
         if (i->exitCode != Goal::ecSuccess) {
 #ifndef _WIN32 // TODO Enable building on Windows
-            if (auto i2 = dynamic_cast<DerivationGoal *>(i.get()))
-                failed.insert(printStorePath(i2->drvPath));
+            if (auto i2 = dynamic_cast<CreateDerivationAndRealiseGoal *>(i.get()))
+                failed.insert(i2->drvReq->to_string(*this));
             else
 #endif
             if (auto i2 = dynamic_cast<PathSubstitutionGoal *>(i.get()))

src/libstore/build/goal.cc

@@ -175,7 +175,7 @@ Goal::Done Goal::amDone(ExitCode result, std::optional<Error> ex)
     exitCode = result;
 
     if (ex) {
-        if (!waiters.empty())
+        if (!preserveException && !waiters.empty())
             logError(ex->info());
         else
             this->ex = std::move(*ex);

src/libstore/build/goal.hh

@@ -50,6 +50,16 @@ enum struct JobCategory {
      * A substitution an arbitrary store object; it will use network resources.
      */
     Substitution,
+    /**
+     * A goal that does no "real" work by itself, and just exists to depend on
+     * other goals which *do* do real work. These goals therefore are not
+     * limited.
+     *
+     * These goals cannot infinitely create themselves, so there is no risk of
+     * a "fork bomb" type situation (which would be a problem even though the
+     * goal do no real work) either.
+     */
+    Administration,
 };
 
 struct Goal : public std::enable_shared_from_this<Goal>
@@ -373,6 +383,17 @@ public:
      */
     BuildResult getBuildResult(const DerivedPath &) const;
 
+    /**
+     * Hack to say that this goal should not log `ex`, but instead keep
+     * it around. Set by a waitee which sees itself as the designated
+     * continuation of this goal, responsible for reporting its
+     * successes or failures.
+     *
+     * @todo this is yet another not-nice hack in the goal system that
+     * we ought to get rid of.
+     */
+    bool preserveException = false;
+
     /**
      * Exception containing an error message, if any.
      */