From fc6291e46dda940a69b627a3287b4633bb89f29f Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Sat, 5 Oct 2024 00:38:35 +0200 Subject: [PATCH] libstore: return goal results from Worker::run() this will be needed to move all interesting result fields out of Goal proper and into WorkResult. once that is done we can treat goals as a totally internal construct of the worker mechanism, which also allows us to fully stop exposing unclear intermediate state to Worker users. Change-Id: I98d7778a4b5b2590b7b070bdfc164a22a0ef7190 --- src/libstore/build/entry-points.cc | 8 ++++---- src/libstore/build/worker.cc | 25 ++++++++++--------------- src/libstore/build/worker.hh | 9 +++++---- 3 files changed, 19 insertions(+), 23 deletions(-) diff --git a/src/libstore/build/entry-points.cc b/src/libstore/build/entry-points.cc index 27c341295..73b0f01f3 100644 --- a/src/libstore/build/entry-points.cc +++ b/src/libstore/build/entry-points.cc @@ -25,7 +25,7 @@ void Store::buildPaths(const std::vector & reqs, BuildMode buildMod StringSet failed; std::shared_ptr ex; - for (auto & i : goals) { + for (auto & [i, result] : goals) { if (i->ex) { if (ex) logError(i->ex->info()); @@ -89,7 +89,7 @@ BuildResult Store::buildDerivation(const StorePath & drvPath, const BasicDerivat goals.emplace(gf.makeBasicDerivationGoal(drvPath, drv, OutputsSpec::All{}, buildMode)); return goals; }); - auto goal = *goals.begin(); + auto [goal, result] = *goals.begin(); return goal->buildResult.restrictTo(DerivedPath::Built { .drvPath = makeConstantStorePathRef(drvPath), .outputs = OutputsSpec::All {}, @@ -116,7 +116,7 @@ void Store::ensurePath(const StorePath & path) goals.emplace(gf.makePathSubstitutionGoal(path)); return goals; }); - auto goal = *goals.begin(); + auto [goal, result] = *goals.begin(); if (goal->exitCode != Goal::ecSuccess) { if (goal->ex) { @@ -138,7 +138,7 @@ void Store::repairPath(const StorePath & path) goals.emplace(gf.makePathSubstitutionGoal(path, Repair)); return goals; }); - auto goal = *goals.begin(); + auto [goal, result] = *goals.begin(); if (goal->exitCode != Goal::ecSuccess) { /* Since substituting the path didn't work, if we have a valid diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index ed994f4ea..0f89159e4 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -231,7 +231,7 @@ void Worker::childStarted(GoalPtr goal, kj::Promise> pr } -kj::Promise> Worker::updateStatistics() +kj::Promise> Worker::updateStatistics() try { while (true) { statisticsUpdateInhibitor = co_await statisticsUpdateSignal.acquire(); @@ -257,7 +257,7 @@ try { co_return result::failure(std::current_exception()); } -std::vector Worker::run(std::function req) +Worker::Results Worker::run(std::function req) { auto topGoals = req(goalFactory()); @@ -265,13 +265,7 @@ std::vector Worker::run(std::function req) running = true; Finally const _stop([&] { running = false; }); - std::vector results; - - for (auto & [goal, _promise] : topGoals) { - results.push_back(goal); - } - - auto onInterrupt = kj::newPromiseAndCrossThreadFulfiller>(); + auto onInterrupt = kj::newPromiseAndCrossThreadFulfiller>(); auto interruptCallback = createInterruptCallback([&] { return result::failure(std::make_exception_ptr(makeInterrupted())); }); @@ -286,12 +280,10 @@ std::vector Worker::run(std::function req) promise = promise.exclusiveJoin(boopGC(*localStore)); } - promise.wait(aio.waitScope).value(); - - return results; + return promise.wait(aio.waitScope).value(); } -kj::Promise> Worker::runImpl(Targets topGoals) +kj::Promise> Worker::runImpl(Targets topGoals) try { debug("entered goal loop"); @@ -300,10 +292,13 @@ try { promises.add(std::move(gp)); } + Results results; + auto collect = AsyncCollect(promises.releaseAsArray()); while (auto done = co_await collect.next()) { // propagate goal exceptions outward BOOST_OUTCOME_CO_TRY(auto result, done->second); + results.emplace(done->first, result); /* If a top-level goal failed, then kill all other goals (unless keepGoing was set). */ @@ -318,12 +313,12 @@ try { --keep-going *is* set, then they must all be finished now. */ assert(!settings.keepGoing || children.isEmpty()); - co_return result::success(); + co_return std::move(results); } catch (...) { co_return result::failure(std::current_exception()); } -kj::Promise> Worker::boopGC(LocalStore & localStore) +kj::Promise> Worker::boopGC(LocalStore & localStore) try { while (true) { co_await aio.provider->getTimer().afterDelay(10 * kj::SECONDS); diff --git a/src/libstore/build/worker.hh b/src/libstore/build/worker.hh index fa1031907..6569de6ee 100644 --- a/src/libstore/build/worker.hh +++ b/src/libstore/build/worker.hh @@ -86,6 +86,7 @@ class Worker : public WorkerBase { public: using Targets = std::map>>; + using Results = std::map; private: @@ -154,7 +155,7 @@ private: /** * Pass current stats counters to the logger for progress bar updates. */ - kj::Promise> updateStatistics(); + kj::Promise> updateStatistics(); AsyncSemaphore statisticsUpdateSignal{1}; std::optional statisticsUpdateInhibitor; @@ -167,8 +168,8 @@ private: statisticsUpdateInhibitor = {}; } - kj::Promise> runImpl(Targets topGoals); - kj::Promise> boopGC(LocalStore & localStore); + kj::Promise> runImpl(Targets topGoals); + kj::Promise> boopGC(LocalStore & localStore); public: @@ -265,7 +266,7 @@ public: /** * Loop until the specified top-level goals have finished. */ - std::vector run(std::function req); + Results run(std::function req); /*** * The exit status in case of failure.