From 40f154c0edc53e160b70fc60b2b5b8652dfbe84b Mon Sep 17 00:00:00 2001
From: eldritch horrors <pennae@lix.systems>
Date: Sat, 5 Oct 2024 00:38:35 +0200
Subject: [PATCH] libstore: remove Worker::topGoals

since we now propagate goal exceptions properly we no longer need to
check topGoals for a reason to abort early. any early abort reasons,
whether by exception or a clean top goal failure, can now be handled
by inspecting the goal result in the main loop. this greatly reduces
goal-to-goal interactions that do not happen at the main loop level.

since the underscore-free name is now available for use as variables
we'll migrate to that where we currently use `_topGoals` for locals.

Change-Id: I5727c5ea7799647c0a69ab76975b1a03a6558aa6
---
 src/libstore/build/worker.cc | 32 +++++++++++++-------------------
 src/libstore/build/worker.hh |  7 +------
 2 files changed, 14 insertions(+), 25 deletions(-)

diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc
index 5ca7cde76..ed994f4ea 100644
--- a/src/libstore/build/worker.cc
+++ b/src/libstore/build/worker.cc
@@ -46,7 +46,6 @@ Worker::~Worker()
        goals that refer to this worker should be gone.  (Otherwise we
        are in trouble, since goals may call childTerminated() etc. in
        their destructors). */
-    topGoals.clear();
     children.clear();
 
     assert(expectedSubstitutions == 0);
@@ -216,14 +215,6 @@ void Worker::removeGoal(GoalPtr goal)
         nix::removeGoal(subGoal, drvOutputSubstitutionGoals);
     else
         assert(false);
-
-    if (topGoals.find(goal) != topGoals.end()) {
-        topGoals.erase(goal);
-        /* If a top-level goal failed, then kill all other goals
-           (unless keepGoing was set). */
-        if (goal->exitCode == Goal::ecFailed && !settings.keepGoing)
-            topGoals.clear();
-    }
 }
 
 
@@ -268,7 +259,7 @@ try {
 
 std::vector<GoalPtr> Worker::run(std::function<Targets (GoalFactory &)> req)
 {
-    auto _topGoals = req(goalFactory());
+    auto topGoals = req(goalFactory());
 
     assert(!running);
     running = true;
@@ -276,9 +267,7 @@ std::vector<GoalPtr> Worker::run(std::function<Targets (GoalFactory &)> req)
 
     std::vector<GoalPtr> results;
 
-    topGoals.clear();
-    for (auto & [goal, _promise] : _topGoals) {
-        topGoals.insert(goal);
+    for (auto & [goal, _promise] : topGoals) {
         results.push_back(goal);
     }
 
@@ -287,7 +276,7 @@ std::vector<GoalPtr> Worker::run(std::function<Targets (GoalFactory &)> req)
         return result::failure(std::make_exception_ptr(makeInterrupted()));
     });
 
-    auto promise = runImpl(std::move(_topGoals))
+    auto promise = runImpl(std::move(topGoals))
                        .exclusiveJoin(updateStatistics())
                        .exclusiveJoin(std::move(onInterrupt.promise));
 
@@ -302,21 +291,26 @@ std::vector<GoalPtr> Worker::run(std::function<Targets (GoalFactory &)> req)
     return results;
 }
 
-kj::Promise<Result<void>> Worker::runImpl(Targets _topGoals)
+kj::Promise<Result<void>> Worker::runImpl(Targets topGoals)
 try {
     debug("entered goal loop");
 
-    kj::Vector<Targets::value_type> promises(_topGoals.size());
-    for (auto & gp : _topGoals) {
+    kj::Vector<Targets::value_type> promises(topGoals.size());
+    for (auto & gp : topGoals) {
         promises.add(std::move(gp));
     }
 
     auto collect = AsyncCollect(promises.releaseAsArray());
     while (auto done = co_await collect.next()) {
         // propagate goal exceptions outward
-        BOOST_OUTCOME_CO_TRYV(done->second);
+        BOOST_OUTCOME_CO_TRY(auto result, done->second);
 
-        if (topGoals.empty()) break;
+        /* If a top-level goal failed, then kill all other goals
+           (unless keepGoing was set). */
+        if (result.exitCode == Goal::ecFailed && !settings.keepGoing) {
+            children.clear();
+            break;
+        }
     }
 
     /* If --keep-going is not set, it's possible that the main goal
diff --git a/src/libstore/build/worker.hh b/src/libstore/build/worker.hh
index 26832c3b1..fa1031907 100644
--- a/src/libstore/build/worker.hh
+++ b/src/libstore/build/worker.hh
@@ -91,11 +91,6 @@ private:
 
     bool running = false;
 
-    /**
-     * The top-level goals of the worker.
-     */
-    Goals topGoals;
-
     template<typename G>
     struct CachedGoal
     {
@@ -172,7 +167,7 @@ private:
         statisticsUpdateInhibitor = {};
     }
 
-    kj::Promise<Result<void>> runImpl(Targets _topGoals);
+    kj::Promise<Result<void>> runImpl(Targets topGoals);
     kj::Promise<Result<void>> boopGC(LocalStore & localStore);
 
 public: