libstore: have goals promise WorkResults, not void

Change-Id: Idd218ec1572eda84dc47accc0dcd8a954d36f098

7 files changed, 43 insertions, 36 deletions

diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc
index edee09e13..7f72efa6a 100644
--- a/src/libstore/build/derivation-goal.cc
+++ b/src/libstore/build/derivation-goal.cc
@@ -267,7 +267,7 @@ try {
     /* We are first going to try to create the invalid output paths
        through substitutes.  If that doesn't work, we'll build
        them. */
-    kj::Vector<std::pair<GoalPtr, kj::Promise<void>>> dependencies;
+    kj::Vector<std::pair<GoalPtr, kj::Promise<Result<WorkResult>>>> dependencies;
     if (settings.useSubstitutes) {
         if (parsedDrv->substitutesAllowed()) {
             for (auto & [outputName, status] : initialOutputs) {
@@ -376,7 +376,7 @@ try {
    produced using a substitute.  So we have to build instead. */
 kj::Promise<Result<Goal::WorkResult>> DerivationGoal::gaveUpOnSubstitution() noexcept
 try {
-    kj::Vector<std::pair<GoalPtr, kj::Promise<void>>> dependencies;
+    kj::Vector<std::pair<GoalPtr, kj::Promise<Result<WorkResult>>>> dependencies;
 
     /* At this point we are building all outputs, so if more are wanted there
        is no need to restart. */
@@ -482,7 +482,7 @@ try {
         }
 
     /* Check each path (slow!). */
-    kj::Vector<std::pair<GoalPtr, kj::Promise<void>>> dependencies;
+    kj::Vector<std::pair<GoalPtr, kj::Promise<Result<WorkResult>>>> dependencies;
     for (auto & i : outputClosure) {
         if (worker.pathContentsGood(i)) continue;
         printError(
diff --git a/src/libstore/build/drv-output-substitution-goal.cc b/src/libstore/build/drv-output-substitution-goal.cc
index 0d6f3fce0..adfed5845 100644
--- a/src/libstore/build/drv-output-substitution-goal.cc
+++ b/src/libstore/build/drv-output-substitution-goal.cc
@@ -103,7 +103,7 @@ try {
         co_return co_await tryNext();
     }
 
-    kj::Vector<std::pair<GoalPtr, kj::Promise<void>>> dependencies;
+    kj::Vector<std::pair<GoalPtr, kj::Promise<Result<WorkResult>>>> dependencies;
     for (const auto & [depId, depPath] : outputInfo->dependentRealisations) {
         if (depId != id) {
             if (auto localOutputInfo = worker.store.queryRealisation(depId);
diff --git a/src/libstore/build/goal.cc b/src/libstore/build/goal.cc
index 7fbf43045..cf52280ed 100644
--- a/src/libstore/build/goal.cc
+++ b/src/libstore/build/goal.cc
@@ -29,7 +29,7 @@ try {
     exitCode = result.exitCode;
     ex = result.ex;
 
-    notify->fulfill();
+    notify->fulfill(result);
     cleanup();
 
     co_return std::move(result);
@@ -38,24 +38,25 @@ try {
 }
 
 kj::Promise<Result<void>>
-Goal::waitForGoals(kj::Array<std::pair<GoalPtr, kj::Promise<void>>> dependencies) noexcept
+Goal::waitForGoals(kj::Array<std::pair<GoalPtr, kj::Promise<Result<WorkResult>>>> dependencies) noexcept
 try {
     auto left = dependencies.size();
     for (auto & [dep, p] : dependencies) {
-        p = p.then([this, dep, &left] {
+        p = p.then([this, dep, &left](auto _result) {
             left--;
             trace(fmt("waitee '%s' done; %d left", dep->name, left));
 
             if (dep->exitCode != Goal::ecSuccess) ++nrFailed;
             if (dep->exitCode == Goal::ecNoSubstituters) ++nrNoSubstituters;
             if (dep->exitCode == Goal::ecIncompleteClosure) ++nrIncompleteClosure;
+            return _result;
         }).eagerlyEvaluate(nullptr);
     }
 
     auto collectDeps = asyncCollect(std::move(dependencies));
 
     while (auto item = co_await collectDeps.next()) {
-        auto & dep = *item;
+        auto & [dep, _result] = *item;
 
         waiteeDone(dep);
 
diff --git a/src/libstore/build/goal.hh b/src/libstore/build/goal.hh
index 10926fffc..fbcbbcffc 100644
--- a/src/libstore/build/goal.hh
+++ b/src/libstore/build/goal.hh
@@ -92,8 +92,10 @@ struct Goal
      */
     BuildResult buildResult;
 
+    struct WorkResult;
+
     // for use by Worker and Goal only. will go away once work() is a promise.
-    kj::Own<kj::PromiseFulfiller<void>> notify;
+    kj::Own<kj::PromiseFulfiller<Result<WorkResult>>> notify;
 
 protected:
     AsyncSemaphore::Token slotToken;
@@ -112,13 +114,15 @@ public:
 protected:
     kj::Promise<void> waitForAWhile();
     kj::Promise<Result<void>>
-    waitForGoals(kj::Array<std::pair<GoalPtr, kj::Promise<void>>> dependencies) noexcept;
+    waitForGoals(kj::Array<std::pair<GoalPtr, kj::Promise<Result<WorkResult>>>> dependencies) noexcept;
 
     template<std::derived_from<Goal>... G>
     kj::Promise<Result<void>>
-    waitForGoals(std::pair<std::shared_ptr<G>, kj::Promise<void>>... goals) noexcept
+    waitForGoals(std::pair<std::shared_ptr<G>, kj::Promise<Result<WorkResult>>>... goals) noexcept
     {
-        return waitForGoals(kj::arrOf<std::pair<GoalPtr, kj::Promise<void>>>(std::move(goals)...));
+        return waitForGoals(
+            kj::arrOf<std::pair<GoalPtr, kj::Promise<Result<WorkResult>>>>(std::move(goals)...)
+        );
     }
 
     virtual kj::Promise<Result<WorkResult>> workImpl() noexcept = 0;
diff --git a/src/libstore/build/substitution-goal.cc b/src/libstore/build/substitution-goal.cc
index 206bc8649..25a5eb5e4 100644
--- a/src/libstore/build/substitution-goal.cc
+++ b/src/libstore/build/substitution-goal.cc
@@ -157,7 +157,7 @@ try {
 
     /* To maintain the closure invariant, we first have to realise the
        paths referenced by this one. */
-    kj::Vector<std::pair<GoalPtr, kj::Promise<void>>> dependencies;
+    kj::Vector<std::pair<GoalPtr, kj::Promise<Result<WorkResult>>>> dependencies;
     for (auto & i : info->references)
         if (i != storePath) /* ignore self-references */
             dependencies.add(worker.goalFactory().makePathSubstitutionGoal(i));
diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc
index e90f17678..839b56bc8 100644
--- a/src/libstore/build/worker.cc
+++ b/src/libstore/build/worker.cc
@@ -53,7 +53,7 @@ Worker::~Worker()
 
 
 template<typename ID, std::derived_from<Goal> G>
-std::pair<std::shared_ptr<G>, kj::Promise<void>> Worker::makeGoalCommon(
+std::pair<std::shared_ptr<G>, kj::Promise<Result<Goal::WorkResult>>> Worker::makeGoalCommon(
     std::map<ID, CachedGoal<G>> & map,
     const ID & key,
     InvocableR<std::unique_ptr<G>> auto create,
@@ -89,7 +89,7 @@ std::pair<std::shared_ptr<G>, kj::Promise<void>> Worker::makeGoalCommon(
 }
 
 
-std::pair<std::shared_ptr<DerivationGoal>, kj::Promise<void>> Worker::makeDerivationGoal(
+std::pair<std::shared_ptr<DerivationGoal>, kj::Promise<Result<Goal::WorkResult>>> Worker::makeDerivationGoal(
     const StorePath & drvPath, const OutputsSpec & wantedOutputs, BuildMode buildMode
 )
 {
@@ -110,7 +110,7 @@ std::pair<std::shared_ptr<DerivationGoal>, kj::Promise<void>> Worker::makeDeriva
 }
 
 
-std::pair<std::shared_ptr<DerivationGoal>, kj::Promise<void>> Worker::makeBasicDerivationGoal(
+std::pair<std::shared_ptr<DerivationGoal>, kj::Promise<Result<Goal::WorkResult>>> Worker::makeBasicDerivationGoal(
     const StorePath & drvPath,
     const BasicDerivation & drv,
     const OutputsSpec & wantedOutputs,
@@ -134,7 +134,7 @@ std::pair<std::shared_ptr<DerivationGoal>, kj::Promise<void>> Worker::makeBasicD
 }
 
 
-std::pair<std::shared_ptr<PathSubstitutionGoal>, kj::Promise<void>>
+std::pair<std::shared_ptr<PathSubstitutionGoal>, kj::Promise<Result<Goal::WorkResult>>>
 Worker::makePathSubstitutionGoal(
     const StorePath & path, RepairFlag repair, std::optional<ContentAddress> ca
 )
@@ -148,7 +148,7 @@ Worker::makePathSubstitutionGoal(
 }
 
 
-std::pair<std::shared_ptr<DrvOutputSubstitutionGoal>, kj::Promise<void>>
+std::pair<std::shared_ptr<DrvOutputSubstitutionGoal>, kj::Promise<Result<Goal::WorkResult>>>
 Worker::makeDrvOutputSubstitutionGoal(
     const DrvOutput & id, RepairFlag repair, std::optional<ContentAddress> ca
 )
@@ -162,16 +162,16 @@ Worker::makeDrvOutputSubstitutionGoal(
 }
 
 
-std::pair<GoalPtr, kj::Promise<void>> Worker::makeGoal(const DerivedPath & req, BuildMode buildMode)
+std::pair<GoalPtr, kj::Promise<Result<Goal::WorkResult>>> Worker::makeGoal(const DerivedPath & req, BuildMode buildMode)
 {
     return std::visit(overloaded {
-        [&](const DerivedPath::Built & bfd) -> std::pair<GoalPtr, kj::Promise<void>> {
+        [&](const DerivedPath::Built & bfd) -> std::pair<GoalPtr, kj::Promise<Result<Goal::WorkResult>>> {
             if (auto bop = std::get_if<DerivedPath::Opaque>(&*bfd.drvPath))
                 return makeDerivationGoal(bop->path, bfd.outputs, buildMode);
             else
                 throw UnimplementedError("Building dynamic derivations in one shot is not yet implemented.");
         },
-        [&](const DerivedPath::Opaque & bo) -> std::pair<GoalPtr, kj::Promise<void>> {
+        [&](const DerivedPath::Opaque & bo) -> std::pair<GoalPtr, kj::Promise<Result<Goal::WorkResult>>> {
             return makePathSubstitutionGoal(bo.path, buildMode == bmRepair ? Repair : NoRepair);
         },
     }, req.raw());
diff --git a/src/libstore/build/worker.hh b/src/libstore/build/worker.hh
index d0bf742c5..78e204b5a 100644
--- a/src/libstore/build/worker.hh
+++ b/src/libstore/build/worker.hh
@@ -30,10 +30,12 @@ struct HookInstance;
 class GoalFactory
 {
 public:
-    virtual std::pair<std::shared_ptr<DerivationGoal>, kj::Promise<void>> makeDerivationGoal(
+    virtual std::pair<std::shared_ptr<DerivationGoal>, kj::Promise<Result<Goal::WorkResult>>>
+    makeDerivationGoal(
         const StorePath & drvPath, const OutputsSpec & wantedOutputs, BuildMode buildMode = bmNormal
     ) = 0;
-    virtual std::pair<std::shared_ptr<DerivationGoal>, kj::Promise<void>> makeBasicDerivationGoal(
+    virtual std::pair<std::shared_ptr<DerivationGoal>, kj::Promise<Result<Goal::WorkResult>>>
+    makeBasicDerivationGoal(
         const StorePath & drvPath,
         const BasicDerivation & drv,
         const OutputsSpec & wantedOutputs,
@@ -43,13 +45,13 @@ public:
     /**
      * @ref SubstitutionGoal "substitution goal"
      */
-    virtual std::pair<std::shared_ptr<PathSubstitutionGoal>, kj::Promise<void>>
+    virtual std::pair<std::shared_ptr<PathSubstitutionGoal>, kj::Promise<Result<Goal::WorkResult>>>
     makePathSubstitutionGoal(
         const StorePath & storePath,
         RepairFlag repair = NoRepair,
         std::optional<ContentAddress> ca = std::nullopt
     ) = 0;
-    virtual std::pair<std::shared_ptr<DrvOutputSubstitutionGoal>, kj::Promise<void>>
+    virtual std::pair<std::shared_ptr<DrvOutputSubstitutionGoal>, kj::Promise<Result<Goal::WorkResult>>>
     makeDrvOutputSubstitutionGoal(
         const DrvOutput & id,
         RepairFlag repair = NoRepair,
@@ -62,7 +64,7 @@ public:
      * It will be a `DerivationGoal` for a `DerivedPath::Built` or
      * a `SubstitutionGoal` for a `DerivedPath::Opaque`.
      */
-    virtual std::pair<GoalPtr, kj::Promise<void>>
+    virtual std::pair<GoalPtr, kj::Promise<Result<Goal::WorkResult>>>
     makeGoal(const DerivedPath & req, BuildMode buildMode = bmNormal) = 0;
 };
 
@@ -95,12 +97,12 @@ private:
     struct CachedGoal
     {
         std::weak_ptr<G> goal;
-        kj::Own<kj::ForkedPromise<void>> promise;
-        kj::Own<kj::PromiseFulfiller<void>> fulfiller;
+        kj::Own<kj::ForkedPromise<Result<Goal::WorkResult>>> promise;
+        kj::Own<kj::PromiseFulfiller<Result<Goal::WorkResult>>> fulfiller;
 
         CachedGoal()
         {
-            auto pf = kj::newPromiseAndFulfiller<void>();
+            auto pf = kj::newPromiseAndFulfiller<Result<Goal::WorkResult>>();
             promise = kj::heap(pf.promise.fork());
             fulfiller = std::move(pf.fulfiller);
         }
@@ -236,29 +238,29 @@ public:
      */
 private:
     template<typename ID, std::derived_from<Goal> G>
-    std::pair<std::shared_ptr<G>, kj::Promise<void>> makeGoalCommon(
+    std::pair<std::shared_ptr<G>, kj::Promise<Result<Goal::WorkResult>>> makeGoalCommon(
         std::map<ID, CachedGoal<G>> & map,
         const ID & key,
         InvocableR<std::unique_ptr<G>> auto create,
         InvocableR<bool, G &> auto modify
     );
-    std::pair<std::shared_ptr<DerivationGoal>, kj::Promise<void>> makeDerivationGoal(
+    std::pair<std::shared_ptr<DerivationGoal>, kj::Promise<Result<Goal::WorkResult>>> makeDerivationGoal(
         const StorePath & drvPath,
         const OutputsSpec & wantedOutputs, BuildMode buildMode = bmNormal) override;
-    std::pair<std::shared_ptr<DerivationGoal>, kj::Promise<void>> makeBasicDerivationGoal(
+    std::pair<std::shared_ptr<DerivationGoal>, kj::Promise<Result<Goal::WorkResult>>> makeBasicDerivationGoal(
         const StorePath & drvPath, const BasicDerivation & drv,
         const OutputsSpec & wantedOutputs, BuildMode buildMode = bmNormal) override;
 
     /**
      * @ref SubstitutionGoal "substitution goal"
      */
-    std::pair<std::shared_ptr<PathSubstitutionGoal>, kj::Promise<void>>
+    std::pair<std::shared_ptr<PathSubstitutionGoal>, kj::Promise<Result<Goal::WorkResult>>>
     makePathSubstitutionGoal(
         const StorePath & storePath,
         RepairFlag repair = NoRepair,
         std::optional<ContentAddress> ca = std::nullopt
     ) override;
-    std::pair<std::shared_ptr<DrvOutputSubstitutionGoal>, kj::Promise<void>>
+    std::pair<std::shared_ptr<DrvOutputSubstitutionGoal>, kj::Promise<Result<Goal::WorkResult>>>
     makeDrvOutputSubstitutionGoal(
         const DrvOutput & id,
         RepairFlag repair = NoRepair,
@@ -271,11 +273,11 @@ private:
      * It will be a `DerivationGoal` for a `DerivedPath::Built` or
      * a `SubstitutionGoal` for a `DerivedPath::Opaque`.
      */
-    std::pair<GoalPtr, kj::Promise<void>>
+    std::pair<GoalPtr, kj::Promise<Result<Goal::WorkResult>>>
     makeGoal(const DerivedPath & req, BuildMode buildMode = bmNormal) override;
 
 public:
-    using Targets = std::map<GoalPtr, kj::Promise<void>>;
+    using Targets = std::map<GoalPtr, kj::Promise<Result<Goal::WorkResult>>>;
 
     /**
      * Loop until the specified top-level goals have finished.