From c355ead0e5ede931313c76b704d8d4c6f9196d28 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 10 Mar 2022 19:57:46 +0000 Subject: [PATCH] Adapt scheduler to work with dynamic derivations To avoid dealing with an optional `drvPath` (because we might not know it yet) everywhere, make an `CreateDerivationAndRealiseGoal`. This goal just builds/substitutes the derivation file, and then kicks of a build for that obtained derivation; in other words it does the chaining of goals when the drv file is missing (as can already be the case) or computed (new case). This also means the `getDerivation` state can be removed from `DerivationGoal`, which makes the `BasicDerivation` / in memory case and `Derivation` / drv file file case closer together. --- .../create-derivation-and-realise-goal.cc | 158 ++++++++++++++++++ .../create-derivation-and-realise-goal.hh | 78 +++++++++ src/libstore/build/derivation-goal.cc | 32 ++-- src/libstore/build/derivation-goal.hh | 4 +- src/libstore/build/entry-points.cc | 11 +- src/libstore/build/worker.cc | 95 +++++++++-- src/libstore/build/worker.hh | 15 ++ src/libstore/derived-path.hh | 2 + tests/dyn-drv/build-built-drv.sh | 4 +- 9 files changed, 353 insertions(+), 46 deletions(-) create mode 100644 src/libstore/build/create-derivation-and-realise-goal.cc create mode 100644 src/libstore/build/create-derivation-and-realise-goal.hh diff --git a/src/libstore/build/create-derivation-and-realise-goal.cc b/src/libstore/build/create-derivation-and-realise-goal.cc new file mode 100644 index 000000000000..d001e97c105c --- /dev/null +++ b/src/libstore/build/create-derivation-and-realise-goal.cc @@ -0,0 +1,158 @@ +#include "create-derivation-and-realise-goal.hh" +#include "worker.hh" + +namespace nix { + +CreateDerivationAndRealiseGoal::CreateDerivationAndRealiseGoal(ref drvReq, + const OutputsSpec & wantedOutputs, Worker & worker, BuildMode buildMode) + : Goal(worker, DerivedPath::Built { .drvPath = drvReq, .outputs = wantedOutputs }) + , drvReq(drvReq) + , wantedOutputs(wantedOutputs) + , buildMode(buildMode) +{ + state = &CreateDerivationAndRealiseGoal::getDerivation; + name = fmt( + "outer obtaining drv from '%s' and then building outputs %s", + drvReq->to_string(worker.store), + std::visit(overloaded { + [&](const OutputsSpec::All) -> std::string { + return "* (all of them)"; + }, + [&](const OutputsSpec::Names os) { + return concatStringsSep(", ", quoteStrings(os)); + }, + }, wantedOutputs.raw())); + trace("created outer"); + + worker.updateProgress(); +} + + +CreateDerivationAndRealiseGoal::~CreateDerivationAndRealiseGoal() +{ +} + + +static StorePath pathPartOfReq(const SingleDerivedPath & req) +{ + return std::visit(overloaded { + [&](const SingleDerivedPath::Opaque & bo) { + return bo.path; + }, + [&](const SingleDerivedPath::Built & bfd) { + return pathPartOfReq(*bfd.drvPath); + }, + }, req.raw()); +} + + +std::string CreateDerivationAndRealiseGoal::key() +{ + /* Ensure that derivations get built in order of their name, + i.e. a derivation named "aardvark" always comes before "baboon". And + substitution goals and inner derivation goals always happen before + derivation goals (due to "b$"). */ + return "c$" + std::string(pathPartOfReq(*drvReq).name()) + "$" + drvReq->to_string(worker.store); +} + + +void CreateDerivationAndRealiseGoal::timedOut(Error && ex) +{ +} + + +void CreateDerivationAndRealiseGoal::work() +{ + (this->*state)(); +} + + +void CreateDerivationAndRealiseGoal::addWantedOutputs(const OutputsSpec & outputs) +{ + /* If we already want all outputs, there is nothing to do. */ + auto newWanted = wantedOutputs.union_(outputs); + bool needRestart = !newWanted.isSubsetOf(wantedOutputs); + wantedOutputs = newWanted; + + if (!needRestart) return; + + if (!optDrvPath) + // haven't started steps where the outputs matter yet + return; + worker.makeDerivationGoal(*optDrvPath, outputs, buildMode); +} + + +void CreateDerivationAndRealiseGoal::getDerivation() +{ + trace("outer init"); + + /* The first thing to do is to make sure that the derivation + exists. If it doesn't, it may be created through a + substitute. */ + { + if (buildMode != bmNormal) goto load; + + auto drvPath = StorePath::dummy; + try { + drvPath = resolveDerivedPath(worker.store, *drvReq); + } catch (MissingRealisation) { + goto load; + } + if (!worker.evalStore.isValidPath(drvPath) && !worker.store.isValidPath(drvPath)) + goto load; + + trace(fmt("already have drv '%s' for '%s', can go straight to building", + worker.store.printStorePath(drvPath), + drvReq->to_string(worker.store))); + + loadAndBuildDerivation(); + return; + } + +load: + trace("need to obtain drv we want to build"); + + addWaitee(worker.makeGoal(DerivedPath::fromSingle(*drvReq))); + + state = &CreateDerivationAndRealiseGoal::loadAndBuildDerivation; + if (waitees.empty()) work(); +} + + +void CreateDerivationAndRealiseGoal::loadAndBuildDerivation() +{ + trace("outer load and build derivation"); + + if (nrFailed != 0) { + amDone(ecFailed, Error("cannot build missing derivation '%s'", drvReq->to_string(worker.store))); + return; + } + + StorePath drvPath = resolveDerivedPath(worker.store, *drvReq); + /* Build this step! */ + concreteDrvGoal = worker.makeDerivationGoal(drvPath, wantedOutputs, buildMode); + addWaitee(upcast_goal(concreteDrvGoal)); + state = &CreateDerivationAndRealiseGoal::buildDone; + optDrvPath = std::move(drvPath); + if (waitees.empty()) work(); +} + + +void CreateDerivationAndRealiseGoal::buildDone() +{ + trace("outer build done"); + + buildResult = upcast_goal(concreteDrvGoal)->getBuildResult(DerivedPath::Built { + .drvPath = drvReq, + .outputs = wantedOutputs, + }); + + if (buildResult.success()) + amDone(ecSuccess); + else + amDone(ecFailed, Error("building '%s' failed", drvReq->to_string(worker.store))); +} + + +} diff --git a/src/libstore/build/create-derivation-and-realise-goal.hh b/src/libstore/build/create-derivation-and-realise-goal.hh new file mode 100644 index 000000000000..9d58e6611a10 --- /dev/null +++ b/src/libstore/build/create-derivation-and-realise-goal.hh @@ -0,0 +1,78 @@ +#pragma once + +#include "parsed-derivations.hh" +#include "lock.hh" +#include "store-api.hh" +#include "pathlocks.hh" +#include "goal.hh" + +namespace nix { + +struct DerivationGoal; + +struct CreateDerivationAndRealiseGoal : 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 goal for the corresponding concrete derivation. + **/ + std::shared_ptr concreteDrvGoal; + + /** + * The specific outputs that we need to build. + */ + OutputsSpec wantedOutputs; + + typedef void (CreateDerivationAndRealiseGoal::*GoalState)(); + GoalState state; + + /** + * The final output paths of the build. + * + * - For input-addressed derivations, always the precomputed paths + * + * - For content-addressed derivations, calcuated from whatever the + * hash ends up being. (Note that fixed outputs derivations that + * produce the "wrong" output still install that data under its + * true content-address.) + */ + OutputPathMap finalOutputs; + + BuildMode buildMode; + + CreateDerivationAndRealiseGoal(ref drvReq, + const OutputsSpec & wantedOutputs, Worker & worker, + BuildMode buildMode = bmNormal); + virtual ~CreateDerivationAndRealiseGoal(); + + void timedOut(Error && ex) override; + + std::string key() override; + + void work() override; + + /** + * Add wanted outputs to an already existing derivation goal. + */ + void addWantedOutputs(const OutputsSpec & outputs); + + /** + * The states. + */ + void getDerivation(); + void loadAndBuildDerivation(); + void buildDone(); + + JobCategory jobCategory() override { return JobCategory::Build; }; +}; + +} diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 405291470490..5e74f27101d7 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -71,7 +71,7 @@ DerivationGoal::DerivationGoal(const StorePath & drvPath, , wantedOutputs(wantedOutputs) , buildMode(buildMode) { - state = &DerivationGoal::getDerivation; + state = &DerivationGoal::loadDerivation; name = fmt( "building of '%s' from .drv file", DerivedPath::Built { makeConstantStorePathRef(drvPath), wantedOutputs }.to_string(worker.store)); @@ -164,24 +164,6 @@ void DerivationGoal::addWantedOutputs(const OutputsSpec & outputs) } -void DerivationGoal::getDerivation() -{ - trace("init"); - - /* The first thing to do is to make sure that the derivation - exists. If it doesn't, it may be created through a - substitute. */ - if (buildMode == bmNormal && worker.evalStore.isValidPath(drvPath)) { - loadDerivation(); - return; - } - - addWaitee(upcast_goal(worker.makePathSubstitutionGoal(drvPath))); - - state = &DerivationGoal::loadDerivation; -} - - void DerivationGoal::loadDerivation() { trace("loading derivation"); @@ -1484,6 +1466,11 @@ void DerivationGoal::done( amDone(buildResult.success() ? ecSuccess : ecFailed, std::move(ex)); } +} + +#include "create-derivation-and-realise-goal.hh" + +namespace nix { void DerivationGoal::waiteeDone(GoalPtr waitee, ExitCode result) { @@ -1492,8 +1479,11 @@ void DerivationGoal::waiteeDone(GoalPtr waitee, ExitCode result) if (!useDerivation) return; auto & fullDrv = *dynamic_cast(drv.get()); - auto * dg = dynamic_cast(&*waitee); - if (!dg) return; + auto * odg = dynamic_cast(&*waitee); + if (!odg) return; + auto * dg = &*odg->concreteDrvGoal; + // By the time we get here, the outer goals should have. + assert(dg); auto outputs = fullDrv.inputDrvs.find(dg->drvPath); if (outputs == fullDrv.inputDrvs.end()) return; diff --git a/src/libstore/build/derivation-goal.hh b/src/libstore/build/derivation-goal.hh index ee8f06f25508..3a48ae685e60 100644 --- a/src/libstore/build/derivation-goal.hh +++ b/src/libstore/build/derivation-goal.hh @@ -66,8 +66,7 @@ struct DerivationGoal : public Goal std::shared_ptr resolvedDrvGoal; /** - * The specific outputs that we need to build. Empty means all of - * them. + * The specific outputs that we need to build. */ OutputsSpec wantedOutputs; @@ -229,7 +228,6 @@ struct DerivationGoal : public Goal /** * The states. */ - void getDerivation(); void loadDerivation(); void haveDerivation(); void outputsSubstitutionTried(); diff --git a/src/libstore/build/entry-points.cc b/src/libstore/build/entry-points.cc index f71fb35a611c..f0f0e5519358 100644 --- a/src/libstore/build/entry-points.cc +++ b/src/libstore/build/entry-points.cc @@ -1,5 +1,6 @@ #include "worker.hh" #include "substitution-goal.hh" +#include "create-derivation-and-realise-goal.hh" #include "derivation-goal.hh" #include "local-store.hh" @@ -15,7 +16,7 @@ void Store::buildPaths(const std::vector & reqs, BuildMode buildMod worker.run(goals); - StorePathSet failed; + StringSet failed; std::optional ex; for (auto & i : goals) { if (i->ex) { @@ -25,8 +26,10 @@ void Store::buildPaths(const std::vector & reqs, BuildMode buildMod ex = std::move(i->ex); } if (i->exitCode != Goal::ecSuccess) { - if (auto i2 = dynamic_cast(i.get())) failed.insert(i2->drvPath); - else if (auto i2 = dynamic_cast(i.get())) failed.insert(i2->storePath); + if (auto i2 = dynamic_cast(i.get())) + failed.insert(i2->drvReq->to_string(*this)); + else if (auto i2 = dynamic_cast(i.get())) + failed.insert(printStorePath(i2->storePath)); } } @@ -35,7 +38,7 @@ void Store::buildPaths(const std::vector & reqs, BuildMode buildMod throw std::move(*ex); } else if (!failed.empty()) { if (ex) logError(ex->info()); - throw Error(worker.failingExitStatus(), "build of %s failed", showPaths(failed)); + throw Error(worker.failingExitStatus(), "build of %s failed", concatStringsSep(", ", quoteStrings(failed))); } } diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index 6779dbcf3870..9723715ffac4 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -2,6 +2,7 @@ #include "worker.hh" #include "substitution-goal.hh" #include "drv-output-substitution-goal.hh" +#include "create-derivation-and-realise-goal.hh" #include "local-derivation-goal.hh" #include "hook-instance.hh" @@ -41,6 +42,39 @@ Worker::~Worker() } +std::shared_ptr Worker::makeCreateDerivationAndRealiseGoal( + ref drvReq, + const OutputsSpec & wantedOutputs, + BuildMode buildMode) +{ + std::function findSlot; + + findSlot = [&](const SingleDerivedPath & drvReq) -> auto & + { + return std::visit(overloaded { + [&](const SingleDerivedPath::Opaque & bo) -> auto & { + return outerDerivationGoals[bo.path]; + }, + [&](const SingleDerivedPath::Built & bfd) -> auto & { + auto & mapNode = findSlot(*bfd.drvPath); + return mapNode.childMap[bfd.output]; + }, + }, drvReq.raw()); + }; + + std::weak_ptr & goal_weak = findSlot(*drvReq).goal; // = outerDerivationGoals[drvReq]; + std::shared_ptr goal = goal_weak.lock(); + if (!goal) { + goal = std::make_shared(drvReq, wantedOutputs, *this, buildMode); + goal_weak = goal; + wakeUp(goal); + } else { + goal->addWantedOutputs(wantedOutputs); + } + return goal; +} + + std::shared_ptr Worker::makeDerivationGoalCommon( const StorePath & drvPath, const OutputsSpec & wantedOutputs, @@ -111,10 +145,7 @@ GoalPtr Worker::makeGoal(const DerivedPath & req, BuildMode buildMode) { return std::visit(overloaded { [&](const DerivedPath::Built & bfd) -> GoalPtr { - if (auto bop = std::get_if(&*bfd.drvPath)) - return makeDerivationGoal(bop->path, bfd.outputs, buildMode); - else - throw UnimplementedError("Building dynamic derivations in one shot is not yet implemented."); + return makeCreateDerivationAndRealiseGoal(bfd.drvPath, bfd.outputs, buildMode); }, [&](const DerivedPath::Opaque & bo) -> GoalPtr { return makePathSubstitutionGoal(bo.path, buildMode == bmRepair ? Repair : NoRepair); @@ -123,24 +154,46 @@ GoalPtr Worker::makeGoal(const DerivedPath & req, BuildMode buildMode) } +template +static void filterMap(std::map & goalMap, F f) +{ + for (auto i = goalMap.begin(); i != goalMap.end();) + if (!f(i->second)) + i = goalMap.erase(i); + else ++i; +} + + template static void removeGoal(std::shared_ptr goal, std::map> & goalMap) { /* !!! inefficient */ - for (auto i = goalMap.begin(); - i != goalMap.end(); ) - if (i->second.lock() == goal) { - auto j = i; ++j; - goalMap.erase(i); - i = j; - } - else ++i; + filterMap(goalMap, [&](const std::weak_ptr & gp) -> bool { + return gp.lock() != goal; + }); +} + +template +static void removeGoal(std::shared_ptr goal, std::map & goalMap); + +template +static void removeGoal(std::shared_ptr goal, std::map & goalMap) +{ + /* !!! inefficient */ + filterMap(goalMap, [&](CreateDerivationAndRealiseGoalsMapNode & node) -> bool { + if (node.goal.lock() == goal) + node.goal.reset(); + removeGoal(goal, node.childMap); + return !node.goal.expired() || !node.childMap.empty(); + }); } void Worker::removeGoal(GoalPtr goal) { - if (auto drvGoal = std::dynamic_pointer_cast(goal)) + if (auto drvGoal = std::dynamic_pointer_cast(goal)) + nix::removeGoal(drvGoal, outerDerivationGoals); + else if (auto drvGoal = std::dynamic_pointer_cast(goal)) nix::removeGoal(drvGoal, derivationGoals); else if (auto subGoal = std::dynamic_pointer_cast(goal)) nix::removeGoal(subGoal, substitutionGoals); @@ -267,8 +320,8 @@ void Worker::run(const Goals & _topGoals) for (auto & i : _topGoals) { topGoals.insert(i); - if (auto goal = dynamic_cast(i.get())) { - topPaths.push_back(DerivedPath::Built{makeConstantStorePathRef(goal->drvPath), goal->wantedOutputs}); + if (auto goal = dynamic_cast(i.get())) { + topPaths.push_back(DerivedPath::Built{goal->drvReq, goal->wantedOutputs}); } else if (auto goal = dynamic_cast(i.get())) { topPaths.push_back(DerivedPath::Opaque{goal->storePath}); } @@ -519,10 +572,18 @@ void Worker::markContentsGood(const StorePath & path) } -GoalPtr upcast_goal(std::shared_ptr subGoal) { +GoalPtr upcast_goal(std::shared_ptr subGoal) +{ + return subGoal; +} + +GoalPtr upcast_goal(std::shared_ptr subGoal) +{ return subGoal; } -GoalPtr upcast_goal(std::shared_ptr subGoal) { + +GoalPtr upcast_goal(std::shared_ptr subGoal) +{ return subGoal; } diff --git a/src/libstore/build/worker.hh b/src/libstore/build/worker.hh index 5abceca0d71a..bd4676f59d0c 100644 --- a/src/libstore/build/worker.hh +++ b/src/libstore/build/worker.hh @@ -13,6 +13,7 @@ namespace nix { /* Forward definition. */ +struct CreateDerivationAndRealiseGoal; struct DerivationGoal; struct PathSubstitutionGoal; class DrvOutputSubstitutionGoal; @@ -31,6 +32,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; @@ -57,6 +59,13 @@ struct Child /* Forward definition. */ struct HookInstance; +struct CreateDerivationAndRealiseGoalsMapNode; +typedef std::map CreateDerivationAndRealiseGoalsChildMap; +struct CreateDerivationAndRealiseGoalsMapNode { + std::weak_ptr goal; + CreateDerivationAndRealiseGoalsChildMap childMap; +}; + /** * The worker class. */ @@ -102,6 +111,9 @@ private: * Maps used to prevent multiple instantiations of a goal for the * same derivation / path. */ + + std::map outerDerivationGoals; + std::map> derivationGoals; std::map> substitutionGoals; std::map> drvOutputSubstitutionGoals; @@ -189,6 +201,9 @@ public: * @ref DerivationGoal "derivation goal" */ private: + std::shared_ptr makeCreateDerivationAndRealiseGoal( + ref drvPath, + const OutputsSpec & wantedOutputs, BuildMode buildMode = bmNormal); std::shared_ptr makeDerivationGoalCommon( const StorePath & drvPath, const OutputsSpec & wantedOutputs, std::function()> mkDrvGoal); diff --git a/src/libstore/derived-path.hh b/src/libstore/derived-path.hh index ec30dac611d5..959d4bcd067f 100644 --- a/src/libstore/derived-path.hh +++ b/src/libstore/derived-path.hh @@ -83,6 +83,8 @@ using _SingleDerivedPathRaw = std::variant< SingleDerivedPathBuilt >; +struct DerivedPath; + /** * A "derived path" is a very simple sort of expression (not a Nix * language expression! But an expression in a the general sense) that diff --git a/tests/dyn-drv/build-built-drv.sh b/tests/dyn-drv/build-built-drv.sh index 647be9457167..94f3550bdc35 100644 --- a/tests/dyn-drv/build-built-drv.sh +++ b/tests/dyn-drv/build-built-drv.sh @@ -18,4 +18,6 @@ clearStore drvDep=$(nix-instantiate ./text-hashed-output.nix -A producingDrv) -expectStderr 1 nix build "${drvDep}^out^out" --no-link | grepQuiet "Building dynamic derivations in one shot is not yet implemented" +out2=$(nix build "${drvDep}^out^out" --no-link) + +test $out1 == $out2