From 8fb642b6e09368d10d8357fcb7c508f706fa5f08 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Tue, 24 Sep 2024 00:21:16 +0200 Subject: [PATCH] libstore: remove Goal::WaitForWorld have DerivationGoal and its subclasses produce a wrapper promise for their intermediate results instead, and return this wrapper promise. Worker already handles promises that do not complete immediately, so we do not have to duplicate this into an entire result type variant. Change-Id: Iae8dbf63cfc742afda4d415922a29ac5a3f39348 --- src/libstore/build/derivation-goal.cc | 15 ++++++++++++++- src/libstore/build/derivation-goal.hh | 2 ++ .../build/drv-output-substitution-goal.cc | 4 +--- src/libstore/build/goal.hh | 4 ---- src/libstore/build/local-derivation-goal.cc | 2 +- src/libstore/build/substitution-goal.cc | 4 +--- src/libstore/build/worker.cc | 11 ----------- 7 files changed, 19 insertions(+), 23 deletions(-) diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index b8c4d278d..037b4fb10 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -787,7 +787,7 @@ try { buildResult.startTime = time(0); // inexact state = &DerivationGoal::buildDone; started(); - return {{WaitForWorld{std::move(a.promise)}}}; + return continueOrError(std::move(a.promise)); }, [&](HookReply::Postpone) -> std::optional>> { /* Not now; wait until at least one child finishes or @@ -1756,4 +1756,17 @@ void DerivationGoal::waiteeDone(GoalPtr waitee) } } +kj::Promise> +DerivationGoal::continueOrError(kj::Promise> p) +{ + return p.then([](auto r) -> Result { + if (r.has_value()) { + return ContinueImmediately{}; + } else if (r.has_error()) { + return r.assume_error(); + } else { + return r.assume_exception(); + } + }); +} } diff --git a/src/libstore/build/derivation-goal.hh b/src/libstore/build/derivation-goal.hh index d60bb0b4c..b1311bd7e 100644 --- a/src/libstore/build/derivation-goal.hh +++ b/src/libstore/build/derivation-goal.hh @@ -327,6 +327,8 @@ protected: Finished tooMuchLogs(); void flushLine(); + static kj::Promise> continueOrError(kj::Promise> p); + public: /** * Wrappers around the corresponding Store methods that first consult the diff --git a/src/libstore/build/drv-output-substitution-goal.cc b/src/libstore/build/drv-output-substitution-goal.cc index 80b2c4cfb..923cbba58 100644 --- a/src/libstore/build/drv-output-substitution-goal.cc +++ b/src/libstore/build/drv-output-substitution-goal.cc @@ -86,9 +86,7 @@ try { }); state = &DrvOutputSubstitutionGoal::realisationFetched; - return {WaitForWorld{ - pipe.promise.then([]() -> Outcome { return result::success(); }) - }}; + return pipe.promise.then([]() -> Result { return ContinueImmediately{}; }); } catch (...) { return {std::current_exception()}; } diff --git a/src/libstore/build/goal.hh b/src/libstore/build/goal.hh index e7a500a00..7933fbc31 100644 --- a/src/libstore/build/goal.hh +++ b/src/libstore/build/goal.hh @@ -115,9 +115,6 @@ public: struct [[nodiscard]] StillAlive {}; struct [[nodiscard]] ContinueImmediately {}; - struct [[nodiscard]] WaitForWorld { - kj::Promise> promise; - }; struct [[nodiscard]] Finished { ExitCode exitCode; BuildResult result; @@ -131,7 +128,6 @@ public: struct [[nodiscard]] WorkResult : std::variant< StillAlive, ContinueImmediately, - WaitForWorld, Finished> { WorkResult() = delete; diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 2443cfb5a..a32f742f4 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -251,7 +251,7 @@ try { state = &DerivationGoal::buildDone; started(); - return {WaitForWorld{std::move(promise)}}; + return continueOrError(std::move(promise)); } catch (BuildError & e) { outputLocks.unlock(); diff --git a/src/libstore/build/substitution-goal.cc b/src/libstore/build/substitution-goal.cc index 74a63ca21..d9d8f1a7d 100644 --- a/src/libstore/build/substitution-goal.cc +++ b/src/libstore/build/substitution-goal.cc @@ -240,9 +240,7 @@ try { }); state = &PathSubstitutionGoal::finished; - return {WaitForWorld{ - pipe.promise.then([]() -> Outcome { return result::success(); }) - }}; + return pipe.promise.then([]() -> Result { return ContinueImmediately{}; }); } catch (...) { return {std::current_exception()}; } diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index 18cdde63a..1b1bf1d5c 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -202,17 +202,6 @@ void Worker::handleWorkResult(GoalPtr goal, Goal::WorkResult how) overloaded{ [&](Goal::StillAlive) {}, [&](Goal::ContinueImmediately) { wakeUp(goal); }, - [&](Goal::WaitForWorld & w) { - childStarted(goal, w.promise.then([](auto r) -> Result { - if (r.has_value()) { - return {Goal::ContinueImmediately{}}; - } else if (r.has_error()) { - return {std::move(r).error()}; - } else { - return r.exception(); - } - })); - }, [&](Goal::Finished & f) { goalFinished(goal, f); }, }, how