Revert "Merge pull request #14022 from obsidiansystems/derivation-resolution-goal"

This reverts commit d02dca099f, reversing
changes made to 9bd09155ac.

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

@@ -1,5 +1,6 @@
 #include "nix/store/build/derivation-building-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
 #  include "nix/store/build/hook-instance.hh"
 #  include "nix/store/build/derivation-builder.hh"
@@ -26,8 +27,8 @@
 namespace nix {
 
 DerivationBuildingGoal::DerivationBuildingGoal(
-    const StorePath & drvPath, const Derivation & drv_, Worker & worker, BuildMode buildMode, bool storeDerivation)
+    const StorePath & drvPath, const Derivation & drv_, Worker & worker, BuildMode buildMode)
-    : Goal(worker, gaveUpOnSubstitution(storeDerivation))
+    : Goal(worker, gaveUpOnSubstitution())
     , drvPath(drvPath)
     , buildMode(buildMode)
 {
@@ -124,10 +125,50 @@ 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(bool storeDerivation)
+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.
 
@@ -172,17 +213,177 @@ Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution(bool storeDerivation)
 
     /* Determine the full set of input paths. */
 
-    if (storeDerivation) {
+    /* First, the input derivations. */
-        assert(drv->inputDrvs.map.empty());
-        /* Store the resolved derivation, as part of the record of
-           what we're actually building */
-        writeDerivation(worker.store, *drv);
-    }
-
     {
+        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);
+            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),
+                });
+
+            /* TODO https://github.com/NixOS/nix/issues/13247 we should
+               let the calling goal do this, so it has a change to pass
+               just the output(s) it cares about. */
+            auto resolvedDrvGoal =
+                worker.makeDerivationTrampolineGoal(pathResolved, OutputsSpec::All{}, drvResolved, buildMode);
+            {
+                Goals waitees{resolvedDrvGoal};
+                co_await await(std::move(waitees));
+            }
+
+            trace("resolved derivation finished");
+
+            auto resolvedResult = resolvedDrvGoal->buildResult;
+
+            // No `std::visit` for coroutines yet
+            if (auto * successP = resolvedResult.tryGetSuccess()) {
+                auto & success = *successP;
+                SingleDrvOutputs builtOutputs;
+
+                auto outputHashes = staticOutputHashes(worker.evalStore, *drv);
+                auto resolvedHashes = staticOutputHashes(worker.store, drvResolved);
+
+                StorePathSet outputPaths;
+
+                for (auto & outputName : drvResolved.outputNames()) {
+                    auto outputHash = get(outputHashes, outputName);
+                    auto resolvedHash = get(resolvedHashes, outputName);
+                    if ((!outputHash) || (!resolvedHash))
+                        throw Error(
+                            "derivation '%s' doesn't have expected output '%s' (derivation-goal.cc/resolve)",
+                            worker.store.printStorePath(drvPath),
+                            outputName);
+
+                    auto realisation = [&] {
+                        auto take1 = get(success.builtOutputs, outputName);
+                        if (take1)
+                            return *take1;
+
+                        /* The above `get` should work. But stateful tracking of
+                           outputs in resolvedResult, this can get out of sync with the
+                           store, which is our actual source of truth. For now we just
+                           check the store directly if it fails. */
+                        auto take2 = worker.evalStore.queryRealisation(DrvOutput{*resolvedHash, outputName});
+                        if (take2)
+                            return *take2;
+
+                        throw Error(
+                            "derivation '%s' doesn't have expected output '%s' (derivation-goal.cc/realisation)",
+                            worker.store.printStorePath(pathResolved),
+                            outputName);
+                    }();
+
+                    if (!drv->type().isImpure()) {
+                        auto newRealisation = realisation;
+                        newRealisation.id = DrvOutput{*outputHash, outputName};
+                        newRealisation.signatures.clear();
+                        if (!drv->type().isFixed()) {
+                            auto & drvStore = worker.evalStore.isValidPath(drvPath) ? worker.evalStore : worker.store;
+                            newRealisation.dependentRealisations =
+                                drvOutputReferences(worker.store, *drv, realisation.outPath, &drvStore);
+                        }
+                        worker.store.signRealisation(newRealisation);
+                        worker.store.registerDrvOutput(newRealisation);
+                    }
+                    outputPaths.insert(realisation.outPath);
+                    builtOutputs.emplace(outputName, realisation);
+                }
+
+                runPostBuildHook(worker.store, *logger, drvPath, outputPaths);
+
+                auto status = success.status;
+                if (status == BuildResult::Success::AlreadyValid)
+                    status = BuildResult::Success::ResolvesToAlreadyValid;
+
+                co_return doneSuccess(success.status, std::move(builtOutputs));
+            } else if (resolvedResult.tryGetFailure()) {
+                co_return doneFailure({
+                    BuildResult::Failure::DependencyFailed,
+                    "build of resolved derivation '%s' failed",
+                    worker.store.printStorePath(pathResolved),
+                });
+            } else
+                assert(false);
+        }
+
         /* If we get this far, we know no dynamic drvs inputs */
 
-        for (auto & [depDrvPath, depNode] : drv->inputDrvs.map) {
+        for (auto & [depDrvPath, depNode] : fullDrv.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-goal.cc

@@ -1,6 +1,5 @@
 #include "nix/store/build/derivation-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
 #  include "nix/store/build/hook-instance.hh"
 #  include "nix/store/build/derivation-builder.hh"
@@ -30,9 +29,8 @@ DerivationGoal::DerivationGoal(
     const Derivation & drv,
     const OutputName & wantedOutput,
     Worker & worker,
-    BuildMode buildMode,
+    BuildMode buildMode)
-    bool storeDerivation)
+    : Goal(worker, haveDerivation())
-    : Goal(worker, haveDerivation(storeDerivation))
     , drvPath(drvPath)
     , wantedOutput(wantedOutput)
     , outputHash{[&] {
@@ -66,7 +64,7 @@ std::string DerivationGoal::key()
     }.to_string(worker.store);
 }
 
-Goal::Co DerivationGoal::haveDerivation(bool storeDerivation)
+Goal::Co DerivationGoal::haveDerivation()
 {
     trace("have derivation");
 
@@ -148,96 +146,9 @@ Goal::Co DerivationGoal::haveDerivation(bool storeDerivation)
                 worker.store.printStorePath(drvPath));
     }
 
-    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"});
-    }
-
-    if (resolutionGoal->resolvedDrv) {
-        auto & [pathResolved, drvResolved] = *resolutionGoal->resolvedDrv;
-
-        auto resolvedDrvGoal =
-            worker.makeDerivationGoal(pathResolved, drvResolved, wantedOutput, buildMode, /*storeDerivation=*/true);
-        {
-            Goals waitees{resolvedDrvGoal};
-            co_await await(std::move(waitees));
-        }
-
-        trace("resolved derivation finished");
-
-        auto resolvedResult = resolvedDrvGoal->buildResult;
-
-        // No `std::visit` for coroutines yet
-        if (auto * successP = resolvedResult.tryGetSuccess()) {
-            auto & success = *successP;
-            auto outputHashes = staticOutputHashes(worker.evalStore, *drv);
-            auto resolvedHashes = staticOutputHashes(worker.store, drvResolved);
-
-            StorePathSet outputPaths;
-
-            auto outputHash = get(outputHashes, wantedOutput);
-            auto resolvedHash = get(resolvedHashes, wantedOutput);
-            if ((!outputHash) || (!resolvedHash))
-                throw Error(
-                    "derivation '%s' doesn't have expected output '%s' (derivation-goal.cc/resolve)",
-                    worker.store.printStorePath(drvPath),
-                    wantedOutput);
-
-            auto realisation = [&] {
-                auto take1 = get(success.builtOutputs, wantedOutput);
-                if (take1)
-                    return *take1;
-
-                /* The above `get` should work. But stateful tracking of
-                   outputs in resolvedResult, this can get out of sync with the
-                   store, which is our actual source of truth. For now we just
-                   check the store directly if it fails. */
-                auto take2 = worker.evalStore.queryRealisation(DrvOutput{*resolvedHash, wantedOutput});
-                if (take2)
-                    return *take2;
-
-                throw Error(
-                    "derivation '%s' doesn't have expected output '%s' (derivation-goal.cc/realisation)",
-                    worker.store.printStorePath(pathResolved),
-                    wantedOutput);
-            }();
-
-            if (!drv->type().isImpure()) {
-                auto newRealisation = realisation;
-                newRealisation.id = DrvOutput{*outputHash, wantedOutput};
-                newRealisation.signatures.clear();
-                if (!drv->type().isFixed()) {
-                    auto & drvStore = worker.evalStore.isValidPath(drvPath) ? worker.evalStore : worker.store;
-                    newRealisation.dependentRealisations =
-                        drvOutputReferences(worker.store, *drv, realisation.outPath, &drvStore);
-                }
-                worker.store.signRealisation(newRealisation);
-                worker.store.registerDrvOutput(newRealisation);
-            }
-            outputPaths.insert(realisation.outPath);
-
-            auto status = success.status;
-            if (status == BuildResult::Success::AlreadyValid)
-                status = BuildResult::Success::ResolvesToAlreadyValid;
-
-            co_return doneSuccess(status, std::move(realisation));
-        } else if (resolvedResult.tryGetFailure()) {
-            co_return doneFailure({
-                BuildResult::Failure::DependencyFailed,
-                "build of resolved derivation '%s' failed",
-                worker.store.printStorePath(pathResolved),
-            });
-        } else
-            assert(false);
-    }
-
     /* Give up on substitution for the output we want, actually build this derivation */
 
-    auto g = worker.makeDerivationBuildingGoal(drvPath, *drv, buildMode, storeDerivation);
+    auto g = worker.makeDerivationBuildingGoal(drvPath, *drv, buildMode);
 
     /* We will finish with it ourselves, as if we were the derivational goal. */
     g->preserveException = true;

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

@@ -1,210 +0,0 @@
-#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,7 +4,6 @@
 #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
@@ -76,26 +75,15 @@ std::shared_ptr<DerivationTrampolineGoal> Worker::makeDerivationTrampolineGoal(
 }
 
 std::shared_ptr<DerivationGoal> Worker::makeDerivationGoal(
-    const StorePath & drvPath,
+    const StorePath & drvPath, const Derivation & drv, const OutputName & wantedOutput, BuildMode buildMode)
-    const Derivation & drv,
-    const OutputName & wantedOutput,
-    BuildMode buildMode,
-    bool storeDerivation)
 {
-    return initGoalIfNeeded(
+    return initGoalIfNeeded(derivationGoals[drvPath][wantedOutput], drvPath, drv, wantedOutput, *this, buildMode);
-        derivationGoals[drvPath][wantedOutput], drvPath, drv, wantedOutput, *this, buildMode, storeDerivation);
 }
 
-std::shared_ptr<DerivationResolutionGoal>
+std::shared_ptr<DerivationBuildingGoal>
-Worker::makeDerivationResolutionGoal(const StorePath & drvPath, const Derivation & drv, BuildMode buildMode)
+Worker::makeDerivationBuildingGoal(const StorePath & drvPath, const Derivation & drv, BuildMode buildMode)
 {
-    return initGoalIfNeeded(derivationResolutionGoals[drvPath], drvPath, drv, *this, buildMode);
+    return initGoalIfNeeded(derivationBuildingGoals[drvPath], drvPath, drv, *this, 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, storeDerivation);
 }
 
 std::shared_ptr<PathSubstitutionGoal>
@@ -170,8 +158,6 @@ 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

@@ -29,21 +29,8 @@ 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 StorePath & drvPath, const Derivation & drv, Worker & worker, BuildMode buildMode = bmNormal);
-        const Derivation & drv,
-        Worker & worker,
-        BuildMode buildMode = bmNormal,
-        bool storeDerivation = false);
     ~DerivationBuildingGoal();
 
 private:
@@ -113,7 +100,7 @@ private:
     /**
      * The states.
      */
-    Co gaveUpOnSubstitution(bool storeDerivation);
+    Co gaveUpOnSubstitution();
     Co tryToBuild();
 
     /**
@@ -168,7 +155,7 @@ private:
 
     JobCategory jobCategory() const override
     {
-        return JobCategory::Administration;
+        return JobCategory::Build;
     };
 };
 

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

@@ -40,16 +40,12 @@ 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
@@ -84,7 +80,7 @@ private:
     /**
      * The states.
      */
-    Co haveDerivation(bool storeDerivation);
+    Co haveDerivation();
 
     /**
      * Return `std::nullopt` if the output is unknown, e.g. un unbuilt

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

@@ -1,82 +0,0 @@
-#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,7 +16,6 @@ namespace nix {
 /* Forward definition. */
 struct DerivationTrampolineGoal;
 struct DerivationGoal;
-struct DerivationResolutionGoal;
 struct DerivationBuildingGoal;
 struct PathSubstitutionGoal;
 class DrvOutputSubstitutionGoal;
@@ -112,7 +111,6 @@ 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;
@@ -223,23 +221,13 @@ public:
         const StorePath & drvPath,
         const Derivation & drv,
         const OutputName & wantedOutput,
-        BuildMode buildMode = bmNormal,
+        BuildMode buildMode = bmNormal);
-        bool storeDerivation = false);
 
     /**
-     * @ref DerivationResolutionGoal "derivation resolution goal"
+     * @ref DerivationBuildingGoal "derivation goal"
      */
-    std::shared_ptr<DerivationResolutionGoal>
+    std::shared_ptr<DerivationBuildingGoal>
-    makeDerivationResolutionGoal(const StorePath & drvPath, const Derivation & drv, BuildMode buildMode = bmNormal);
+    makeDerivationBuildingGoal(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,
-        bool storeDerivation = false);
 
     /**
      * @ref PathSubstitutionGoal "substitution goal"

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

@@ -18,7 +18,6 @@ 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

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

tests/functional/ca/issue-13247.sh

@@ -65,4 +65,7 @@ buildViaSubstitute use-a-prime-more-outputs^first
 # Should only fetch the output we asked for
 [[ -d "$(jq -r <"$TEST_ROOT"/a.json '.[0].outputs.out')" ]]
 [[ -f "$(jq -r <"$TEST_ROOT"/a.json '.[2].outputs.first')" ]]
-[[ ! -e "$(jq -r <"$TEST_ROOT"/a.json '.[2].outputs.second')" ]]
+
+# Output should *not* be here, this is the bug
+[[ -e "$(jq -r <"$TEST_ROOT"/a.json '.[2].outputs.second')" ]]
+skipTest "bug is not yet fixed"