From faee771b302dc2871e3a91e3797cf1417416ce43 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Sun, 20 Oct 2024 22:55:00 +0200 Subject: [PATCH] libstore: hide Worker and goals where possible goals should be considered internal to the worker architecture due to the tight coupling of the two, and we can finally do that. doing this is also a prerequisite for turning Worker::run() into a real promise. Change-Id: I7cf273d4a6fdb75b8d192fce1af07c6265ff6980 --- src/libstore/build/entry-points.cc | 48 +++++++++++------------------- src/libstore/build/worker.cc | 6 ++-- src/libstore/build/worker.hh | 23 ++++++++++++-- 3 files changed, 40 insertions(+), 37 deletions(-) diff --git a/src/libstore/build/entry-points.cc b/src/libstore/build/entry-points.cc index 13112d827..f7669db5d 100644 --- a/src/libstore/build/entry-points.cc +++ b/src/libstore/build/entry-points.cc @@ -6,20 +6,14 @@ namespace nix { -static auto runWorker(Worker & worker, auto mkGoals) -{ - return worker.run(mkGoals); -} - void Store::buildPaths(const std::vector & reqs, BuildMode buildMode, std::shared_ptr evalStore) { auto aio = kj::setupAsyncIo(); - Worker worker(*this, evalStore ? *evalStore : *this, aio); - auto results = runWorker(worker, [&](GoalFactory & gf) { + auto results = processGoals(*this, evalStore ? *evalStore : *this, aio, [&](GoalFactory & gf) { Worker::Targets goals; for (auto & br : reqs) - goals.emplace(gf.makeGoal(br, buildMode)); + goals.emplace_back(gf.makeGoal(br, buildMode)); return goals; }); @@ -53,24 +47,19 @@ std::vector Store::buildPathsWithResults( std::shared_ptr evalStore) { auto aio = kj::setupAsyncIo(); - Worker worker(*this, evalStore ? *evalStore : *this, aio); - std::vector> state; - - auto goals = runWorker(worker, [&](GoalFactory & gf) { + auto goals = processGoals(*this, evalStore ? *evalStore : *this, aio, [&](GoalFactory & gf) { Worker::Targets goals; for (const auto & req : reqs) { - auto goal = gf.makeGoal(req, buildMode); - state.push_back({req, goal.first}); - goals.emplace(std::move(goal)); + goals.emplace_back(gf.makeGoal(req, buildMode)); } return goals; }).goals; std::vector results; - for (auto & [req, goalPtr] : state) - results.emplace_back(goals[goalPtr].result.restrictTo(req)); + for (auto && [goalIdx, req] : enumerate(reqs)) + results.emplace_back(goals[goalIdx].result.restrictTo(req)); return results; } @@ -79,15 +68,14 @@ BuildResult Store::buildDerivation(const StorePath & drvPath, const BasicDerivat BuildMode buildMode) { auto aio = kj::setupAsyncIo(); - Worker worker(*this, *this, aio); try { - auto results = runWorker(worker, [&](GoalFactory & gf) { + auto results = processGoals(*this, *this, aio, [&](GoalFactory & gf) { Worker::Targets goals; - goals.emplace(gf.makeBasicDerivationGoal(drvPath, drv, OutputsSpec::All{}, buildMode)); + goals.emplace_back(gf.makeBasicDerivationGoal(drvPath, drv, OutputsSpec::All{}, buildMode)); return goals; }); - auto [goal, result] = *results.goals.begin(); + auto & result = results.goals.begin()->second; return result.result.restrictTo(DerivedPath::Built { .drvPath = makeConstantStorePathRef(drvPath), .outputs = OutputsSpec::All {}, @@ -107,14 +95,13 @@ void Store::ensurePath(const StorePath & path) if (isValidPath(path)) return; auto aio = kj::setupAsyncIo(); - Worker worker(*this, *this, aio); - auto results = runWorker(worker, [&](GoalFactory & gf) { + auto results = processGoals(*this, *this, aio, [&](GoalFactory & gf) { Worker::Targets goals; - goals.emplace(gf.makePathSubstitutionGoal(path)); + goals.emplace_back(gf.makePathSubstitutionGoal(path)); return goals; }); - auto [goal, result] = *results.goals.begin(); + auto & result = results.goals.begin()->second; if (result.exitCode != Goal::ecSuccess) { if (result.ex) { @@ -129,23 +116,22 @@ void Store::ensurePath(const StorePath & path) void Store::repairPath(const StorePath & path) { auto aio = kj::setupAsyncIo(); - Worker worker(*this, *this, aio); - auto results = runWorker(worker, [&](GoalFactory & gf) { + auto results = processGoals(*this, *this, aio, [&](GoalFactory & gf) { Worker::Targets goals; - goals.emplace(gf.makePathSubstitutionGoal(path, Repair)); + goals.emplace_back(gf.makePathSubstitutionGoal(path, Repair)); return goals; }); - auto [goal, result] = *results.goals.begin(); + auto & result = results.goals.begin()->second; if (result.exitCode != Goal::ecSuccess) { /* Since substituting the path didn't work, if we have a valid deriver, then rebuild the deriver. */ auto info = queryPathInfo(path); if (info->deriver && isValidPath(*info->deriver)) { - worker.run([&](GoalFactory & gf) { + processGoals(*this, *this, aio, [&](GoalFactory & gf) { Worker::Targets goals; - goals.emplace(gf.makeGoal( + goals.emplace_back(gf.makeGoal( DerivedPath::Built{ .drvPath = makeConstantStorePathRef(*info->deriver), // FIXME: Should just build the specific output we need. diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index 33bee1019..1cb4a6090 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -259,9 +259,9 @@ kj::Promise> Worker::runImpl(Targets topGoals) try { debug("entered goal loop"); - kj::Vector promises(topGoals.size()); - for (auto & gp : topGoals) { - promises.add(std::move(gp)); + kj::Vector>>> promises(topGoals.size()); + for (auto && [idx, gp] : enumerate(topGoals)) { + promises.add(idx, std::move(gp.second)); } Results results; diff --git a/src/libstore/build/worker.hh b/src/libstore/build/worker.hh index e80981876..dc5e0e878 100644 --- a/src/libstore/build/worker.hh +++ b/src/libstore/build/worker.hh @@ -85,9 +85,16 @@ protected: class Worker : public WorkerBase { public: - using Targets = std::map>>; + using Targets = std::vector>>>; struct Results { - std::map goals; + /** Results of individual goals, if available. Goal results will be + * added to this map with the index they had in the `Targets` list + * returned by the goal factory function passed to `work`. If some + * goals did not complete processing, e.g. due to an early exit on + * goal failures, not all indices will be set. This may be used to + * detect which of the goals were cancelled before they completed. + */ + std::map goals; /** * The exit status in case of failure. @@ -217,6 +224,7 @@ public: NotifyingCounter expectedNarSize{[this] { updateStatisticsLater(); }}; NotifyingCounter doneNarSize{[this] { updateStatisticsLater(); }}; +private: Worker(Store & store, Store & evalStore, kj::AsyncIoContext & aio); ~Worker(); @@ -227,7 +235,6 @@ public: /** * @ref DerivationGoal "derivation goal" */ -private: template G> std::pair, kj::Promise>> makeGoalCommon( std::map> & map, @@ -280,6 +287,16 @@ public: bool pathContentsGood(const StorePath & path); void markContentsGood(const StorePath & path); + + template + friend Results + processGoals(Store & store, Store & evalStore, kj::AsyncIoContext & aio, MkGoals && mkGoals); }; +template +Worker::Results +processGoals(Store & store, Store & evalStore, kj::AsyncIoContext & aio, MkGoals && mkGoals) +{ + return Worker(store, evalStore, aio).run(std::forward(mkGoals)); +} }