diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index a916da191..ab7b2b88c 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -1534,7 +1534,10 @@ Goal::Finished DerivationGoal::done( fs << worker.store.printStorePath(drvPath) << "\t" << buildResult.toString() << std::endl; } - return amDone(buildResult.success() ? ecSuccess : ecFailed, std::move(ex)); + return Finished{ + .result = buildResult.success() ? ecSuccess : ecFailed, + .ex = ex ? std::make_unique(std::move(*ex)) : nullptr, + }; } diff --git a/src/libstore/build/drv-output-substitution-goal.cc b/src/libstore/build/drv-output-substitution-goal.cc index 4d10543cd..62e86e1cc 100644 --- a/src/libstore/build/drv-output-substitution-goal.cc +++ b/src/libstore/build/drv-output-substitution-goal.cc @@ -26,7 +26,7 @@ Goal::WorkResult DrvOutputSubstitutionGoal::init() /* If the derivation already exists, we’re done */ if (worker.store.queryRealisation(id)) { - return amDone(ecSuccess); + return Finished{ecSuccess}; } subs = settings.useSubstitutes ? getDefaultSubstituters() : std::list>(); @@ -60,7 +60,7 @@ Goal::WorkResult DrvOutputSubstitutionGoal::tryNext() /* Hack: don't indicate failure if there were no substituters. In that case the calling derivation should just do a build. */ - return amDone(substituterFailed ? ecFailed : ecNoSubstituters); + return Finished{substituterFailed ? ecFailed : ecNoSubstituters}; } sub = subs.front(); @@ -137,7 +137,9 @@ Goal::WorkResult DrvOutputSubstitutionGoal::outPathValid() if (nrFailed > 0) { debug("The output path of the derivation output '%s' could not be substituted", id.to_string()); - return amDone(nrNoSubstituters > 0 || nrIncompleteClosure > 0 ? ecIncompleteClosure : ecFailed); + return Finished{ + nrNoSubstituters > 0 || nrIncompleteClosure > 0 ? ecIncompleteClosure : ecFailed + }; } worker.store.registerDrvOutput(*outputInfo); @@ -147,7 +149,7 @@ Goal::WorkResult DrvOutputSubstitutionGoal::outPathValid() Goal::WorkResult DrvOutputSubstitutionGoal::finished() { trace("finished"); - return amDone(ecSuccess); + return Finished{ecSuccess}; } std::string DrvOutputSubstitutionGoal::key() diff --git a/src/libstore/build/goal.cc b/src/libstore/build/goal.cc index 70ef5706e..40a3bae8d 100644 --- a/src/libstore/build/goal.cc +++ b/src/libstore/build/goal.cc @@ -45,31 +45,6 @@ void Goal::waiteeDone(GoalPtr waitee, ExitCode result) } -Goal::Finished Goal::amDone(ExitCode result, std::optional ex) -{ - trace("done"); - assert(!exitCode.has_value()); - exitCode = result; - - if (ex) { - if (!waiters.empty()) - logError(ex->info()); - else - this->ex = std::make_unique(std::move(*ex)); - } - - for (auto & i : waiters) { - GoalPtr goal = i.lock(); - if (goal) goal->waiteeDone(shared_from_this(), result); - } - waiters.clear(); - worker.removeGoal(shared_from_this()); - - cleanup(); - return Finished{}; -} - - void Goal::trace(std::string_view s) { debug("%1%: %2%", name, s); diff --git a/src/libstore/build/goal.hh b/src/libstore/build/goal.hh index db20b5cdb..adb3ab94b 100644 --- a/src/libstore/build/goal.hh +++ b/src/libstore/build/goal.hh @@ -105,10 +105,13 @@ struct Goal : public std::enable_shared_from_this public: - struct StillAlive {}; - struct Finished {}; + struct [[nodiscard]] StillAlive {}; + struct [[nodiscard]] Finished { + ExitCode result; + std::unique_ptr ex; + }; - struct WorkResult : std::variant + struct [[nodiscard]] WorkResult : std::variant { WorkResult() = delete; using variant::variant; @@ -159,8 +162,6 @@ public: virtual std::string key() = 0; - Finished amDone(ExitCode result, std::optional ex = {}); - virtual void cleanup() { } /** diff --git a/src/libstore/build/substitution-goal.cc b/src/libstore/build/substitution-goal.cc index 2c5dfea84..fb2949fd0 100644 --- a/src/libstore/build/substitution-goal.cc +++ b/src/libstore/build/substitution-goal.cc @@ -35,7 +35,7 @@ Goal::Finished PathSubstitutionGoal::done( debug(*errorMsg); buildResult.errorMsg = *errorMsg; } - return amDone(result); + return Finished{result}; } diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index 5b2e36acb..f4c352b61 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -139,6 +139,40 @@ static void removeGoal(std::shared_ptr goal, std::map> & } +void Worker::goalFinished(GoalPtr goal, Goal::Finished & f) +{ + goal->trace("done"); + assert(!goal->exitCode.has_value()); + goal->exitCode = f.result; + + if (f.ex) { + if (!goal->waiters.empty()) + logError(f.ex->info()); + else + goal->ex = std::move(f.ex); + } + + for (auto & i : goal->waiters) { + if (GoalPtr waiting = i.lock()) { + waiting->waiteeDone(goal, f.result); + } + } + goal->waiters.clear(); + removeGoal(goal); + goal->cleanup(); +} + +void Worker::handleWorkResult(GoalPtr goal, Goal::WorkResult how) +{ + std::visit( + overloaded{ + [&](Goal::StillAlive) {}, + [&](Goal::Finished & f) { goalFinished(goal, f); }, + }, + how + ); +} + void Worker::removeGoal(GoalPtr goal) { if (auto drvGoal = std::dynamic_pointer_cast(goal)) @@ -299,7 +333,7 @@ void Worker::run(const Goals & _topGoals) awake.clear(); for (auto & goal : awake2) { checkInterrupt(); - goal->work(); + handleWorkResult(goal, goal->work()); actDerivations.progress( doneBuilds, expectedBuilds + doneBuilds, runningBuilds, failedBuilds @@ -429,9 +463,14 @@ void Worker::waitForInput() j->respectTimeouts && after - j->lastOutput >= std::chrono::seconds(settings.maxSilentTime)) { - goal->timedOut(Error( + handleWorkResult( + goal, + goal->timedOut(Error( "%1% timed out after %2% seconds of silence", - goal->getName(), settings.maxSilentTime)); + goal->getName(), + settings.maxSilentTime + )) + ); continue; } @@ -440,9 +479,12 @@ void Worker::waitForInput() j->respectTimeouts && after - j->timeStarted >= std::chrono::seconds(settings.buildTimeout)) { - goal->timedOut(Error( - "%1% timed out after %2% seconds", - goal->getName(), settings.buildTimeout)); + handleWorkResult( + goal, + goal->timedOut( + Error("%1% timed out after %2% seconds", goal->getName(), settings.buildTimeout) + ) + ); continue; } @@ -469,7 +511,7 @@ void Worker::waitForInput() goal->getName(), rd); std::string_view data((char *) buffer.data(), rd); j->lastOutput = after; - goal->handleChildOutput(k, data); + handleWorkResult(goal, goal->handleChildOutput(k, data)); } } } diff --git a/src/libstore/build/worker.hh b/src/libstore/build/worker.hh index 903380c45..5af93b49e 100644 --- a/src/libstore/build/worker.hh +++ b/src/libstore/build/worker.hh @@ -105,6 +105,9 @@ private: */ std::map pathContentsGoodCache; + void goalFinished(GoalPtr goal, Goal::Finished & f); + void handleWorkResult(GoalPtr goal, Goal::WorkResult how); + public: const Activity act;