Inline buildDone from DerivationGoal into use sites

The basic idea is that while we have duplicated this function, we now
have one call-site in the local build case, and one call site in the
build hook case. This unlocks big opportunities to specialize each copy,
since they really shouldn't be doing the same things. By the time we are
are done, there should not be much duplication left.

See #12628 for further info.

src/libstore/build/derivation-goal.cc

@@ -640,7 +640,7 @@ Goal::Co DerivationGoal::tryToBuild()
                 buildResult.startTime = time(0); // inexact
                 started();
                 co_await Suspend{};
-                co_return buildDone();
+                co_return hookDone();
             case rpPostpone:
                 /* Not now; wait until at least one child finishes or
                    the wake-up timeout expires. */
@@ -935,7 +935,7 @@ void appendLogTailErrorMsg(
 }
 
 
-Goal::Co DerivationGoal::buildDone()
+Goal::Co DerivationGoal::hookDone()
 {
     trace("build done");
 

src/libstore/build/derivation-goal.hh

@@ -55,6 +55,13 @@ struct InitialOutput {
     std::optional<InitialOutputStatus> known;
 };
 
+/** Used internally */
+void runPostBuildHook(
+    Store & store,
+    Logger & logger,
+    const StorePath & drvPath,
+    const StorePathSet & outputPaths);
+
 /** Used internally */
 void appendLogTailErrorMsg(
     Worker & worker,
@@ -246,7 +253,7 @@ struct DerivationGoal : public Goal
     Co gaveUpOnSubstitution();
     Co tryToBuild();
     virtual Co tryLocalBuild();
-    Co buildDone();
+    Co hookDone();
 
     Co resolvedFinished();
 

src/libstore/unix/build/local-derivation-goal.cc

@@ -263,8 +263,114 @@ Goal::Co LocalDerivationGoal::tryLocalBuild()
 
     started();
     co_await Suspend{};
-    // after EOF on child
+
-    co_return buildDone();
+    trace("build done");
+
+    Finally releaseBuildUser([&](){ this->cleanupHookFinally(); });
+
+    cleanupPreChildKill();
+
+    /* Since we got an EOF on the logger pipe, the builder is presumed
+       to have terminated.  In fact, the builder could also have
+       simply have closed its end of the pipe, so just to be sure,
+       kill it. */
+    int status = getChildStatus();
+
+    debug("builder process for '%s' finished", worker.store.printStorePath(drvPath));
+
+    buildResult.timesBuilt++;
+    buildResult.stopTime = time(0);
+
+    /* So the child is gone now. */
+    worker.childTerminated(this);
+
+    /* Close the read side of the logger pipe. */
+    closeReadPipes();
+
+    /* Close the log file. */
+    closeLogFile();
+
+    cleanupPostChildKill();
+
+    if (buildResult.cpuUser && buildResult.cpuSystem) {
+        debug("builder for '%s' terminated with status %d, user CPU %.3fs, system CPU %.3fs",
+            worker.store.printStorePath(drvPath),
+            status,
+            ((double) buildResult.cpuUser->count()) / 1000000,
+            ((double) buildResult.cpuSystem->count()) / 1000000);
+    }
+
+    bool diskFull = false;
+
+    try {
+
+        /* Check the exit status. */
+        if (!statusOk(status)) {
+
+            diskFull |= cleanupDecideWhetherDiskFull();
+
+            auto msg = fmt("builder for '%s' %s",
+                Magenta(worker.store.printStorePath(drvPath)),
+                statusToString(status));
+
+            appendLogTailErrorMsg(worker, drvPath, logTail, msg);
+
+            if (diskFull)
+                msg += "\nnote: build failure may have been caused by lack of free disk space";
+
+            throw BuildError(msg);
+        }
+
+        /* Compute the FS closure of the outputs and register them as
+           being valid. */
+        auto builtOutputs = registerOutputs();
+
+        StorePathSet outputPaths;
+        for (auto & [_, output] : builtOutputs)
+            outputPaths.insert(output.outPath);
+        runPostBuildHook(
+            worker.store,
+            *logger,
+            drvPath,
+            outputPaths
+        );
+
+        cleanupPostOutputsRegisteredModeNonCheck();
+
+        /* It is now safe to delete the lock files, since all future
+           lockers will see that the output paths are valid; they will
+           not create new lock files with the same names as the old
+           (unlinked) lock files. */
+        outputLocks.setDeletion(true);
+        outputLocks.unlock();
+
+        co_return done(BuildResult::Built, std::move(builtOutputs));
+
+    } catch (BuildError & e) {
+        outputLocks.unlock();
+
+        BuildResult::Status st = BuildResult::MiscFailure;
+
+#ifndef _WIN32 // TODO abstract over proc exit status
+        if (hook && WIFEXITED(status) && WEXITSTATUS(status) == 101)
+            st = BuildResult::TimedOut;
+
+        else if (hook && (!WIFEXITED(status) || WEXITSTATUS(status) != 100)) {
+        }
+
+        else
+#endif
+        {
+            assert(derivationType);
+            st =
+                dynamic_cast<NotDeterministic*>(&e) ? BuildResult::NotDeterministic :
+                statusOk(status) ? BuildResult::OutputRejected :
+                !derivationType->isSandboxed() || diskFull ? BuildResult::TransientFailure :
+                BuildResult::PermanentFailure;
+        }
+
+        co_return done(st, {}, std::move(e));
+    }
 }
 
 static void chmod_(const Path & path, mode_t mode)