diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 60389fdd5..c6ac39ddf 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -118,7 +118,7 @@ void DerivationGoal::killChild() } -Goal::Finished DerivationGoal::timedOut(Error && ex) +Goal::WorkResult DerivationGoal::timedOut(Error && ex) { killChild(); return done(BuildResult::TimedOut, {}, std::move(ex)); @@ -728,7 +728,7 @@ retry: if (!actLock) actLock = std::make_unique(*logger, lvlWarn, actBuildWaiting, fmt("waiting for lock on %s", Magenta(showPaths(lockFiles)))); - (co_await waitForAWhile()).value(); + co_await waitForAWhile(); // we can loop very often, and `co_return co_await` always allocates a new frame goto retry; } @@ -799,7 +799,7 @@ retry: actLock = std::make_unique(*logger, lvlTalkative, actBuildWaiting, fmt("waiting for a machine to build '%s'", Magenta(worker.store.printStorePath(drvPath)))); outputLocks.unlock(); - (co_await waitForAWhile()).value(); + co_await waitForAWhile(); goto retry; } @@ -1331,7 +1331,7 @@ void DerivationGoal::closeLogFile() } -Goal::Finished DerivationGoal::tooMuchLogs() +Goal::WorkResult DerivationGoal::tooMuchLogs() { killChild(); return done( @@ -1380,7 +1380,7 @@ struct DerivationGoal::InputStream final : private kj::AsyncObject } }; -kj::Promise> DerivationGoal::handleBuilderOutput(InputStream & in) noexcept +kj::Promise> DerivationGoal::handleBuilderOutput(InputStream & in) noexcept try { auto buf = kj::heapArray(4096); while (true) { @@ -1413,7 +1413,7 @@ try { co_return std::current_exception(); } -kj::Promise> DerivationGoal::handleHookOutput(InputStream & in) noexcept +kj::Promise> DerivationGoal::handleHookOutput(InputStream & in) noexcept try { auto buf = kj::heapArray(4096); while (true) { @@ -1467,7 +1467,7 @@ try { co_return std::current_exception(); } -kj::Promise> DerivationGoal::handleChildOutput() noexcept +kj::Promise> DerivationGoal::handleChildOutput() noexcept try { assert(builderOutFD); @@ -1483,7 +1483,7 @@ try { handlers = handlers.exclusiveJoin( worker.aio.provider->getTimer() .afterDelay(settings.buildTimeout.get() * kj::SECONDS) - .then([this]() -> Outcome { + .then([this]() -> Outcome { return timedOut( Error("%1% timed out after %2% seconds", name, settings.buildTimeout) ); @@ -1491,7 +1491,7 @@ try { ); } - return handlers.then([this](auto r) -> Outcome { + return handlers.then([this](auto r) -> Outcome { if (!currentLogLine.empty()) flushLine(); return r; }); @@ -1499,7 +1499,7 @@ try { return {std::current_exception()}; } -kj::Promise> DerivationGoal::monitorForSilence() noexcept +kj::Promise> DerivationGoal::monitorForSilence() noexcept { while (true) { const auto stash = lastChildActivity; @@ -1513,13 +1513,13 @@ kj::Promise> DerivationGoal::monitorForSilence() n } } -kj::Promise> +kj::Promise> DerivationGoal::handleChildStreams(InputStream & builderIn, InputStream * hookIn) noexcept { lastChildActivity = worker.aio.provider->getTimer().now(); auto handlers = kj::joinPromisesFailFast([&] { - kj::Vector>> parts{2}; + kj::Vector>> parts{2}; parts.add(handleBuilderOutput(builderIn)); if (hookIn) { @@ -1680,7 +1680,7 @@ SingleDrvOutputs DerivationGoal::assertPathValidity() } -Goal::Finished DerivationGoal::done( +Goal::WorkResult DerivationGoal::done( BuildResult::Status status, SingleDrvOutputs builtOutputs, std::optional ex) @@ -1717,7 +1717,7 @@ Goal::Finished DerivationGoal::done( logError(ex->info()); } - return Finished{ + return WorkResult{ .exitCode = buildResult.success() ? ecSuccess : ecFailed, .result = buildResult, .ex = ex ? std::make_shared(std::move(*ex)) : nullptr, diff --git a/src/libstore/build/derivation-goal.hh b/src/libstore/build/derivation-goal.hh index 1e2f9b55c..0cacf75fd 100644 --- a/src/libstore/build/derivation-goal.hh +++ b/src/libstore/build/derivation-goal.hh @@ -18,7 +18,7 @@ struct HookInstance; struct HookReplyBase { struct [[nodiscard]] Accept { - kj::Promise> promise; + kj::Promise> promise; }; struct [[nodiscard]] Decline {}; struct [[nodiscard]] Postpone {}; @@ -248,7 +248,7 @@ struct DerivationGoal : public Goal BuildMode buildMode = bmNormal); virtual ~DerivationGoal() noexcept(false); - Finished timedOut(Error && ex); + WorkResult timedOut(Error && ex); kj::Promise> work() noexcept override; @@ -319,13 +319,13 @@ struct DerivationGoal : public Goal protected: kj::TimePoint lastChildActivity = kj::minValue; - kj::Promise> handleChildOutput() noexcept; - kj::Promise> + kj::Promise> handleChildOutput() noexcept; + kj::Promise> handleChildStreams(InputStream & builderIn, InputStream * hookIn) noexcept; - kj::Promise> handleBuilderOutput(InputStream & in) noexcept; - kj::Promise> handleHookOutput(InputStream & in) noexcept; - kj::Promise> monitorForSilence() noexcept; - Finished tooMuchLogs(); + kj::Promise> handleBuilderOutput(InputStream & in) noexcept; + kj::Promise> handleHookOutput(InputStream & in) noexcept; + kj::Promise> monitorForSilence() noexcept; + WorkResult tooMuchLogs(); void flushLine(); public: @@ -360,7 +360,7 @@ public: void started(); - Finished done( + WorkResult done( BuildResult::Status status, SingleDrvOutputs builtOutputs = {}, std::optional ex = {}); diff --git a/src/libstore/build/drv-output-substitution-goal.cc b/src/libstore/build/drv-output-substitution-goal.cc index 9ffa33d7b..765908aeb 100644 --- a/src/libstore/build/drv-output-substitution-goal.cc +++ b/src/libstore/build/drv-output-substitution-goal.cc @@ -30,7 +30,7 @@ try { /* If the derivation already exists, we’re done */ if (worker.store.queryRealisation(id)) { - co_return Finished{ecSuccess, std::move(buildResult)}; + co_return WorkResult{ecSuccess, std::move(buildResult)}; } subs = settings.useSubstitutes ? getDefaultSubstituters() : std::list>(); @@ -61,7 +61,7 @@ try { /* Hack: don't indicate failure if there were no substituters. In that case the calling derivation should just do a build. */ - co_return Finished{substituterFailed ? ecFailed : ecNoSubstituters, std::move(buildResult)}; + co_return WorkResult{substituterFailed ? ecFailed : ecNoSubstituters, std::move(buildResult)}; } sub = subs.front(); @@ -140,7 +140,7 @@ try { if (nrFailed > 0) { debug("The output path of the derivation output '%s' could not be substituted", id.to_string()); - return {Finished{ + return {WorkResult{ nrNoSubstituters > 0 || nrIncompleteClosure > 0 ? ecIncompleteClosure : ecFailed, std::move(buildResult), }}; @@ -155,7 +155,7 @@ try { kj::Promise> DrvOutputSubstitutionGoal::finished() noexcept try { trace("finished"); - return {Finished{ecSuccess, std::move(buildResult)}}; + return {WorkResult{ecSuccess, std::move(buildResult)}}; } catch (...) { return {std::current_exception()}; } diff --git a/src/libstore/build/goal.cc b/src/libstore/build/goal.cc index cfdb6717c..5910c37b2 100644 --- a/src/libstore/build/goal.cc +++ b/src/libstore/build/goal.cc @@ -11,18 +11,15 @@ void Goal::trace(std::string_view s) debug("%1%: %2%", name, s); } -kj::Promise> Goal::waitForAWhile() -try { +kj::Promise Goal::waitForAWhile() +{ trace("wait for a while"); /* If we are polling goals that are waiting for a lock, then wake up after a few seconds at most. */ - co_await worker.aio.provider->getTimer().afterDelay(settings.pollInterval.get() * kj::SECONDS); - co_return StillAlive{}; -} catch (...) { - co_return std::current_exception(); + return worker.aio.provider->getTimer().afterDelay(settings.pollInterval.get() * kj::SECONDS); } -kj::Promise> +kj::Promise> Goal::waitForGoals(kj::Array>> dependencies) noexcept try { auto left = dependencies.size(); @@ -45,11 +42,11 @@ try { waiteeDone(dep); if (dep->exitCode == ecFailed && !settings.keepGoing) { - co_return result::success(StillAlive{}); + co_return result::success(); } } - co_return result::success(StillAlive{}); + co_return result::success(); } catch (...) { co_return result::failure(std::current_exception()); } diff --git a/src/libstore/build/goal.hh b/src/libstore/build/goal.hh index 17c3d85db..03c2cf309 100644 --- a/src/libstore/build/goal.hh +++ b/src/libstore/build/goal.hh @@ -99,11 +99,7 @@ protected: AsyncSemaphore::Token slotToken; public: - - struct Finished; - - struct [[nodiscard]] StillAlive {}; - struct [[nodiscard]] Finished { + struct [[nodiscard]] WorkResult { ExitCode exitCode; BuildResult result; std::shared_ptr ex; @@ -113,21 +109,13 @@ public: bool checkMismatch = false; }; - struct [[nodiscard]] WorkResult : std::variant< - StillAlive, - Finished> - { - WorkResult() = delete; - using variant::variant; - }; - protected: - kj::Promise> waitForAWhile(); - kj::Promise> + kj::Promise waitForAWhile(); + kj::Promise> waitForGoals(kj::Array>> dependencies) noexcept; template... G> - kj::Promise> + kj::Promise> waitForGoals(std::pair, kj::Promise>... goals) noexcept { return waitForGoals(kj::arrOf>>(std::move(goals)...)); diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 5c1fcf1f8..c8c68f99f 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -214,7 +214,7 @@ retry: if (!actLock) actLock = std::make_unique(*logger, lvlWarn, actBuildWaiting, fmt("waiting for a free build user ID for '%s'", Magenta(worker.store.printStorePath(drvPath)))); - (co_await waitForAWhile()).value(); + co_await waitForAWhile(); // we can loop very often, and `co_return co_await` always allocates a new frame goto retry; } @@ -399,7 +399,7 @@ void LocalDerivationGoal::cleanupPostOutputsRegisteredModeNonCheck() // NOTE this one isn't noexcept because it's called from places that expect // exceptions to signal failure to launch. we should change this some time. -kj::Promise> LocalDerivationGoal::startBuilder() +kj::Promise> LocalDerivationGoal::startBuilder() { if ((buildUser && buildUser->getUIDCount() != 1) #if __linux__ diff --git a/src/libstore/build/local-derivation-goal.hh b/src/libstore/build/local-derivation-goal.hh index 44bcd2ffe..cd6ea2b55 100644 --- a/src/libstore/build/local-derivation-goal.hh +++ b/src/libstore/build/local-derivation-goal.hh @@ -218,7 +218,7 @@ struct LocalDerivationGoal : public DerivationGoal /** * Start building a derivation. */ - kj::Promise> startBuilder(); + kj::Promise> startBuilder(); /** * Fill in the environment for the builder. diff --git a/src/libstore/build/substitution-goal.cc b/src/libstore/build/substitution-goal.cc index 6dddf6e40..c0dd95da5 100644 --- a/src/libstore/build/substitution-goal.cc +++ b/src/libstore/build/substitution-goal.cc @@ -32,7 +32,7 @@ PathSubstitutionGoal::~PathSubstitutionGoal() } -Goal::Finished PathSubstitutionGoal::done( +Goal::WorkResult PathSubstitutionGoal::done( ExitCode result, BuildResult::Status status, std::optional errorMsg) @@ -42,7 +42,7 @@ Goal::Finished PathSubstitutionGoal::done( debug(*errorMsg); buildResult.errorMsg = *errorMsg; } - return Finished{result, std::move(buildResult)}; + return WorkResult{result, std::move(buildResult)}; } diff --git a/src/libstore/build/substitution-goal.hh b/src/libstore/build/substitution-goal.hh index dc701bcba..41e665779 100644 --- a/src/libstore/build/substitution-goal.hh +++ b/src/libstore/build/substitution-goal.hh @@ -72,7 +72,7 @@ struct PathSubstitutionGoal : public Goal */ std::optional ca; - Finished done( + WorkResult done( ExitCode result, BuildResult::Status status, std::optional errorMsg = {}); diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index e063ede71..0ca805b4d 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -193,7 +193,7 @@ static void removeGoal(std::shared_ptr goal, auto & goalMap) } -void Worker::goalFinished(GoalPtr goal, Goal::Finished & f) +void Worker::goalFinished(GoalPtr goal, Goal::WorkResult & f) { goal->trace("done"); assert(!goal->exitCode.has_value()); @@ -210,20 +210,6 @@ void Worker::goalFinished(GoalPtr goal, Goal::Finished & f) goal->cleanup(); } -void Worker::handleWorkResult(GoalPtr goal, Goal::WorkResult how) -{ - std::visit( - overloaded{ - [&](Goal::StillAlive) { - childStarted(goal, kj::evalLater([goal] { return goal->work(); })); - }, - [&](Goal::Finished & f) { goalFinished(goal, f); }, - }, - how - ); - updateStatistics(); -} - void Worker::removeGoal(GoalPtr goal) { if (auto drvGoal = std::dynamic_pointer_cast(goal)) @@ -250,7 +236,7 @@ void Worker::childStarted(GoalPtr goal, kj::Promise> pr children.add(promise .then([this, goal](auto result) { if (result.has_value()) { - handleWorkResult(goal, std::move(result.assume_value())); + goalFinished(goal, result.assume_value()); } else { childException = result.assume_error(); } diff --git a/src/libstore/build/worker.hh b/src/libstore/build/worker.hh index cd49fb860..d0bf742c5 100644 --- a/src/libstore/build/worker.hh +++ b/src/libstore/build/worker.hh @@ -139,8 +139,7 @@ private: */ bool checkMismatch = false; - void goalFinished(GoalPtr goal, Goal::Finished & f); - void handleWorkResult(GoalPtr goal, Goal::WorkResult how); + void goalFinished(GoalPtr goal, Goal::WorkResult & f); kj::Own> childFinished;