diff --git a/src/libstore/build/derivation-building-goal.cc b/src/libstore/build/derivation-building-goal.cc index 9a91b3592..3019d9d72 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-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" @@ -72,7 +72,7 @@ std::string DerivationBuildingGoal::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$"). */ + derivation goals (due to "bd$"). */ return "bd$" + std::string(drvPath.name()) + "$" + worker.store.printStorePath(drvPath); } @@ -266,7 +266,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); @@ -296,9 +296,11 @@ Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution() worker.store.printStorePath(pathResolved), }); - // FIXME wanted outputs - auto resolvedDrvGoal = worker.makeDerivationGoal( - makeConstantStorePathRef(pathResolved), OutputsSpec::All{}, buildMode); + /* 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)); @@ -306,20 +308,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)) @@ -340,7 +338,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-goal.cc b/src/libstore/build/derivation-goal.cc index 3fcc376ed..128d360c9 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::OutputsUnmodifiedDontNeed: - 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,18 +94,26 @@ Goal::Co DerivationGoal::haveDerivation(StorePath drvPath) trace("outer build done"); - buildResult = g->getBuildResult(DerivedPath::Built{ - .drvPath = makeConstantStorePathRef(drvPath), - .outputs = wantedOutputs, - }); + buildResult = g->buildResult; if (buildMode == bmCheck) { /* In checking mode, the builder will not register any outputs. So we want to make sure the ones that we wanted to check are properly there. */ - buildResult.builtOutputs = assertPathValidity(drvPath); + buildResult.builtOutputs = assertPathValidity(); } + 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); }; @@ -261,11 +158,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)); } } @@ -302,25 +199,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::OutputsUnmodifiedDontNeed; - 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", @@ -343,7 +235,7 @@ struct value_comparison }; -Goal::Co DerivationGoal::repairClosure(StorePath drvPath) +Goal::Co DerivationGoal::repairClosure() { assert(!drv->type().isImpure()); @@ -353,11 +245,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). */ @@ -411,11 +302,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()); @@ -431,7 +322,7 @@ std::map> DerivationGoal::queryPartialDeri return res; } -OutputPathMap DerivationGoal::queryDerivationOutputMap(const StorePath & drvPath) +OutputPathMap DerivationGoal::queryDerivationOutputMap() { assert(!drv->type().isImpure()); @@ -447,28 +338,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 caught 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) { @@ -527,9 +411,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; @@ -537,7 +421,6 @@ SingleDrvOutputs DerivationGoal::assertPathValidity(const StorePath & drvPath) Goal::Done DerivationGoal::done( - const StorePath & drvPath, BuildResult::Status status, SingleDrvOutputs builtOutputs, std::optional ex) @@ -553,7 +436,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/derivation-trampoline-goal.cc b/src/libstore/build/derivation-trampoline-goal.cc new file mode 100644 index 000000000..e8ca47dfe --- /dev/null +++ b/src/libstore/build/derivation-trampoline-goal.cc @@ -0,0 +1,175 @@ +#include "nix/store/build/derivation-trampoline-goal.hh" +#include "nix/store/build/worker.hh" +#include "nix/store/derivations.hh" + +namespace nix { + +DerivationTrampolineGoal::DerivationTrampolineGoal( + ref drvReq, const OutputsSpec & wantedOutputs, Worker & worker, BuildMode buildMode) + : Goal(worker, init()) + , drvReq(drvReq) + , wantedOutputs(wantedOutputs) + , buildMode(buildMode) +{ + commonInit(); +} + +DerivationTrampolineGoal::DerivationTrampolineGoal( + 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 DerivationTrampolineGoal::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(); +} + +DerivationTrampolineGoal::~DerivationTrampolineGoal() {} + +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 DerivationTrampolineGoal::key() +{ + /* Ensure that derivations get built in order of their name, + i.e. a derivation named "aardvark" always comes before "baboon". And + substitution goals, derivation goals, and derivation building goals always happen before + derivation goals (due to "bt$"). */ + return "bt$" + std::string(pathPartOfReq(*drvReq).name()) + "$" + DerivedPath::Built{ + .drvPath = drvReq, + .outputs = wantedOutputs, + }.to_string(worker.store); +} + +void DerivationTrampolineGoal::timedOut(Error && ex) {} + +Goal::Co DerivationTrampolineGoal::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 in the 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 DerivationTrampolineGoal::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)); + } + + // 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/entry-points.cc b/src/libstore/build/entry-points.cc index 39fd471c4..6c842554c 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-trampoline-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.makeDerivationTrampolineGoal(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..38ac73768 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-trampoline-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::makeDerivationTrampolineGoal( 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( + derivationTrampolineGoals.ensureSlot(*drvReq).value[wantedOutputs], + drvReq, wantedOutputs, *this, buildMode); } -std::shared_ptr Worker::makeDerivationGoal(ref drvReq, - const OutputsSpec & wantedOutputs, BuildMode buildMode) +std::shared_ptr Worker::makeDerivationTrampolineGoal( + 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( + derivationTrampolineGoals.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 makeDerivationTrampolineGoal(bfd.drvPath, bfd.outputs, buildMode); }, [&](const DerivedPath::Opaque & bo) -> GoalPtr { return makePathSubstitutionGoal(bo.path, buildMode == bmRepair ? Repair : NoRepair); @@ -126,46 +115,52 @@ GoalPtr Worker::makeGoal(const DerivedPath & req, BuildMode buildMode) }, req.raw()); } - -template -static void cullMap(std::map & goalMap, F f) +/** + * This function is polymorphic (both via type parameters and + * overloading) and recursive in order to work on a various types of + * trees + * + * @return Whether the tree node we are processing is not empty / should + * be kept alive. In the case of this overloading the node in question + * is the leaf, the weak reference itself. If the weak reference points + * to the goal we are looking for, our caller can delete it. In the + * inductive case where the node is an interior node, we'll likewise + * return whether the interior node is non-empty. If it is empty + * (because we just deleted its last child), then our caller can + * likewise delete it. + */ +template +static bool removeGoal(std::shared_ptr goal, std::weak_ptr & gp) { - for (auto i = goalMap.begin(); i != goalMap.end();) - if (!f(i->second)) + return gp.lock() != goal; +} + +template +static bool removeGoal(std::shared_ptr goal, std::map & goalMap) +{ + /* !!! inefficient */ + for (auto i = goalMap.begin(); i != goalMap.end();) { + if (!removeGoal(goal, i->second)) i = goalMap.erase(i); - else ++i; + else + ++i; + } + return !goalMap.empty(); } - -template -static void removeGoal(std::shared_ptr goal, std::map> & goalMap) +template +static bool removeGoal(std::shared_ptr goal, typename DerivedPathMap>>::ChildNode & node) { - /* !!! inefficient */ - cullMap(goalMap, [&](const std::weak_ptr & gp) -> bool { - 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) -{ - /* !!! 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(); - }); + 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, derivationTrampolineGoals.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,13 +307,12 @@ 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, }); - } else - if (auto goal = dynamic_cast(i.get())) { + } else if (auto goal = dynamic_cast(i.get())) { topPaths.push_back(DerivedPath::Opaque{goal->storePath}); } } diff --git a/src/libstore/derived-path-map.cc b/src/libstore/derived-path-map.cc index 408d1a6b9..e34deb744 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-trampoline-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-building-goal.hh b/src/libstore/include/nix/store/build/derivation-building-goal.hh index bff2e7a89..569d1ddbb 100644 --- a/src/libstore/include/nix/store/build/derivation-building-goal.hh +++ b/src/libstore/include/nix/store/build/derivation-building-goal.hh @@ -29,7 +29,9 @@ void runPostBuildHook( const StorePathSet & outputPaths); /** - * A goal for building some or all of the outputs of a derivation. + * A goal for building a derivation. Substitution, (or any other method of + * obtaining the outputs) will not be attempted, so it is the calling goal's + * responsibility to try to substitute first. */ struct DerivationBuildingGoal : public Goal { diff --git a/src/libstore/include/nix/store/build/derivation-goal.hh b/src/libstore/include/nix/store/build/derivation-goal.hh index 9d4257cb3..ac9ec5346 100644 --- a/src/libstore/include/nix/store/build/derivation-goal.hh +++ b/src/libstore/include/nix/store/build/derivation-goal.hh @@ -22,47 +22,33 @@ void runPostBuildHook( const StorePathSet & outputPaths); /** - * A goal for building some or all of the outputs of a derivation. + * A goal for realising a single output of a derivation. Various sorts of + * fetching (which will be done by other goal types) is tried, and if none of + * those succeed, the derivation is attempted to be built. + * + * This is a purely "administrative" goal type, which doesn't do any + * "real work" of substituting (that would be `PathSubstitutionGoal` or + * `DrvOutputSubstitutionGoal`) or building (that would be a + * `DerivationBuildingGoal`). This goal type creates those types of + * goals to attempt each way of realisation a derivation; they are tried + * sequentially in order of preference. + * + * The derivation must already be gotten (in memory, in C++, parsed) and passed + * to the caller. If the derivation itself needs to be gotten first, a + * `DerivationTrampolineGoal` 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. - */ - OutputsUnmodifiedDontNeed, - /** - * `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::OutputsUnmodifiedDontNeed; - - /** - * The derivation stored at `drvReq`. + * The derivation stored at drvPath. */ std::unique_ptr drv; @@ -76,11 +62,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; @@ -88,24 +71,18 @@ struct DerivationGoal : public Goal std::string key() override; - /** - * Add wanted outputs to an already existing derivation goal. - */ - void addWantedOutputs(const OutputsSpec & outputs); - /** * 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 +90,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/derivation-trampoline-goal.hh b/src/libstore/include/nix/store/build/derivation-trampoline-goal.hh new file mode 100644 index 000000000..648337695 --- /dev/null +++ b/src/libstore/include/nix/store/build/derivation-trampoline-goal.hh @@ -0,0 +1,134 @@ +#pragma once +///@file + +#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 is the "outermost" goal type relating to derivations --- by that + * we mean that this one calls all the others for a given derivation. + * + * This is a purely "administrative" goal type, which doesn't do any "real + * work". See `DerivationGoal` for what we mean by such an administrative goal. + * + * # Rationale + * + * It exists to solve two problems: + * + * 1. We want to build a derivation we don't yet have. + * + * Traditionally, that simply means we try to substitute the missing + * derivation; simple enough. However, with (currently experimental) + * dynamic derivations, derivations themselves can be the outputs of + * other derivations. That means the general case is that a + * `DerivationTrampolineGoal` needs to create *another* + * `DerivationTrampolineGoal` goal to realize the derivation it needs. + * That goal in turn might need to create a third + * `DerivationTrampolineGoal`, the induction down to a statically known + * derivation as the base case is arbitrary deep. + * + * 2. Only a subset of outputs is needed, but such subsets are discovered + * dynamically. + * + * Consider derivations: + * + * - A has outputs x, y, and z + * + * - B needs A^x,y + * + * - C needs A^y,z and B's single output + * + * With the current `Worker` architecture, we're first discover + * needing `A^y,z` and then discover needing `A^x,y`. Of course, we + * don't want to download `A^y` twice, either. + * + * The way we handle sharing work for `A^y` is to have + * `DerivationGoal` just handle a single output, and do slightly more + * work (though it is just an "administrative" goal too), and + * `DerivationTrampolineGoal` handle sets of goals, but have it (once the + * derivation itself has been gotten) *just* create + * `DerivationGoal`s. + * + * That means it is fine to create man `DerivationTrampolineGoal` with + * overlapping sets of outputs, because all the "real work" will be + * coordinated via `DerivationGoal`s, and sharing will be discovered. + * + * Both these problems *can* be solved by having just a more powerful + * `DerivationGoal`, but that makes `DerivationGoal` more complex. + * However the more complex `DerivationGoal` has these downsides: + * + * 1. It needs to cope with only sometimes knowing a `StorePath drvPath` + * (as opposed to a more general `SingleDerivedPath drvPath` with will + * be only resolved to a `StorePath` part way through the control flow). + * + * 2. It needs complicated "restarting logic" to cope with the set of + * "wanted outputs" growing over time. + * + * (1) is not so bad, but (2) is quite scary, and has been a source of + * bugs in the past. By splitting out `DerivationTrampolineGoal`, we + * crucially avoid a need for (2), letting goal sharing rather than + * ad-hoc retry mechanisms accomplish the deduplication we need. Solving + * (1) is just a by-product and extra bonus of creating + * `DerivationTrampolineGoal`. + * + * # Misc Notes + * + * If we already have the derivation (e.g. if the evaluator has created + * the derivation locally and then instructed the store to build it), we + * can skip the derivation-getting goal entirely as a small + * optimization. + */ +struct DerivationTrampolineGoal : public Goal +{ + /** + * How to obtain a store path of the derivation to build. + */ + ref drvReq; + + /** + * The specific outputs that we need to build. + */ + OutputsSpec wantedOutputs; + + DerivationTrampolineGoal( + ref drvReq, + const OutputsSpec & wantedOutputs, + Worker & worker, + BuildMode buildMode = bmNormal); + + DerivationTrampolineGoal( + const StorePath & drvPath, + const OutputsSpec & wantedOutputs, + const Derivation & drv, + Worker & worker, + BuildMode buildMode = bmNormal); + + virtual ~DerivationTrampolineGoal(); + + void timedOut(Error && ex) override; + + std::string key() override; + + JobCategory jobCategory() const override + { + return JobCategory::Administration; + }; + +private: + + BuildMode buildMode; + + Co init(); + Co haveDerivation(StorePath drvPath, Derivation drv); + + /** + * Shared between both constructors + */ + void commonInit(); +}; + +} 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..491b8f494 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 DerivationTrampolineGoal; 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>> derivationTrampolineGoals; + 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 makeDerivationTrampolineGoal( 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 makeDerivationTrampolineGoal( + 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" @@ -232,7 +239,7 @@ public: * Make a goal corresponding to the `DerivedPath`. * * It will be a `DerivationGoal` for a `DerivedPath::Built` or - * a `SubstitutionGoal` for a `DerivedPath::Opaque`. + * a `PathSubstitutionGoal` for a `DerivedPath::Opaque`. */ GoalPtr makeGoal(const DerivedPath & req, BuildMode buildMode = bmNormal); diff --git a/src/libstore/include/nix/store/derived-path-map.hh b/src/libstore/include/nix/store/derived-path-map.hh index 16ffeb05e..6dae73fab 100644 --- a/src/libstore/include/nix/store/derived-path-map.hh +++ b/src/libstore/include/nix/store/derived-path-map.hh @@ -22,7 +22,7 @@ namespace nix { * @param V A type to instantiate for each output. It should probably * should be an "optional" type so not every interior node has to have a * value. For example, the scheduler uses - * `DerivedPathMap>` to + * `DerivedPathMap>` to * remember which goals correspond to which outputs. `* const Something` * or `std::optional` would also be good choices for * "optional" types. diff --git a/src/libstore/include/nix/store/local-store.hh b/src/libstore/include/nix/store/local-store.hh index 9a118fcc5..1aa22e311 100644 --- a/src/libstore/include/nix/store/local-store.hh +++ b/src/libstore/include/nix/store/local-store.hh @@ -403,7 +403,6 @@ private: void addBuildLog(const StorePath & drvPath, std::string_view log) override; friend struct PathSubstitutionGoal; - friend struct SubstitutionGoal; friend struct DerivationGoal; }; diff --git a/src/libstore/include/nix/store/meson.build b/src/libstore/include/nix/store/meson.build index a18430417..44d9815de 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-trampoline-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..d82bcddc1 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-trampoline-goal.cc', 'build/drv-output-substitution-goal.cc', 'build/entry-points.cc', 'build/goal.cc',