From 8a9bde362a77caea525d5a3b65c736ddd7cd1c19 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 3 Mar 2025 10:33:54 -0500 Subject: [PATCH] Get rid of `addWantedOutputs` Due this by introducing `DerivationCreationAndRealisationGoal`. `DerivationGoal` now only tracks a single output, and is back to tracking a plain store path `drvPath`, not a deriving path one. Its `addWantedOutputs` method is gone. These changes will allow subsequent PRs to simplify it greatly. Because the purpose of each goal is back to being immutable, we can also once again make `Goal::buildResult` a public field, and get rid of the `getBuildResult` method. This simplifies things also. `DerivationCreationAndRealisationGoal` is a cheap "trampoline" goal. It takes immutable sets of wanted outputs, and just kicks of `DerivationGoal`s for them. Since now "actual work" is done in these goals, it is not wasteful to have separate ones for separate sets of outputs, even if those outputs (and the derivations they are from) overlap. This separation of concerns will make it possible for future work on issues like #11928, and to continue the path of having more goal types, but each goal type does fewer things (issue #12628). This commit in some sense reverts f4f28cdd0e01a9bfb0901e8a53ead719265fa9b8, but that one kept around `addWantedOutputs`. I am quite sure it was having two layers of goals with `addWantedOutputs` that caused the issues --- restarting logic like `addWantedOutputs` has is very tempermental! In this version of the change, we have *zero* layers of `addWantedOutputs` --- no goal type needs it, or otherwise has a mutable objective --- and so I think this change is safe. --- .../include/nix/fetchers/input-cache.hh | 2 +- .../build/derivation-building-goal.cc | 20 +- ...erivation-creation-and-realisation-goal.cc | 177 +++++++++++++++ src/libstore/build/derivation-goal.cc | 202 ++++-------------- src/libstore/build/entry-points.cc | 26 +-- src/libstore/build/goal.cc | 24 --- src/libstore/build/worker.cc | 99 +++++---- src/libstore/derived-path-map.cc | 4 +- ...erivation-creation-and-realisation-goal.hh | 91 ++++++++ .../nix/store/build/derivation-goal.hh | 58 ++--- src/libstore/include/nix/store/build/goal.hh | 14 -- .../include/nix/store/build/worker.hh | 25 ++- src/libstore/include/nix/store/meson.build | 1 + src/libstore/meson.build | 1 + 14 files changed, 409 insertions(+), 335 deletions(-) create mode 100644 src/libstore/build/derivation-creation-and-realisation-goal.cc create mode 100644 src/libstore/include/nix/store/build/derivation-creation-and-realisation-goal.hh diff --git a/src/libfetchers/include/nix/fetchers/input-cache.hh b/src/libfetchers/include/nix/fetchers/input-cache.hh index 9b1c5a310..f9278053a 100644 --- a/src/libfetchers/include/nix/fetchers/input-cache.hh +++ b/src/libfetchers/include/nix/fetchers/input-cache.hh @@ -1,4 +1,4 @@ -#include "fetchers.hh" +#include "nix/fetchers/fetchers.hh" namespace nix::fetchers { diff --git a/src/libstore/build/derivation-building-goal.cc b/src/libstore/build/derivation-building-goal.cc index 14c427b83..3c02514d9 100644 --- a/src/libstore/build/derivation-building-goal.cc +++ b/src/libstore/build/derivation-building-goal.cc @@ -1,5 +1,5 @@ #include "nix/store/build/derivation-building-goal.hh" -#include "nix/store/build/derivation-goal.hh" +#include "nix/store/build/derivation-creation-and-realisation-goal.hh" #ifndef _WIN32 // TODO enable build hook on Windows # include "nix/store/build/hook-instance.hh" # include "nix/store/build/derivation-builder.hh" @@ -264,7 +264,7 @@ Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution() auto mEntry = get(inputGoals, drvPath); if (!mEntry) return std::nullopt; - auto buildResult = (*mEntry)->getBuildResult(DerivedPath::Built{drvPath, OutputsSpec::Names{outputName}}); + auto & buildResult = (*mEntry)->buildResult; if (!buildResult.success()) return std::nullopt; auto i = get(buildResult.builtOutputs, outputName); @@ -295,8 +295,8 @@ Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution() }); // FIXME wanted outputs - auto resolvedDrvGoal = worker.makeDerivationGoal( - makeConstantStorePathRef(pathResolved), OutputsSpec::All{}, buildMode); + auto resolvedDrvGoal = worker.makeDerivationCreationAndRealisationGoal( + pathResolved, OutputsSpec::All{}, drvResolved, buildMode); { Goals waitees{resolvedDrvGoal}; co_await await(std::move(waitees)); @@ -304,20 +304,16 @@ Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution() trace("resolved derivation finished"); - auto resolvedDrv = *resolvedDrvGoal->drv; - auto resolvedResult = resolvedDrvGoal->getBuildResult(DerivedPath::Built{ - .drvPath = makeConstantStorePathRef(pathResolved), - .outputs = OutputsSpec::All{}, - }); + auto resolvedResult = resolvedDrvGoal->buildResult; SingleDrvOutputs builtOutputs; if (resolvedResult.success()) { - auto resolvedHashes = staticOutputHashes(worker.store, resolvedDrv); + auto resolvedHashes = staticOutputHashes(worker.store, drvResolved); StorePathSet outputPaths; - for (auto & outputName : resolvedDrv.outputNames()) { + for (auto & outputName : drvResolved.outputNames()) { auto initialOutput = get(initialOutputs, outputName); auto resolvedHash = get(resolvedHashes, outputName); if ((!initialOutput) || (!resolvedHash)) @@ -338,7 +334,7 @@ Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution() throw Error( "derivation '%s' doesn't have expected output '%s' (derivation-goal.cc/realisation)", - resolvedDrvGoal->drvReq->to_string(worker.store), outputName); + worker.store.printStorePath(pathResolved), outputName); }(); if (!drv->type().isImpure()) { diff --git a/src/libstore/build/derivation-creation-and-realisation-goal.cc b/src/libstore/build/derivation-creation-and-realisation-goal.cc new file mode 100644 index 000000000..777eabfe7 --- /dev/null +++ b/src/libstore/build/derivation-creation-and-realisation-goal.cc @@ -0,0 +1,177 @@ +#include "nix/store/build/derivation-creation-and-realisation-goal.hh" +#include "nix/store/build/worker.hh" +#include "nix/store/derivations.hh" + +namespace nix { + +DerivationCreationAndRealisationGoal::DerivationCreationAndRealisationGoal( + ref drvReq, const OutputsSpec & wantedOutputs, Worker & worker, BuildMode buildMode) + : Goal(worker, init()) + , drvReq(drvReq) + , wantedOutputs(wantedOutputs) + , buildMode(buildMode) +{ + commonInit(); +} + +DerivationCreationAndRealisationGoal::DerivationCreationAndRealisationGoal( + const StorePath & drvPath, + const OutputsSpec & wantedOutputs, + const Derivation & drv, + Worker & worker, + BuildMode buildMode) + : Goal(worker, haveDerivation(drvPath, drv)) + , drvReq(makeConstantStorePathRef(drvPath)) + , wantedOutputs(wantedOutputs) + , buildMode(buildMode) +{ + commonInit(); +} + +void DerivationCreationAndRealisationGoal::commonInit() +{ + 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(); +} + +DerivationCreationAndRealisationGoal::~DerivationCreationAndRealisationGoal() {} + +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 DerivationCreationAndRealisationGoal::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()) + "$" + DerivedPath::Built{ + .drvPath = drvReq, + .outputs = wantedOutputs, + }.to_string(worker.store); +} + +void DerivationCreationAndRealisationGoal::timedOut(Error && ex) {} + +Goal::Co DerivationCreationAndRealisationGoal::init() +{ + trace("need to load derivation from file"); + + /* The first thing to do is to make sure that the derivation + exists. If it doesn't, it may be built from another derivation, + or merely substituted. We can make goal to get it and not worry + about which method it takes to get the derivation. */ + if (auto optDrvPath = [this]() -> std::optional { + 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"); + Goals waitees{worker.makeGoal(DerivedPath::fromSingle(*drvReq))}; + co_await await(std::move(waitees)); + } + + 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); + + /* `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. */ + worker.evalStore.addTempRoot(drvPath); + + /* Get the derivation. It is probably in the eval store, but it might be inthe main store: + + - Resolved derivation are resolved against main store realisations, and so must be stored there. + + - Dynamic derivations are built, and so are found in the main store. + */ + auto drv = [&] { + for (auto * drvStore : {&worker.evalStore, &worker.store}) + if (drvStore->isValidPath(drvPath)) + return drvStore->readDerivation(drvPath); + assert(false); + }(); + + co_return haveDerivation(std::move(drvPath), std::move(drv)); +} + +Goal::Co DerivationCreationAndRealisationGoal::haveDerivation(StorePath drvPath, Derivation drv) +{ + trace("have derivation, will kick off derivations goals per wanted output"); + + auto resolvedWantedOutputs = std::visit( + overloaded{ + [&](const OutputsSpec::Names & names) -> OutputsSpec::Names { return names; }, + [&](const OutputsSpec::All &) -> OutputsSpec::Names { + StringSet outputs; + for (auto & [outputName, _] : drv.outputs) + outputs.insert(outputName); + return outputs; + }, + }, + wantedOutputs.raw); + + Goals concreteDrvGoals; + + /* Build this step! */ + + for (auto & output : resolvedWantedOutputs) { + auto g = upcast_goal(worker.makeDerivationGoal(drvPath, drv, output, buildMode)); + g->preserveException = true; + /* We will finish with it ourselves, as if we were the derivational goal. */ + concreteDrvGoals.insert(std::move(g)); + } + + optDrvPath = std::move(drvPath); + + // Copy on purpose + co_await await(Goals(concreteDrvGoals)); + + trace("outer build done"); + + auto & g = *concreteDrvGoals.begin(); + buildResult = g->buildResult; + for (auto & g2 : concreteDrvGoals) { + for (auto && [x, y] : g2->buildResult.builtOutputs) + buildResult.builtOutputs.insert_or_assign(x, y); + } + + co_return amDone(g->exitCode, g->ex); +} + +} diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 63dca3083..57ea67b1b 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -24,35 +24,18 @@ namespace nix { -DerivationGoal::DerivationGoal(ref drvReq, - const OutputsSpec & wantedOutputs, Worker & worker, BuildMode buildMode) - : Goal(worker, loadDerivation()) - , drvReq(drvReq) - , wantedOutputs(wantedOutputs) - , buildMode(buildMode) -{ - name = fmt( - "building of '%s' from .drv file", - DerivedPath::Built { drvReq, wantedOutputs }.to_string(worker.store)); - trace("created"); - - mcExpectedBuilds = std::make_unique>(worker.expectedBuilds); - worker.updateProgress(); -} - - -DerivationGoal::DerivationGoal(const StorePath & drvPath, const BasicDerivation & drv, - const OutputsSpec & wantedOutputs, Worker & worker, BuildMode buildMode) - : Goal(worker, haveDerivation(drvPath)) - , drvReq(makeConstantStorePathRef(drvPath)) - , wantedOutputs(wantedOutputs) +DerivationGoal::DerivationGoal(const StorePath & drvPath, const Derivation & drv, + const OutputName & wantedOutput, Worker & worker, BuildMode buildMode) + : Goal(worker, haveDerivation()) + , drvPath(drvPath) + , wantedOutput(wantedOutput) , buildMode(buildMode) { this->drv = std::make_unique(drv); name = fmt( "building of '%s' from in-memory derivation", - DerivedPath::Built { drvReq, drv.outputNames() }.to_string(worker.store)); + DerivedPath::Built { makeConstantStorePathRef(drvPath), drv.outputNames() }.to_string(worker.store)); trace("created"); mcExpectedBuilds = std::make_unique>(worker.expectedBuilds); @@ -61,114 +44,20 @@ DerivationGoal::DerivationGoal(const StorePath & drvPath, const BasicDerivation } -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 DerivationGoal::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 "b$"). */ - return "b$" + std::string(pathPartOfReq(*drvReq).name()) + "$" + drvReq->to_string(worker.store); + return "b$" + std::string(drvPath.name()) + "$" + SingleDerivedPath::Built{ + .drvPath = makeConstantStorePathRef(drvPath), + .output = wantedOutput, + }.to_string(worker.store); } -void DerivationGoal::addWantedOutputs(const OutputsSpec & outputs) -{ - auto newWanted = wantedOutputs.union_(outputs); - switch (needRestart) { - case NeedRestartForMoreOutputs::OutputsUnmodifedDontNeed: - if (!newWanted.isSubsetOf(wantedOutputs)) - needRestart = NeedRestartForMoreOutputs::OutputsAddedDoNeed; - break; - case NeedRestartForMoreOutputs::OutputsAddedDoNeed: - /* No need to check whether we added more outputs, because a - restart is already queued up. */ - break; - case NeedRestartForMoreOutputs::BuildInProgressWillNotNeed: - /* We are already building all outputs, so it doesn't matter if - we now want more. */ - break; - }; - wantedOutputs = newWanted; -} - - -Goal::Co DerivationGoal::loadDerivation() { - trace("need to load derivation from file"); - - { - /* The first thing to do is to make sure that the derivation - exists. If it doesn't, it may be built from another - derivation, or merely substituted. We can make goal to get it - and not worry about which method it takes to get the - derivation. */ - - if (auto optDrvPath = [this]() -> std::optional { - 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"); - Goals waitees{worker.makeGoal(DerivedPath::fromSingle(*drvReq))}; - co_await await(std::move(waitees)); - } - - trace("loading 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); - - /* `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. */ - worker.evalStore.addTempRoot(drvPath); - - /* Get the derivation. It is probably in the eval store, but it might be inthe main store: - - - Resolved derivation are resolved against main store realisations, and so must be stored there. - - - Dynamic derivations are built, and so are found in the main store. - */ - for (auto * drvStore : { &worker.evalStore, &worker.store }) { - if (drvStore->isValidPath(drvPath)) { - drv = std::make_unique(drvStore->readDerivation(drvPath)); - break; - } - } - assert(drv); - - co_return haveDerivation(drvPath); - } -} - - -Goal::Co DerivationGoal::haveDerivation(StorePath drvPath) +Goal::Co DerivationGoal::haveDerivation() { trace("have derivation"); @@ -205,10 +94,17 @@ Goal::Co DerivationGoal::haveDerivation(StorePath drvPath) trace("outer build done"); - buildResult = g->getBuildResult(DerivedPath::Built{ - .drvPath = makeConstantStorePathRef(drvPath), - .outputs = wantedOutputs, - }); + buildResult = g->buildResult; + for (auto it = buildResult.builtOutputs.begin(); it != buildResult.builtOutputs.end(); ) { + if (it->first != wantedOutput) { + it = buildResult.builtOutputs.erase(it); + } else { + ++it; + } + } + + if (buildResult.success()) + assert(buildResult.builtOutputs.count(wantedOutput) > 0); co_return amDone(g->exitCode, g->ex); }; @@ -254,11 +150,11 @@ Goal::Co DerivationGoal::haveDerivation(StorePath drvPath) { /* Check what outputs paths are not already valid. */ - auto [allValid, validOutputs] = checkPathValidity(drvPath); + auto [allValid, validOutputs] = checkPathValidity(); /* If they are all valid, then we're done. */ if (allValid && buildMode == bmNormal) { - co_return done(drvPath, BuildResult::AlreadyValid, std::move(validOutputs)); + co_return done(BuildResult::AlreadyValid, std::move(validOutputs)); } } @@ -295,25 +191,20 @@ Goal::Co DerivationGoal::haveDerivation(StorePath drvPath) assert(!drv->type().isImpure()); if (nrFailed > 0 && nrFailed > nrNoSubstituters && !settings.tryFallback) { - co_return done(drvPath, BuildResult::TransientFailure, {}, + co_return done(BuildResult::TransientFailure, {}, Error("some substitutes for the outputs of derivation '%s' failed (usually happens due to networking issues); try '--fallback' to build derivation from source ", worker.store.printStorePath(drvPath))); } nrFailed = nrNoSubstituters = 0; - if (needRestart == NeedRestartForMoreOutputs::OutputsAddedDoNeed) { - needRestart = NeedRestartForMoreOutputs::OutputsUnmodifedDontNeed; - co_return haveDerivation(std::move(drvPath)); - } - - auto [allValid, validOutputs] = checkPathValidity(drvPath); + auto [allValid, validOutputs] = checkPathValidity(); if (buildMode == bmNormal && allValid) { - co_return done(drvPath, BuildResult::Substituted, std::move(validOutputs)); + co_return done(BuildResult::Substituted, std::move(validOutputs)); } if (buildMode == bmRepair && allValid) { - co_return repairClosure(std::move(drvPath)); + co_return repairClosure(); } if (buildMode == bmCheck && !allValid) throw Error("some outputs of '%s' are not valid, so checking is not possible", @@ -336,7 +227,7 @@ struct value_comparison }; -Goal::Co DerivationGoal::repairClosure(StorePath drvPath) +Goal::Co DerivationGoal::repairClosure() { assert(!drv->type().isImpure()); @@ -346,11 +237,10 @@ Goal::Co DerivationGoal::repairClosure(StorePath drvPath) that produced those outputs. */ /* Get the output closure. */ - auto outputs = queryDerivationOutputMap(drvPath); + auto outputs = queryDerivationOutputMap(); StorePathSet outputClosure; - for (auto & i : outputs) { - if (!wantedOutputs.contains(i.first)) continue; - worker.store.computeFSClosure(i.second, outputClosure); + if (auto mPath = get(outputs, wantedOutput)) { + worker.store.computeFSClosure(*mPath, outputClosure); } /* Filter out our own outputs (which we have already checked). */ @@ -404,11 +294,11 @@ Goal::Co DerivationGoal::repairClosure(StorePath drvPath) throw Error("some paths in the output closure of derivation '%s' could not be repaired", worker.store.printStorePath(drvPath)); } - co_return done(drvPath, BuildResult::AlreadyValid, assertPathValidity(drvPath)); + co_return done(BuildResult::AlreadyValid, assertPathValidity()); } -std::map> DerivationGoal::queryPartialDerivationOutputMap(const StorePath & drvPath) +std::map> DerivationGoal::queryPartialDerivationOutputMap() { assert(!drv->type().isImpure()); @@ -424,7 +314,7 @@ std::map> DerivationGoal::queryPartialDeri return res; } -OutputPathMap DerivationGoal::queryDerivationOutputMap(const StorePath & drvPath) +OutputPathMap DerivationGoal::queryDerivationOutputMap() { assert(!drv->type().isImpure()); @@ -440,28 +330,21 @@ OutputPathMap DerivationGoal::queryDerivationOutputMap(const StorePath & drvPath } -std::pair DerivationGoal::checkPathValidity(const StorePath & drvPath) +std::pair DerivationGoal::checkPathValidity() { if (drv->type().isImpure()) return { false, {} }; bool checkHash = buildMode == bmRepair; - auto wantedOutputsLeft = std::visit(overloaded { - [&](const OutputsSpec::All &) { - return StringSet {}; - }, - [&](const OutputsSpec::Names & names) { - return static_cast(names); - }, - }, wantedOutputs.raw); + StringSet wantedOutputsLeft{wantedOutput}; SingleDrvOutputs validOutputs; - for (auto & i : queryPartialDerivationOutputMap(drvPath)) { + for (auto & i : queryPartialDerivationOutputMap()) { auto initialOutput = get(initialOutputs, i.first); if (!initialOutput) // this is an invalid output, gets catched with (!wantedOutputsLeft.empty()) continue; auto & info = *initialOutput; - info.wanted = wantedOutputs.contains(i.first); + info.wanted = wantedOutput == i.first; if (info.wanted) wantedOutputsLeft.erase(i.first); if (i.second) { @@ -520,9 +403,9 @@ std::pair DerivationGoal::checkPathValidity(const StoreP } -SingleDrvOutputs DerivationGoal::assertPathValidity(const StorePath & drvPath) +SingleDrvOutputs DerivationGoal::assertPathValidity() { - auto [allValid, validOutputs] = checkPathValidity(drvPath); + auto [allValid, validOutputs] = checkPathValidity(); if (!allValid) throw Error("some outputs are unexpectedly invalid"); return validOutputs; @@ -530,7 +413,6 @@ SingleDrvOutputs DerivationGoal::assertPathValidity(const StorePath & drvPath) Goal::Done DerivationGoal::done( - const StorePath & drvPath, BuildResult::Status status, SingleDrvOutputs builtOutputs, std::optional ex) @@ -546,7 +428,7 @@ Goal::Done DerivationGoal::done( mcExpectedBuilds.reset(); if (buildResult.success()) { - auto wantedBuiltOutputs = filterDrvOutputs(wantedOutputs, std::move(builtOutputs)); + auto wantedBuiltOutputs = filterDrvOutputs(OutputsSpec::Names{wantedOutput}, std::move(builtOutputs)); assert(!wantedBuiltOutputs.empty()); buildResult.builtOutputs = std::move(wantedBuiltOutputs); if (status == BuildResult::Built) diff --git a/src/libstore/build/entry-points.cc b/src/libstore/build/entry-points.cc index 39fd471c4..e74652523 100644 --- a/src/libstore/build/entry-points.cc +++ b/src/libstore/build/entry-points.cc @@ -1,8 +1,7 @@ +#include "nix/store/derivations.hh" #include "nix/store/build/worker.hh" #include "nix/store/build/substitution-goal.hh" -#ifndef _WIN32 // TODO Enable building on Windows -# include "nix/store/build/derivation-goal.hh" -#endif +#include "nix/store/build/derivation-creation-and-realisation-goal.hh" #include "nix/store/local-store.hh" #include "nix/util/strings.hh" @@ -28,12 +27,9 @@ void Store::buildPaths(const std::vector & reqs, BuildMode buildMod ex = std::move(i->ex); } if (i->exitCode != Goal::ecSuccess) { -#ifndef _WIN32 // TODO Enable building on Windows - if (auto i2 = dynamic_cast(i.get())) + if (auto i2 = dynamic_cast(i.get())) failed.insert(i2->drvReq->to_string(*this)); - else -#endif - if (auto i2 = dynamic_cast(i.get())) + else if (auto i2 = dynamic_cast(i.get())) failed.insert(printStorePath(i2->storePath)); } } @@ -70,7 +66,7 @@ std::vector Store::buildPathsWithResults( for (auto & [req, goalPtr] : state) results.emplace_back(KeyedBuildResult { - goalPtr->getBuildResult(req), + goalPtr->buildResult, /* .path = */ req, }); @@ -81,19 +77,11 @@ BuildResult Store::buildDerivation(const StorePath & drvPath, const BasicDerivat BuildMode buildMode) { Worker worker(*this, *this); -#ifndef _WIN32 // TODO Enable building on Windows - auto goal = worker.makeBasicDerivationGoal(drvPath, drv, OutputsSpec::All {}, buildMode); -#else - std::shared_ptr goal; - throw UnimplementedError("Building derivations not yet implemented on windows."); -#endif + auto goal = worker.makeDerivationCreationAndRealisationGoal(drvPath, OutputsSpec::All {}, drv, buildMode); try { worker.run(Goals{goal}); - return goal->getBuildResult(DerivedPath::Built { - .drvPath = makeConstantStorePathRef(drvPath), - .outputs = OutputsSpec::All {}, - }); + return goal->buildResult; } catch (Error & e) { return BuildResult { .status = BuildResult::MiscFailure, diff --git a/src/libstore/build/goal.cc b/src/libstore/build/goal.cc index 8a8d79283..88b0c28c0 100644 --- a/src/libstore/build/goal.cc +++ b/src/libstore/build/goal.cc @@ -101,30 +101,6 @@ bool CompareGoalPtrs::operator() (const GoalPtr & a, const GoalPtr & b) const { return s1 < s2; } - -BuildResult Goal::getBuildResult(const DerivedPath & req) const { - BuildResult res { buildResult }; - - if (auto pbp = std::get_if(&req)) { - auto & bp = *pbp; - - /* Because goals are in general shared between derived paths - that share the same derivation, we need to filter their - results to get back just the results we care about. - */ - - for (auto it = res.builtOutputs.begin(); it != res.builtOutputs.end();) { - if (bp.outputs.contains(it->first)) - ++it; - else - it = res.builtOutputs.erase(it); - } - } - - return res; -} - - void addToWeakGoals(WeakGoals & goals, GoalPtr p) { if (goals.find(p) != goals.end()) diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index 12d4aaa2d..7dd14a688 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -5,6 +5,7 @@ #include "nix/store/build/drv-output-substitution-goal.hh" #include "nix/store/build/derivation-goal.hh" #include "nix/store/build/derivation-building-goal.hh" +#include "nix/store/build/derivation-creation-and-realisation-goal.hh" #ifndef _WIN32 // TODO Enable building on Windows # include "nix/store/build/hook-instance.hh" #endif @@ -53,52 +54,40 @@ std::shared_ptr Worker::initGoalIfNeeded(std::weak_ptr & goal_weak, Args & return goal; } -std::shared_ptr Worker::makeDerivationGoalCommon( +std::shared_ptr Worker::makeDerivationCreationAndRealisationGoal( ref drvReq, const OutputsSpec & wantedOutputs, - std::function()> mkDrvGoal) + BuildMode buildMode) { - std::weak_ptr & goal_weak = derivationGoals.ensureSlot(*drvReq).value; - std::shared_ptr goal = goal_weak.lock(); - if (!goal) { - goal = mkDrvGoal(); - goal_weak = goal; - wakeUp(goal); - } else { - goal->addWantedOutputs(wantedOutputs); - } - return goal; + return initGoalIfNeeded( + derivationCreationAndRealisationGoals.ensureSlot(*drvReq).value[wantedOutputs], + drvReq, wantedOutputs, *this, buildMode); } -std::shared_ptr Worker::makeDerivationGoal(ref drvReq, - const OutputsSpec & wantedOutputs, BuildMode buildMode) +std::shared_ptr Worker::makeDerivationCreationAndRealisationGoal( + const StorePath & drvPath, + const OutputsSpec & wantedOutputs, + const Derivation & drv, + BuildMode buildMode) { - return makeDerivationGoalCommon(drvReq, wantedOutputs, [&]() -> std::shared_ptr { - return std::make_shared(drvReq, wantedOutputs, *this, buildMode); - }); + return initGoalIfNeeded( + derivationCreationAndRealisationGoals.ensureSlot(DerivedPath::Opaque{drvPath}).value[wantedOutputs], + drvPath, wantedOutputs, drv, *this, buildMode); } -std::shared_ptr Worker::makeBasicDerivationGoal(const StorePath & drvPath, - const BasicDerivation & drv, const OutputsSpec & wantedOutputs, BuildMode buildMode) + +std::shared_ptr Worker::makeDerivationGoal(const StorePath & drvPath, + const Derivation & drv, const OutputName & wantedOutput, BuildMode buildMode) { - return makeDerivationGoalCommon(makeConstantStorePathRef(drvPath), wantedOutputs, [&]() -> std::shared_ptr { - return std::make_shared(drvPath, drv, wantedOutputs, *this, buildMode); - }); + return initGoalIfNeeded(derivationGoals[drvPath][wantedOutput], drvPath, drv, wantedOutput, *this, buildMode); } std::shared_ptr Worker::makeDerivationBuildingGoal(const StorePath & drvPath, const Derivation & drv, BuildMode buildMode) { - std::weak_ptr & goal_weak = derivationBuildingGoals[drvPath]; - auto goal = goal_weak.lock(); // FIXME - if (!goal) { - goal = std::make_shared(drvPath, drv, *this, buildMode); - goal_weak = goal; - wakeUp(goal); - } - return goal; + return initGoalIfNeeded(derivationBuildingGoals[drvPath], drvPath, drv, *this, buildMode); } @@ -118,7 +107,7 @@ GoalPtr Worker::makeGoal(const DerivedPath & req, BuildMode buildMode) { return std::visit(overloaded { [&](const DerivedPath::Built & bfd) -> GoalPtr { - return makeDerivationGoal(bfd.drvPath, bfd.outputs, buildMode); + return makeDerivationCreationAndRealisationGoal(bfd.drvPath, bfd.outputs, buildMode); }, [&](const DerivedPath::Opaque & bo) -> GoalPtr { return makePathSubstitutionGoal(bo.path, buildMode == bmRepair ? Repair : NoRepair); @@ -128,44 +117,52 @@ GoalPtr Worker::makeGoal(const DerivedPath & req, BuildMode buildMode) template -static void cullMap(std::map & goalMap, F f) +static void cullMap(std::map & goalMap, F && f) { for (auto i = goalMap.begin(); i != goalMap.end();) - if (!f(i->second)) + if (!std::forward(f)(i->second)) i = goalMap.erase(i); else ++i; } -template -static void removeGoal(std::shared_ptr goal, std::map> & goalMap) +template +static bool removeGoal(std::shared_ptr goal, std::weak_ptr & gp) { - /* !!! inefficient */ - cullMap(goalMap, [&](const std::weak_ptr & gp) -> bool { - return gp.lock() != goal; - }); + return gp.lock() != goal; } -template -static void removeGoal(std::shared_ptr goal, std::map>::ChildNode> & goalMap); -template -static void removeGoal(std::shared_ptr goal, std::map>::ChildNode> & goalMap) +template +static bool removeGoal(std::shared_ptr goal, std::map & goalMap); + +template +static bool removeGoal(std::shared_ptr goal, std::map & goalMap) { /* !!! inefficient */ - cullMap(goalMap, [&](DerivedPathMap>::ChildNode & node) -> bool { - if (node.value.lock() == goal) - node.value.reset(); - removeGoal(goal, node.childMap); - return !node.value.expired() || !node.childMap.empty(); + cullMap(goalMap, [&](Inner & inner) -> bool { + return removeGoal(goal, inner); }); + return !goalMap.empty(); +} + + +template +static bool removeGoal(std::shared_ptr goal, typename DerivedPathMap>>::ChildNode & node); + +template +static bool removeGoal(std::shared_ptr goal, typename DerivedPathMap>>::ChildNode & node) +{ + return removeGoal(goal, node.value) || removeGoal(goal, node.childMap); } void Worker::removeGoal(GoalPtr goal) { - if (auto drvGoal = std::dynamic_pointer_cast(goal)) - nix::removeGoal(drvGoal, derivationGoals.map); + if (auto drvGoal = std::dynamic_pointer_cast(goal)) + nix::removeGoal(drvGoal, derivationCreationAndRealisationGoals.map); + else if (auto drvGoal = std::dynamic_pointer_cast(goal)) + nix::removeGoal(drvGoal, derivationGoals); else if (auto drvBuildingGoal = std::dynamic_pointer_cast(goal)) nix::removeGoal(drvBuildingGoal, derivationBuildingGoals); else if (auto subGoal = std::dynamic_pointer_cast(goal)) @@ -312,7 +309,7 @@ void Worker::run(const Goals & _topGoals) for (auto & i : _topGoals) { topGoals.insert(i); - if (auto goal = dynamic_cast(i.get())) { + if (auto goal = dynamic_cast(i.get())) { topPaths.push_back(DerivedPath::Built { .drvPath = goal->drvReq, .outputs = goal->wantedOutputs, diff --git a/src/libstore/derived-path-map.cc b/src/libstore/derived-path-map.cc index 408d1a6b9..c5856701a 100644 --- a/src/libstore/derived-path-map.cc +++ b/src/libstore/derived-path-map.cc @@ -52,7 +52,7 @@ typename DerivedPathMap::ChildNode * DerivedPathMap::findSlot(const Single // instantiations -#include "nix/store/build/derivation-goal.hh" +#include "nix/store/build/derivation-creation-and-realisation-goal.hh" namespace nix { template<> @@ -69,7 +69,7 @@ std::strong_ordering DerivedPathMap::ChildNode::operator <=> ( template struct DerivedPathMap::ChildNode; template struct DerivedPathMap; -template struct DerivedPathMap>; +template struct DerivedPathMap>>; }; diff --git a/src/libstore/include/nix/store/build/derivation-creation-and-realisation-goal.hh b/src/libstore/include/nix/store/build/derivation-creation-and-realisation-goal.hh new file mode 100644 index 000000000..4eccc54e6 --- /dev/null +++ b/src/libstore/include/nix/store/build/derivation-creation-and-realisation-goal.hh @@ -0,0 +1,91 @@ +#pragma once + +#include "nix/store/parsed-derivations.hh" +#include "nix/store/store-api.hh" +#include "nix/store/pathlocks.hh" +#include "nix/store/build/goal.hh" + +namespace nix { + +/** + * 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* + * `DerivationCreationAndRealisationGoal`. 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 evaluator 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 DerivationCreationAndRealisationGoal : public Goal +{ + /** + * How to obtain a store path of the derivation to build. + */ + ref drvReq; + + /** + * The path of the derivation, once obtained. + **/ + std::optional optDrvPath; + + /** + * 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; + + DerivationCreationAndRealisationGoal( + ref drvReq, + const OutputsSpec & wantedOutputs, + Worker & worker, + BuildMode buildMode = bmNormal); + + DerivationCreationAndRealisationGoal( + const StorePath & drvPath, + const OutputsSpec & wantedOutputs, + const Derivation & drv, + Worker & worker, + BuildMode buildMode = bmNormal); + + virtual ~DerivationCreationAndRealisationGoal(); + + void timedOut(Error && ex) override; + + std::string key() override; + + Co init(); + Co haveDerivation(StorePath drvPath, Derivation drv); + + JobCategory jobCategory() const override + { + return JobCategory::Administration; + }; + +private: + /** + * Shared between both constructors + */ + void commonInit(); +}; + +} diff --git a/src/libstore/include/nix/store/build/derivation-goal.hh b/src/libstore/include/nix/store/build/derivation-goal.hh index b422aec7a..b0a69d41c 100644 --- a/src/libstore/include/nix/store/build/derivation-goal.hh +++ b/src/libstore/include/nix/store/build/derivation-goal.hh @@ -23,46 +23,23 @@ void runPostBuildHook( /** * 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 + * `DerivationCreationAndRealisationGoal` goal must be used instead. */ struct DerivationGoal : public Goal { /** The path of the derivation. */ - ref drvReq; + StorePath drvPath; /** * The specific outputs that we need to build. */ - OutputsSpec wantedOutputs; + OutputName wantedOutput; /** - * See `needRestart`; just for that field. - */ - enum struct NeedRestartForMoreOutputs { - /** - * The goal state machine is progressing based on the current value of - * `wantedOutputs. No actions are needed. - */ - OutputsUnmodifedDontNeed, - /** - * `wantedOutputs` has been extended, but the state machine is - * proceeding according to its old value, so we need to restart. - */ - OutputsAddedDoNeed, - /** - * The goal state machine has progressed to the point of doing a build, - * in which case all outputs will be produced, so extensions to - * `wantedOutputs` no longer require a restart. - */ - BuildInProgressWillNotNeed, - }; - - /** - * Whether additional wanted outputs have been added. - */ - NeedRestartForMoreOutputs needRestart = NeedRestartForMoreOutputs::OutputsUnmodifedDontNeed; - - /** - * The derivation stored at `drvReq`. + * The derivation stored at drvPath. */ std::unique_ptr drv; @@ -76,11 +53,8 @@ struct DerivationGoal : public Goal std::unique_ptr> mcExpectedBuilds; - DerivationGoal(ref drvReq, - const OutputsSpec & wantedOutputs, Worker & worker, - BuildMode buildMode = bmNormal); - DerivationGoal(const StorePath & drvPath, const BasicDerivation & drv, - const OutputsSpec & wantedOutputs, Worker & worker, + DerivationGoal(const StorePath & drvPath, const Derivation & drv, + const OutputName & wantedOutput, Worker & worker, BuildMode buildMode = bmNormal); ~DerivationGoal() = default; @@ -96,16 +70,15 @@ struct DerivationGoal : public Goal /** * The states. */ - Co loadDerivation(); - Co haveDerivation(StorePath drvPath); + Co haveDerivation(); /** * Wrappers around the corresponding Store methods that first consult the * derivation. This is currently needed because when there is no drv file * there also is no DB entry. */ - std::map> queryPartialDerivationOutputMap(const StorePath & drvPath); - OutputPathMap queryDerivationOutputMap(const StorePath & drvPath); + std::map> queryPartialDerivationOutputMap(); + OutputPathMap queryDerivationOutputMap(); /** * Update 'initialOutputs' to determine the current status of the @@ -113,18 +86,17 @@ struct DerivationGoal : public Goal * whether all outputs are valid and non-corrupt, and a * 'SingleDrvOutputs' structure containing the valid outputs. */ - std::pair checkPathValidity(const StorePath & drvPath); + std::pair checkPathValidity(); /** * Aborts if any output is not valid or corrupt, and otherwise * returns a 'SingleDrvOutputs' structure containing all outputs. */ - SingleDrvOutputs assertPathValidity(const StorePath & drvPath); + SingleDrvOutputs assertPathValidity(); - Co repairClosure(StorePath drvPath); + Co repairClosure(); Done done( - const StorePath & drvPath, BuildResult::Status status, SingleDrvOutputs builtOutputs = {}, std::optional ex = {}); diff --git a/src/libstore/include/nix/store/build/goal.hh b/src/libstore/include/nix/store/build/goal.hh index 577ce1e84..ee69c9cc7 100644 --- a/src/libstore/include/nix/store/build/goal.hh +++ b/src/libstore/include/nix/store/build/goal.hh @@ -105,13 +105,11 @@ public: */ ExitCode exitCode = ecBusy; -protected: /** * Build result. */ BuildResult buildResult; -public: /** * Suspend our goal and wait until we get `work`-ed again. * `co_await`-able by @ref Co. @@ -358,18 +356,6 @@ protected: public: virtual void cleanup() { } - /** - * Project a `BuildResult` with just the information that pertains - * to the given request. - * - * In general, goals may be aliased between multiple requests, and - * the stored `BuildResult` has information for the union of all - * requests. We don't want to leak what the other request are for - * sake of both privacy and determinism, and this "safe accessor" - * ensures we don't. - */ - 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 diff --git a/src/libstore/include/nix/store/build/worker.hh b/src/libstore/include/nix/store/build/worker.hh index c70c72377..5f83ab76b 100644 --- a/src/libstore/include/nix/store/build/worker.hh +++ b/src/libstore/include/nix/store/build/worker.hh @@ -14,6 +14,7 @@ namespace nix { /* Forward definition. */ +struct DerivationCreationAndRealisationGoal; struct DerivationGoal; struct DerivationBuildingGoal; struct PathSubstitutionGoal; @@ -33,6 +34,7 @@ class DrvOutputSubstitutionGoal; */ GoalPtr upcast_goal(std::shared_ptr subGoal); GoalPtr upcast_goal(std::shared_ptr subGoal); +GoalPtr upcast_goal(std::shared_ptr subGoal); typedef std::chrono::time_point steady_time_point; @@ -106,8 +108,9 @@ private: * same derivation / path. */ - DerivedPathMap> derivationGoals; + DerivedPathMap>> derivationCreationAndRealisationGoals; + std::map>> derivationGoals; std::map> derivationBuildingGoals; std::map> substitutionGoals; std::map> drvOutputSubstitutionGoals; @@ -204,16 +207,20 @@ private: template std::shared_ptr initGoalIfNeeded(std::weak_ptr & goal_weak, Args && ...args); - std::shared_ptr makeDerivationGoalCommon( - ref drvReq, const OutputsSpec & wantedOutputs, - std::function()> mkDrvGoal); -public: - std::shared_ptr makeDerivationGoal( + std::shared_ptr makeDerivationCreationAndRealisationGoal( ref drvReq, const OutputsSpec & wantedOutputs, BuildMode buildMode = bmNormal); - std::shared_ptr makeBasicDerivationGoal( - const StorePath & drvPath, const BasicDerivation & drv, - const OutputsSpec & wantedOutputs, BuildMode buildMode = bmNormal); + +public: + std::shared_ptr makeDerivationCreationAndRealisationGoal( + const StorePath & drvPath, + const OutputsSpec & wantedOutputs, + const Derivation & drv, + BuildMode buildMode = bmNormal); + + std::shared_ptr makeDerivationGoal( + const StorePath & drvPath, const Derivation & drv, + const OutputName & wantedOutput, BuildMode buildMode = bmNormal); /** * @ref DerivationBuildingGoal "derivation goal" diff --git a/src/libstore/include/nix/store/meson.build b/src/libstore/include/nix/store/meson.build index a18430417..f17847f77 100644 --- a/src/libstore/include/nix/store/meson.build +++ b/src/libstore/include/nix/store/meson.build @@ -15,6 +15,7 @@ headers = [config_pub_h] + files( 'build/derivation-goal.hh', 'build/derivation-building-goal.hh', 'build/derivation-building-misc.hh', + 'build/derivation-creation-and-realisation-goal.hh', 'build/drv-output-substitution-goal.hh', 'build/goal.hh', 'build/substitution-goal.hh', diff --git a/src/libstore/meson.build b/src/libstore/meson.build index 94b8951fd..a4762715a 100644 --- a/src/libstore/meson.build +++ b/src/libstore/meson.build @@ -255,6 +255,7 @@ sources = files( 'build-result.cc', 'build/derivation-goal.cc', 'build/derivation-building-goal.cc', + 'build/derivation-creation-and-realisation-goal.cc', 'build/drv-output-substitution-goal.cc', 'build/entry-points.cc', 'build/goal.cc',