Split out DerivationResolutionGoal

This prepares the way for fixing a few issues.

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

@@ -1,4 +1,5 @@
 #include "nix/store/build/derivation-building-goal.hh"
+#include "nix/store/build/derivation-resolution-goal.hh"
 #include "nix/store/build/derivation-env-desugar.hh"
 #include "nix/store/build/derivation-trampoline-goal.hh"
 #ifndef _WIN32 // TODO enable build hook on Windows
@@ -129,46 +130,6 @@ Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution()
 {
     Goals waitees;
 
-    std::map<ref<const SingleDerivedPath>, GoalPtr, value_comparison> inputGoals;
-
-    {
-        std::function<void(ref<const SingleDerivedPath>, const DerivedPathMap<StringSet>::ChildNode &)>
-            addWaiteeDerivedPath;
-
-        addWaiteeDerivedPath = [&](ref<const SingleDerivedPath> inputDrv,
-                                   const DerivedPathMap<StringSet>::ChildNode & inputNode) {
-            if (!inputNode.value.empty()) {
-                auto g = worker.makeGoal(
-                    DerivedPath::Built{
-                        .drvPath = inputDrv,
-                        .outputs = inputNode.value,
-                    },
-                    buildMode == bmRepair ? bmRepair : bmNormal);
-                inputGoals.insert_or_assign(inputDrv, g);
-                waitees.insert(std::move(g));
-            }
-            for (const auto & [outputName, childNode] : inputNode.childMap)
-                addWaiteeDerivedPath(
-                    make_ref<SingleDerivedPath>(SingleDerivedPath::Built{inputDrv, outputName}), childNode);
-        };
-
-        for (const auto & [inputDrvPath, inputNode] : drv->inputDrvs.map) {
-            /* Ensure that pure, non-fixed-output derivations don't
-               depend on impure derivations. */
-            if (experimentalFeatureSettings.isEnabled(Xp::ImpureDerivations) && !drv->type().isImpure()
-                && !drv->type().isFixed()) {
-                auto inputDrv = worker.evalStore.readDerivation(inputDrvPath);
-                if (inputDrv.type().isImpure())
-                    throw Error(
-                        "pure derivation '%s' depends on impure derivation '%s'",
-                        worker.store.printStorePath(drvPath),
-                        worker.store.printStorePath(inputDrvPath));
-            }
-
-            addWaiteeDerivedPath(makeConstantStorePathRef(inputDrvPath), inputNode);
-        }
-    }
-
     /* Copy the input sources from the eval store to the build
        store.
 
@@ -213,88 +174,22 @@ Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution()
 
     /* Determine the full set of input paths. */
 
-    /* First, the input derivations. */
     {
-        auto & fullDrv = *drv;
+        auto resolutionGoal = worker.makeDerivationResolutionGoal(drvPath, *drv, buildMode);
+        {
+            Goals waitees{resolutionGoal};
+            co_await await(std::move(waitees));
+        }
+        if (nrFailed != 0) {
+            co_return doneFailure({BuildResult::Failure::DependencyFailed, "resolution failed"});
+        }
 
-        auto drvType = fullDrv.type();
-        bool resolveDrv =
-            std::visit(
-                overloaded{
-                    [&](const DerivationType::InputAddressed & ia) {
-                        /* must resolve if deferred. */
-                        return ia.deferred;
-                    },
-                    [&](const DerivationType::ContentAddressed & ca) {
-                        return !fullDrv.inputDrvs.map.empty()
-                               && (ca.fixed
-                                       /* Can optionally resolve if fixed, which is good
-                                          for avoiding unnecessary rebuilds. */
-                                       ? experimentalFeatureSettings.isEnabled(Xp::CaDerivations)
-                                       /* Must resolve if floating and there are any inputs
-                                          drvs. */
-                                       : true);
-                    },
-                    [&](const DerivationType::Impure &) { return true; }},
-                drvType.raw)
-            /* no inputs are outputs of dynamic derivations */
-            || std::ranges::any_of(fullDrv.inputDrvs.map.begin(), fullDrv.inputDrvs.map.end(), [](auto & pair) {
-                   return !pair.second.childMap.empty();
-               });
+        if (resolutionGoal->resolvedDrv) {
+            auto & [pathResolved, drvResolved] = *resolutionGoal->resolvedDrv;
 
-        if (resolveDrv && !fullDrv.inputDrvs.map.empty()) {
-            experimentalFeatureSettings.require(Xp::CaDerivations);
-
-            /* We are be able to resolve this derivation based on the
-               now-known results of dependencies. If so, we become a
-               stub goal aliasing that resolved derivation goal. */
-            std::optional attempt = fullDrv.tryResolve(
-                worker.store,
-                [&](ref<const SingleDerivedPath> drvPath, const std::string & outputName) -> std::optional<StorePath> {
-                    auto mEntry = get(inputGoals, drvPath);
-                    if (!mEntry)
-                        return std::nullopt;
-
-                    auto & buildResult = (*mEntry)->buildResult;
-                    return std::visit(
-                        overloaded{
-                            [](const BuildResult::Failure &) -> std::optional<StorePath> { return std::nullopt; },
-                            [&](const BuildResult::Success & success) -> std::optional<StorePath> {
-                                auto i = get(success.builtOutputs, outputName);
-                                if (!i)
-                                    return std::nullopt;
-
-                                return i->outPath;
-                            },
-                        },
-                        buildResult.inner);
-                });
-            if (!attempt) {
-                /* TODO (impure derivations-induced tech debt) (see below):
-                   The above attempt should have found it, but because we manage
-                   inputDrvOutputs statefully, sometimes it gets out of sync with
-                   the real source of truth (store). So we query the store
-                   directly if there's a problem. */
-                attempt = fullDrv.tryResolve(worker.store, &worker.evalStore);
-            }
-            assert(attempt);
-            Derivation drvResolved{std::move(*attempt)};
-
-            auto pathResolved = writeDerivation(worker.store, drvResolved);
-
-            auto msg =
-                fmt("resolved derivation: '%s' -> '%s'",
-                    worker.store.printStorePath(drvPath),
-                    worker.store.printStorePath(pathResolved));
-            act = std::make_unique<Activity>(
-                *logger,
-                lvlInfo,
-                actBuildWaiting,
-                msg,
-                Logger::Fields{
-                    worker.store.printStorePath(drvPath),
-                    worker.store.printStorePath(pathResolved),
-                });
+            /* Store the resolved derivation, as part of the record of
+               what we're actually building */
+            writeDerivation(worker.store, drvResolved);
 
             /* TODO https://github.com/NixOS/nix/issues/13247 we should
                let the calling goal do this, so it has a change to pass
@@ -383,7 +278,7 @@ Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution()
 
         /* If we get this far, we know no dynamic drvs inputs */
 
-        for (auto & [depDrvPath, depNode] : fullDrv.inputDrvs.map) {
+        for (auto & [depDrvPath, depNode] : drv->inputDrvs.map) {
             for (auto & outputName : depNode.value) {
                 /* Don't need to worry about `inputGoals`, because
                    impure derivations are always resolved above. Can

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

@@ -0,0 +1,210 @@
+#include "nix/store/build/derivation-resolution-goal.hh"
+#include "nix/store/build/derivation-env-desugar.hh"
+#include "nix/store/build/worker.hh"
+#include "nix/util/util.hh"
+#include "nix/store/common-protocol.hh"
+#include "nix/store/globals.hh"
+
+#include <sys/types.h>
+#include <fcntl.h>
+#include <unistd.h>
+
+#include <nlohmann/json.hpp>
+
+namespace nix {
+
+DerivationResolutionGoal::DerivationResolutionGoal(
+    const StorePath & drvPath, const Derivation & drv_, Worker & worker, BuildMode buildMode)
+    : Goal(worker, resolveDerivation())
+    , drvPath(drvPath)
+{
+    drv = std::make_unique<Derivation>(drv_);
+
+    name = fmt("building of '%s' from in-memory derivation", worker.store.printStorePath(drvPath));
+    trace("created");
+
+    /* Prevent the .chroot directory from being
+       garbage-collected. (See isActiveTempFile() in gc.cc.) */
+    worker.store.addTempRoot(this->drvPath);
+}
+
+void DerivationResolutionGoal::timedOut(Error && ex) {}
+
+std::string DerivationResolutionGoal::key()
+{
+    /* Ensure that derivations get built in order of their name,
+       i.e. a derivation named "aardvark" always comes before
+       "baboon". And substitution goals always happen before
+       derivation goals (due to "bd$"). */
+    return "rd$" + std::string(drvPath.name()) + "$" + worker.store.printStorePath(drvPath);
+}
+
+/**
+ * Used for `inputGoals` local variable below
+ */
+struct value_comparison
+{
+    template<typename T>
+    bool operator()(const ref<T> & lhs, const ref<T> & rhs) const
+    {
+        return *lhs < *rhs;
+    }
+};
+
+/* At least one of the output paths could not be
+   produced using a substitute.  So we have to build instead. */
+Goal::Co DerivationResolutionGoal::resolveDerivation()
+{
+    Goals waitees;
+
+    std::map<ref<const SingleDerivedPath>, GoalPtr, value_comparison> inputGoals;
+
+    {
+        std::function<void(ref<const SingleDerivedPath>, const DerivedPathMap<StringSet>::ChildNode &)>
+            addWaiteeDerivedPath;
+
+        addWaiteeDerivedPath = [&](ref<const SingleDerivedPath> inputDrv,
+                                   const DerivedPathMap<StringSet>::ChildNode & inputNode) {
+            if (!inputNode.value.empty()) {
+                auto g = worker.makeGoal(
+                    DerivedPath::Built{
+                        .drvPath = inputDrv,
+                        .outputs = inputNode.value,
+                    },
+                    buildMode == bmRepair ? bmRepair : bmNormal);
+                inputGoals.insert_or_assign(inputDrv, g);
+                waitees.insert(std::move(g));
+            }
+            for (const auto & [outputName, childNode] : inputNode.childMap)
+                addWaiteeDerivedPath(
+                    make_ref<SingleDerivedPath>(SingleDerivedPath::Built{inputDrv, outputName}), childNode);
+        };
+
+        for (const auto & [inputDrvPath, inputNode] : drv->inputDrvs.map) {
+            /* Ensure that pure, non-fixed-output derivations don't
+               depend on impure derivations. */
+            if (experimentalFeatureSettings.isEnabled(Xp::ImpureDerivations) && !drv->type().isImpure()
+                && !drv->type().isFixed()) {
+                auto inputDrv = worker.evalStore.readDerivation(inputDrvPath);
+                if (inputDrv.type().isImpure())
+                    throw Error(
+                        "pure derivation '%s' depends on impure derivation '%s'",
+                        worker.store.printStorePath(drvPath),
+                        worker.store.printStorePath(inputDrvPath));
+            }
+
+            addWaiteeDerivedPath(makeConstantStorePathRef(inputDrvPath), inputNode);
+        }
+    }
+
+    co_await await(std::move(waitees));
+
+    trace("all inputs realised");
+
+    if (nrFailed != 0) {
+        auto msg =
+            fmt("Cannot build '%s'.\n"
+                "Reason: " ANSI_RED "%d %s failed" ANSI_NORMAL ".",
+                Magenta(worker.store.printStorePath(drvPath)),
+                nrFailed,
+                nrFailed == 1 ? "dependency" : "dependencies");
+        msg += showKnownOutputs(worker.store, *drv);
+        co_return amDone(ecFailed, {BuildError(BuildResult::Failure::DependencyFailed, msg)});
+    }
+
+    /* Gather information necessary for computing the closure and/or
+       running the build hook. */
+
+    /* Determine the full set of input paths. */
+
+    /* First, the input derivations. */
+    {
+        auto & fullDrv = *drv;
+
+        auto drvType = fullDrv.type();
+        bool resolveDrv =
+            std::visit(
+                overloaded{
+                    [&](const DerivationType::InputAddressed & ia) {
+                        /* must resolve if deferred. */
+                        return ia.deferred;
+                    },
+                    [&](const DerivationType::ContentAddressed & ca) {
+                        return !fullDrv.inputDrvs.map.empty()
+                               && (ca.fixed
+                                       /* Can optionally resolve if fixed, which is good
+                                          for avoiding unnecessary rebuilds. */
+                                       ? experimentalFeatureSettings.isEnabled(Xp::CaDerivations)
+                                       /* Must resolve if floating and there are any inputs
+                                          drvs. */
+                                       : true);
+                    },
+                    [&](const DerivationType::Impure &) { return true; }},
+                drvType.raw)
+            /* no inputs are outputs of dynamic derivations */
+            || std::ranges::any_of(fullDrv.inputDrvs.map.begin(), fullDrv.inputDrvs.map.end(), [](auto & pair) {
+                   return !pair.second.childMap.empty();
+               });
+
+        if (resolveDrv && !fullDrv.inputDrvs.map.empty()) {
+            experimentalFeatureSettings.require(Xp::CaDerivations);
+
+            /* We are be able to resolve this derivation based on the
+               now-known results of dependencies. If so, we become a
+               stub goal aliasing that resolved derivation goal. */
+            std::optional attempt = fullDrv.tryResolve(
+                worker.store,
+                [&](ref<const SingleDerivedPath> drvPath, const std::string & outputName) -> std::optional<StorePath> {
+                    auto mEntry = get(inputGoals, drvPath);
+                    if (!mEntry)
+                        return std::nullopt;
+
+                    auto & buildResult = (*mEntry)->buildResult;
+                    return std::visit(
+                        overloaded{
+                            [](const BuildResult::Failure &) -> std::optional<StorePath> { return std::nullopt; },
+                            [&](const BuildResult::Success & success) -> std::optional<StorePath> {
+                                auto i = get(success.builtOutputs, outputName);
+                                if (!i)
+                                    return std::nullopt;
+
+                                return i->outPath;
+                            },
+                        },
+                        buildResult.inner);
+                });
+            if (!attempt) {
+                /* TODO (impure derivations-induced tech debt) (see below):
+                   The above attempt should have found it, but because we manage
+                   inputDrvOutputs statefully, sometimes it gets out of sync with
+                   the real source of truth (store). So we query the store
+                   directly if there's a problem. */
+                attempt = fullDrv.tryResolve(worker.store, &worker.evalStore);
+            }
+            assert(attempt);
+
+            auto pathResolved = writeDerivation(worker.store, *attempt, NoRepair, /*readOnly =*/true);
+
+            auto msg =
+                fmt("resolved derivation: '%s' -> '%s'",
+                    worker.store.printStorePath(drvPath),
+                    worker.store.printStorePath(pathResolved));
+            act = std::make_unique<Activity>(
+                *logger,
+                lvlInfo,
+                actBuildWaiting,
+                msg,
+                Logger::Fields{
+                    worker.store.printStorePath(drvPath),
+                    worker.store.printStorePath(pathResolved),
+                });
+
+            resolvedDrv =
+                std::make_unique<std::pair<StorePath, BasicDerivation>>(std::move(pathResolved), *std::move(attempt));
+        }
+    }
+
+    co_return amDone(ecSuccess, std::nullopt);
+}
+
+} // namespace nix

src/libstore/build/worker.cc

@@ -4,6 +4,7 @@
 #include "nix/store/build/substitution-goal.hh"
 #include "nix/store/build/drv-output-substitution-goal.hh"
 #include "nix/store/build/derivation-goal.hh"
+#include "nix/store/build/derivation-resolution-goal.hh"
 #include "nix/store/build/derivation-building-goal.hh"
 #include "nix/store/build/derivation-trampoline-goal.hh"
 #ifndef _WIN32 // TODO Enable building on Windows
@@ -80,6 +81,12 @@ std::shared_ptr<DerivationGoal> Worker::makeDerivationGoal(
     return initGoalIfNeeded(derivationGoals[drvPath][wantedOutput], drvPath, drv, wantedOutput, *this, buildMode);
 }
 
+std::shared_ptr<DerivationResolutionGoal>
+Worker::makeDerivationResolutionGoal(const StorePath & drvPath, const Derivation & drv, BuildMode buildMode)
+{
+    return initGoalIfNeeded(derivationResolutionGoals[drvPath], drvPath, drv, *this, buildMode);
+}
+
 std::shared_ptr<DerivationBuildingGoal>
 Worker::makeDerivationBuildingGoal(const StorePath & drvPath, const Derivation & drv, BuildMode buildMode)
 {
@@ -158,6 +165,8 @@ void Worker::removeGoal(GoalPtr goal)
         nix::removeGoal(drvGoal, derivationTrampolineGoals.map);
     else if (auto drvGoal = std::dynamic_pointer_cast<DerivationGoal>(goal))
         nix::removeGoal(drvGoal, derivationGoals);
+    else if (auto drvResolutionGoal = std::dynamic_pointer_cast<DerivationResolutionGoal>(goal))
+        nix::removeGoal(drvResolutionGoal, derivationResolutionGoals);
     else if (auto drvBuildingGoal = std::dynamic_pointer_cast<DerivationBuildingGoal>(goal))
         nix::removeGoal(drvBuildingGoal, derivationBuildingGoals);
     else if (auto subGoal = std::dynamic_pointer_cast<PathSubstitutionGoal>(goal))

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

@@ -155,7 +155,7 @@ private:
 
     JobCategory jobCategory() const override
     {
-        return JobCategory::Build;
+        return JobCategory::Administration;
     };
 };
 

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

@@ -0,0 +1,82 @@
+#pragma once
+///@file
+
+#include "nix/store/derivations.hh"
+#include "nix/store/derivation-options.hh"
+#include "nix/store/build/derivation-building-misc.hh"
+#include "nix/store/store-api.hh"
+#include "nix/store/build/goal.hh"
+
+namespace nix {
+
+struct BuilderFailureError;
+
+/**
+ * A goal for resolving a derivation. Resolving a derivation (@see
+ * `Derivation::tryResolve`) simplifies its inputs, replacing
+ * `inputDrvs` with `inputSrcs.
+ *
+ * Conceptually, we resolve all derivations. For input-addressed
+ * derivations (that don't transtively depend on content-addressed
+ * derivations), however, we don't actually use the resolved derivation,
+ * because the output paths would appear invalid (if we tried to verify
+ * them), since they are computed from the original, unresolved inputs.
+ *
+ * That said, if we ever made the new flavor of input-addressing as described
+ * in issue #9259, then the input-addressing would be based on the resolved
+ * inputs, and we like the CA case *would* use the output of this goal.
+ *
+ * (The point of this discussion is not to randomly stuff information on
+ * a yet-unimplemented feature (issue #9259) in the codebase, but
+ * rather, to illustrate that there is no inherent tension between
+ * explicit derivation resolution and input-addressing in general. That
+ * tension only exists with the type of input-addressing we've
+ * historically used.)
+ */
+struct DerivationResolutionGoal : public Goal
+{
+    DerivationResolutionGoal(
+        const StorePath & drvPath, const Derivation & drv, Worker & worker, BuildMode buildMode = bmNormal);
+
+    /**
+     * If the derivation needed to be resolved, this is resulting
+     * resolved derivations and its path.
+     */
+    std::unique_ptr<std::pair<StorePath, BasicDerivation>> resolvedDrv;
+
+    void timedOut(Error && ex) override;
+
+private:
+
+    /**
+     * The path of the derivation.
+     */
+    StorePath drvPath;
+
+    /**
+     * The derivation stored at drvPath.
+     */
+    std::unique_ptr<Derivation> drv;
+
+    /**
+     * The remainder is state held during the build.
+     */
+
+    BuildMode buildMode;
+
+    std::unique_ptr<Activity> act;
+
+    std::string key() override;
+
+    /**
+     * The states.
+     */
+    Co resolveDerivation();
+
+    JobCategory jobCategory() const override
+    {
+        return JobCategory::Administration;
+    };
+};
+
+} // namespace nix

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

@@ -16,6 +16,7 @@ namespace nix {
 /* Forward definition. */
 struct DerivationTrampolineGoal;
 struct DerivationGoal;
+struct DerivationResolutionGoal;
 struct DerivationBuildingGoal;
 struct PathSubstitutionGoal;
 class DrvOutputSubstitutionGoal;
@@ -111,6 +112,7 @@ private:
     DerivedPathMap<std::map<OutputsSpec, std::weak_ptr<DerivationTrampolineGoal>>> derivationTrampolineGoals;
 
     std::map<StorePath, std::map<OutputName, std::weak_ptr<DerivationGoal>>> derivationGoals;
+    std::map<StorePath, std::weak_ptr<DerivationResolutionGoal>> derivationResolutionGoals;
     std::map<StorePath, std::weak_ptr<DerivationBuildingGoal>> derivationBuildingGoals;
     std::map<StorePath, std::weak_ptr<PathSubstitutionGoal>> substitutionGoals;
     std::map<DrvOutput, std::weak_ptr<DrvOutputSubstitutionGoal>> drvOutputSubstitutionGoals;
@@ -224,7 +226,13 @@ public:
         BuildMode buildMode = bmNormal);
 
     /**
-     * @ref DerivationBuildingGoal "derivation goal"
+     * @ref DerivationResolutionGoal "derivation resolution goal"
+     */
+    std::shared_ptr<DerivationResolutionGoal>
+    makeDerivationResolutionGoal(const StorePath & drvPath, const Derivation & drv, BuildMode buildMode = bmNormal);
+
+    /**
+     * @ref DerivationBuildingGoal "derivation building goal"
      */
     std::shared_ptr<DerivationBuildingGoal>
     makeDerivationBuildingGoal(const StorePath & drvPath, const Derivation & drv, BuildMode buildMode = bmNormal);

src/libstore/include/nix/store/meson.build

@@ -17,6 +17,7 @@ headers = [ config_pub_h ] + files(
   'build/derivation-building-misc.hh',
   'build/derivation-env-desugar.hh',
   'build/derivation-goal.hh',
+  'build/derivation-resolution-goal.hh',
   'build/derivation-trampoline-goal.hh',
   'build/drv-output-substitution-goal.hh',
   'build/goal.hh',

src/libstore/meson.build

@@ -274,6 +274,7 @@ sources = files(
   'build/derivation-check.cc',
   'build/derivation-env-desugar.cc',
   'build/derivation-goal.cc',
+  'build/derivation-resolution-goal.cc',
   'build/derivation-trampoline-goal.cc',
   'build/drv-output-substitution-goal.cc',
   'build/entry-points.cc',

tests/functional/build.sh

@@ -178,7 +178,8 @@ test "$(<<<"$out" grep -cE '^error:')" = 4
 
 out="$(nix build -f fod-failing.nix -L x4 2>&1)" && status=0 || status=$?
 test "$status" = 1
-test "$(<<<"$out" grep -cE '^error:')" = 2
+# Precise number of errors depends on daemon version / goal refactorings
+(( "$(<<<"$out" grep -cE '^error:')" >= 2 ))
 
 if isDaemonNewer "2.29pre"; then
     <<<"$out" grepQuiet -E "error: Cannot build '.*-x4\\.drv'"
@@ -186,11 +187,13 @@ if isDaemonNewer "2.29pre"; then
 else
     <<<"$out" grepQuiet -E "error: 1 dependencies of derivation '.*-x4\\.drv' failed to build"
 fi
-<<<"$out" grepQuiet -E "hash mismatch in fixed-output derivation '.*-x2\\.drv'"
+# Either x2 or x3 could have failed, x4 depends on both symmetrically
+<<<"$out" grepQuiet -E "hash mismatch in fixed-output derivation '.*-x[23]\\.drv'"
 
 out="$(nix build -f fod-failing.nix -L x4 --keep-going 2>&1)" && status=0 || status=$?
 test "$status" = 1
-test "$(<<<"$out" grep -cE '^error:')" = 3
+# Precise number of errors depends on daemon version / goal refactorings
+(( "$(<<<"$out" grep -cE '^error:')" >= 3 ))
 if isDaemonNewer "2.29pre"; then
     <<<"$out" grepQuiet -E "error: Cannot build '.*-x4\\.drv'"
     <<<"$out" grepQuiet -E "Reason: 2 dependencies failed."