From 7ef44660181b5c9743475ea73bc2e87a5f1d318f Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Sat, 5 Oct 2024 00:38:35 +0200 Subject: [PATCH] libstore: have goals promise WorkResults, not void Change-Id: Idd218ec1572eda84dc47accc0dcd8a954d36f098 --- src/libstore/build/derivation-goal.cc | 6 ++-- .../build/drv-output-substitution-goal.cc | 2 +- src/libstore/build/goal.cc | 9 +++--- src/libstore/build/goal.hh | 12 ++++--- src/libstore/build/substitution-goal.cc | 2 +- src/libstore/build/worker.cc | 16 +++++----- src/libstore/build/worker.hh | 32 ++++++++++--------- 7 files changed, 43 insertions(+), 36 deletions(-) diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index edee09e13..7f72efa6a 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -267,7 +267,7 @@ try { /* We are first going to try to create the invalid output paths through substitutes. If that doesn't work, we'll build them. */ - kj::Vector>> dependencies; + kj::Vector>>> dependencies; if (settings.useSubstitutes) { if (parsedDrv->substitutesAllowed()) { for (auto & [outputName, status] : initialOutputs) { @@ -376,7 +376,7 @@ try { produced using a substitute. So we have to build instead. */ kj::Promise> DerivationGoal::gaveUpOnSubstitution() noexcept try { - kj::Vector>> dependencies; + kj::Vector>>> dependencies; /* At this point we are building all outputs, so if more are wanted there is no need to restart. */ @@ -482,7 +482,7 @@ try { } /* Check each path (slow!). */ - kj::Vector>> dependencies; + kj::Vector>>> dependencies; for (auto & i : outputClosure) { if (worker.pathContentsGood(i)) continue; printError( diff --git a/src/libstore/build/drv-output-substitution-goal.cc b/src/libstore/build/drv-output-substitution-goal.cc index 0d6f3fce0..adfed5845 100644 --- a/src/libstore/build/drv-output-substitution-goal.cc +++ b/src/libstore/build/drv-output-substitution-goal.cc @@ -103,7 +103,7 @@ try { co_return co_await tryNext(); } - kj::Vector>> dependencies; + kj::Vector>>> dependencies; for (const auto & [depId, depPath] : outputInfo->dependentRealisations) { if (depId != id) { if (auto localOutputInfo = worker.store.queryRealisation(depId); diff --git a/src/libstore/build/goal.cc b/src/libstore/build/goal.cc index 7fbf43045..cf52280ed 100644 --- a/src/libstore/build/goal.cc +++ b/src/libstore/build/goal.cc @@ -29,7 +29,7 @@ try { exitCode = result.exitCode; ex = result.ex; - notify->fulfill(); + notify->fulfill(result); cleanup(); co_return std::move(result); @@ -38,24 +38,25 @@ try { } kj::Promise> -Goal::waitForGoals(kj::Array>> dependencies) noexcept +Goal::waitForGoals(kj::Array>>> dependencies) noexcept try { auto left = dependencies.size(); for (auto & [dep, p] : dependencies) { - p = p.then([this, dep, &left] { + p = p.then([this, dep, &left](auto _result) { left--; trace(fmt("waitee '%s' done; %d left", dep->name, left)); if (dep->exitCode != Goal::ecSuccess) ++nrFailed; if (dep->exitCode == Goal::ecNoSubstituters) ++nrNoSubstituters; if (dep->exitCode == Goal::ecIncompleteClosure) ++nrIncompleteClosure; + return _result; }).eagerlyEvaluate(nullptr); } auto collectDeps = asyncCollect(std::move(dependencies)); while (auto item = co_await collectDeps.next()) { - auto & dep = *item; + auto & [dep, _result] = *item; waiteeDone(dep); diff --git a/src/libstore/build/goal.hh b/src/libstore/build/goal.hh index 10926fffc..fbcbbcffc 100644 --- a/src/libstore/build/goal.hh +++ b/src/libstore/build/goal.hh @@ -92,8 +92,10 @@ struct Goal */ BuildResult buildResult; + struct WorkResult; + // for use by Worker and Goal only. will go away once work() is a promise. - kj::Own> notify; + kj::Own>> notify; protected: AsyncSemaphore::Token slotToken; @@ -112,13 +114,15 @@ public: protected: kj::Promise waitForAWhile(); kj::Promise> - waitForGoals(kj::Array>> dependencies) noexcept; + waitForGoals(kj::Array>>> dependencies) noexcept; template... G> kj::Promise> - waitForGoals(std::pair, kj::Promise>... goals) noexcept + waitForGoals(std::pair, kj::Promise>>... goals) noexcept { - return waitForGoals(kj::arrOf>>(std::move(goals)...)); + return waitForGoals( + kj::arrOf>>>(std::move(goals)...) + ); } virtual kj::Promise> workImpl() noexcept = 0; diff --git a/src/libstore/build/substitution-goal.cc b/src/libstore/build/substitution-goal.cc index 206bc8649..25a5eb5e4 100644 --- a/src/libstore/build/substitution-goal.cc +++ b/src/libstore/build/substitution-goal.cc @@ -157,7 +157,7 @@ try { /* To maintain the closure invariant, we first have to realise the paths referenced by this one. */ - kj::Vector>> dependencies; + kj::Vector>>> dependencies; for (auto & i : info->references) if (i != storePath) /* ignore self-references */ dependencies.add(worker.goalFactory().makePathSubstitutionGoal(i)); diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index e90f17678..839b56bc8 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -53,7 +53,7 @@ Worker::~Worker() template G> -std::pair, kj::Promise> Worker::makeGoalCommon( +std::pair, kj::Promise>> Worker::makeGoalCommon( std::map> & map, const ID & key, InvocableR> auto create, @@ -89,7 +89,7 @@ std::pair, kj::Promise> Worker::makeGoalCommon( } -std::pair, kj::Promise> Worker::makeDerivationGoal( +std::pair, kj::Promise>> Worker::makeDerivationGoal( const StorePath & drvPath, const OutputsSpec & wantedOutputs, BuildMode buildMode ) { @@ -110,7 +110,7 @@ std::pair, kj::Promise> Worker::makeDeriva } -std::pair, kj::Promise> Worker::makeBasicDerivationGoal( +std::pair, kj::Promise>> Worker::makeBasicDerivationGoal( const StorePath & drvPath, const BasicDerivation & drv, const OutputsSpec & wantedOutputs, @@ -134,7 +134,7 @@ std::pair, kj::Promise> Worker::makeBasicD } -std::pair, kj::Promise> +std::pair, kj::Promise>> Worker::makePathSubstitutionGoal( const StorePath & path, RepairFlag repair, std::optional ca ) @@ -148,7 +148,7 @@ Worker::makePathSubstitutionGoal( } -std::pair, kj::Promise> +std::pair, kj::Promise>> Worker::makeDrvOutputSubstitutionGoal( const DrvOutput & id, RepairFlag repair, std::optional ca ) @@ -162,16 +162,16 @@ Worker::makeDrvOutputSubstitutionGoal( } -std::pair> Worker::makeGoal(const DerivedPath & req, BuildMode buildMode) +std::pair>> Worker::makeGoal(const DerivedPath & req, BuildMode buildMode) { return std::visit(overloaded { - [&](const DerivedPath::Built & bfd) -> std::pair> { + [&](const DerivedPath::Built & bfd) -> std::pair>> { 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."); }, - [&](const DerivedPath::Opaque & bo) -> std::pair> { + [&](const DerivedPath::Opaque & bo) -> std::pair>> { return makePathSubstitutionGoal(bo.path, buildMode == bmRepair ? Repair : NoRepair); }, }, req.raw()); diff --git a/src/libstore/build/worker.hh b/src/libstore/build/worker.hh index d0bf742c5..78e204b5a 100644 --- a/src/libstore/build/worker.hh +++ b/src/libstore/build/worker.hh @@ -30,10 +30,12 @@ struct HookInstance; class GoalFactory { public: - virtual std::pair, kj::Promise> makeDerivationGoal( + virtual std::pair, kj::Promise>> + makeDerivationGoal( const StorePath & drvPath, const OutputsSpec & wantedOutputs, BuildMode buildMode = bmNormal ) = 0; - virtual std::pair, kj::Promise> makeBasicDerivationGoal( + virtual std::pair, kj::Promise>> + makeBasicDerivationGoal( const StorePath & drvPath, const BasicDerivation & drv, const OutputsSpec & wantedOutputs, @@ -43,13 +45,13 @@ public: /** * @ref SubstitutionGoal "substitution goal" */ - virtual std::pair, kj::Promise> + virtual std::pair, kj::Promise>> makePathSubstitutionGoal( const StorePath & storePath, RepairFlag repair = NoRepair, std::optional ca = std::nullopt ) = 0; - virtual std::pair, kj::Promise> + virtual std::pair, kj::Promise>> makeDrvOutputSubstitutionGoal( const DrvOutput & id, RepairFlag repair = NoRepair, @@ -62,7 +64,7 @@ public: * It will be a `DerivationGoal` for a `DerivedPath::Built` or * a `SubstitutionGoal` for a `DerivedPath::Opaque`. */ - virtual std::pair> + virtual std::pair>> makeGoal(const DerivedPath & req, BuildMode buildMode = bmNormal) = 0; }; @@ -95,12 +97,12 @@ private: struct CachedGoal { std::weak_ptr goal; - kj::Own> promise; - kj::Own> fulfiller; + kj::Own>> promise; + kj::Own>> fulfiller; CachedGoal() { - auto pf = kj::newPromiseAndFulfiller(); + auto pf = kj::newPromiseAndFulfiller>(); promise = kj::heap(pf.promise.fork()); fulfiller = std::move(pf.fulfiller); } @@ -236,29 +238,29 @@ public: */ private: template G> - std::pair, kj::Promise> makeGoalCommon( + std::pair, kj::Promise>> makeGoalCommon( std::map> & map, const ID & key, InvocableR> auto create, InvocableR auto modify ); - std::pair, kj::Promise> makeDerivationGoal( + std::pair, kj::Promise>> makeDerivationGoal( const StorePath & drvPath, const OutputsSpec & wantedOutputs, BuildMode buildMode = bmNormal) override; - std::pair, kj::Promise> makeBasicDerivationGoal( + std::pair, kj::Promise>> makeBasicDerivationGoal( const StorePath & drvPath, const BasicDerivation & drv, const OutputsSpec & wantedOutputs, BuildMode buildMode = bmNormal) override; /** * @ref SubstitutionGoal "substitution goal" */ - std::pair, kj::Promise> + std::pair, kj::Promise>> makePathSubstitutionGoal( const StorePath & storePath, RepairFlag repair = NoRepair, std::optional ca = std::nullopt ) override; - std::pair, kj::Promise> + std::pair, kj::Promise>> makeDrvOutputSubstitutionGoal( const DrvOutput & id, RepairFlag repair = NoRepair, @@ -271,11 +273,11 @@ private: * It will be a `DerivationGoal` for a `DerivedPath::Built` or * a `SubstitutionGoal` for a `DerivedPath::Opaque`. */ - std::pair> + std::pair>> makeGoal(const DerivedPath & req, BuildMode buildMode = bmNormal) override; public: - using Targets = std::map>; + using Targets = std::map>>; /** * Loop until the specified top-level goals have finished.