From 55dbb7f1ccc0e991df6b09af0a16ce9246ac3f19 Mon Sep 17 00:00:00 2001
From: regnat <rg@regnat.ovh>
Date: Mon, 13 Dec 2021 16:56:44 +0100
Subject: [PATCH] More properly track the status of CA builds
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Make the build of unresolved derivations return the same status as the
resolved one, except in the case of an `AlreadyValid` in which case it
will return `ResolvesToAlreadyValid` to mean that the outputs of the unresolved
derivation weren’t known, but the resolved one is.
---
 src/libstore/build/derivation-goal.cc | 26 +++++++++++++++++---------
 src/libstore/build/derivation-goal.hh |  4 ++--
 src/libstore/store-api.hh             |  3 ++-
 3 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc
index 60945403e..a713d7222 100644
--- a/src/libstore/build/derivation-goal.cc
+++ b/src/libstore/build/derivation-goal.cc
@@ -464,7 +464,6 @@ void DerivationGoal::inputsRealised()
             Derivation drvResolved { *std::move(attempt) };
 
             auto pathResolved = writeDerivation(worker.store, drvResolved);
-            resolvedDrv = drvResolved;
 
             auto msg = fmt("Resolved derivation: '%s' -> '%s'",
                 worker.store.printStorePath(drvPath),
@@ -475,9 +474,9 @@ void DerivationGoal::inputsRealised()
                        worker.store.printStorePath(pathResolved),
                    });
 
-            auto resolvedGoal = worker.makeDerivationGoal(
+            resolvedDrvGoal = worker.makeDerivationGoal(
                 pathResolved, wantedOutputs, buildMode);
-            addWaitee(resolvedGoal);
+            addWaitee(resolvedDrvGoal);
 
             state = &DerivationGoal::resolvedFinished;
             return;
@@ -938,16 +937,17 @@ void DerivationGoal::buildDone()
 }
 
 void DerivationGoal::resolvedFinished() {
-    assert(resolvedDrv);
+    assert(resolvedDrvGoal);
+    auto resolvedDrv = *resolvedDrvGoal->drv;
 
-    auto resolvedHashes = staticOutputHashes(worker.store, *resolvedDrv);
+    auto resolvedHashes = staticOutputHashes(worker.store, resolvedDrv);
 
     StorePathSet outputPaths;
 
     // `wantedOutputs` might be empty, which means “all the outputs”
     auto realWantedOutputs = wantedOutputs;
     if (realWantedOutputs.empty())
-        realWantedOutputs = resolvedDrv->outputNames();
+        realWantedOutputs = resolvedDrv.outputNames();
 
     for (auto & wantedOutput : realWantedOutputs) {
         assert(initialOutputs.count(wantedOutput) != 0);
@@ -979,9 +979,17 @@ void DerivationGoal::resolvedFinished() {
         outputPaths
     );
 
-    // This is potentially a bit fishy in terms of error reporting. Not sure
-    // how to do it in a cleaner way
-    amDone(nrFailed == 0 ? ecSuccess : ecFailed, ex);
+    auto status = [&]() {
+        auto resolvedResult = resolvedDrvGoal->getResult();
+        switch (resolvedResult.status) {
+            case BuildResult::AlreadyValid:
+                return BuildResult::ResolvesToAlreadyValid;
+            default:
+                return resolvedResult.status;
+        }
+    }();
+
+    done(status);
 }
 
 HookReply DerivationGoal::tryBuildHook()
diff --git a/src/libstore/build/derivation-goal.hh b/src/libstore/build/derivation-goal.hh
index 704b77caf..e112542c7 100644
--- a/src/libstore/build/derivation-goal.hh
+++ b/src/libstore/build/derivation-goal.hh
@@ -50,8 +50,8 @@ struct DerivationGoal : public Goal
     /* The path of the derivation. */
     StorePath drvPath;
 
-    /* The path of the corresponding resolved derivation */
-    std::optional<BasicDerivation> resolvedDrv;
+    /* The goal for the corresponding resolved derivation */
+    std::shared_ptr<DerivationGoal> resolvedDrvGoal;
 
     /* The specific outputs that we need to build.  Empty means all of
        them. */
diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh
index aa44651d4..3a385b293 100644
--- a/src/libstore/store-api.hh
+++ b/src/libstore/store-api.hh
@@ -151,6 +151,7 @@ struct BuildResult
         DependencyFailed,
         LogLimitExceeded,
         NotDeterministic,
+        ResolvesToAlreadyValid,
     } status = MiscFailure;
     std::string errorMsg;
 
@@ -170,7 +171,7 @@ struct BuildResult
     time_t startTime = 0, stopTime = 0;
 
     bool success() {
-        return status == Built || status == Substituted || status == AlreadyValid;
+        return status == Built || status == Substituted || status == AlreadyValid || status == ResolvesToAlreadyValid;
     }
 };