DerivationBuilderImpl::unprepareBuild Just throw error

Aftet the previous simplifications, there is no reason to catch the
error and immediately return it with a `std::variant` --- just let the
caller catch it instead.

src/libstore/build/derivation-building-goal.cc

@@ -804,21 +804,22 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
 
     trace("build done");
 
-    auto res = builder->unprepareBuild();
-    // N.B. cannot use `std::visit` with co-routine return
-    if (auto * ste = std::get_if<0>(&res)) {
+    SingleDrvOutputs builtOutputs;
+    try {
+        builtOutputs = builder->unprepareBuild();
+    } catch (BuildError & e) {
         outputLocks.unlock();
 // Allow selecting a subset of enum values
 #  pragma GCC diagnostic push
 #  pragma GCC diagnostic ignored "-Wswitch-enum"
-        switch (ste->status) {
+        switch (e.status) {
         case BuildResult::HashMismatch:
             worker.hashMismatch = true;
             /* See header, the protocols don't know about `HashMismatch`
                yet, so change it to `OutputRejected`, which they expect
                for this case (hash mismatch is a type of output
                rejection). */
-            ste->status = BuildResult::OutputRejected;
+            e.status = BuildResult::OutputRejected;
             break;
         case BuildResult::NotDeterministic:
             worker.checkMismatch = true;
@@ -828,10 +829,11 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
             break;
         }
 #  pragma GCC diagnostic pop
-        co_return doneFailure(std::move(*ste));
-    } else if (auto * builtOutputs = std::get_if<1>(&res)) {
+        co_return doneFailure(std::move(e));
+    }
+    {
         StorePathSet outputPaths;
-        for (auto & [_, output] : *builtOutputs)
+        for (auto & [_, output] : builtOutputs)
             outputPaths.insert(output.outPath);
         runPostBuildHook(worker.store, *logger, drvPath, outputPaths);
 
@@ -841,9 +843,7 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
            (unlinked) lock files. */
         outputLocks.setDeletion(true);
         outputLocks.unlock();
-        co_return doneSuccess(BuildResult::Built, std::move(*builtOutputs));
-    } else {
-        unreachable();
+        co_return doneSuccess(BuildResult::Built, std::move(builtOutputs));
     }
 #endif
 }

src/libstore/include/nix/store/build/derivation-builder.hh

@@ -187,8 +187,10 @@ struct DerivationBuilder : RestrictionContext
      * processing. A status code and exception are returned, providing
      * more information. The second case indicates success, and
      * realisations for each output of the derivation are returned.
+     *
+     * @throws BuildError
      */
-    virtual std::variant<BuildError, SingleDrvOutputs> unprepareBuild() = 0;
+    virtual SingleDrvOutputs unprepareBuild() = 0;
 
     /**
      * Stop the in-process nix daemon thread.

src/libstore/unix/build/derivation-builder.cc

@@ -191,7 +191,7 @@ public:
 
     void startBuilder() override;
 
-    std::variant<BuildError, SingleDrvOutputs> unprepareBuild() override;
+    SingleDrvOutputs unprepareBuild() override;
 
 protected:
 
@@ -426,7 +426,7 @@ bool DerivationBuilderImpl::prepareBuild()
     return true;
 }
 
-std::variant<BuildError, SingleDrvOutputs> DerivationBuilderImpl::unprepareBuild()
+SingleDrvOutputs DerivationBuilderImpl::unprepareBuild()
 {
     // FIXME: get rid of this, rely on RAII.
     Finally releaseBuildUser([&]() {
@@ -477,49 +477,42 @@ std::variant<BuildError, SingleDrvOutputs> DerivationBuilderImpl::unprepareBuild
 
     bool diskFull = false;
 
-    try {
+    /* Check the exit status. */
+    if (!statusOk(status)) {
 
-        /* Check the exit status. */
-        if (!statusOk(status)) {
+        diskFull |= decideWhetherDiskFull();
 
-            diskFull |= decideWhetherDiskFull();
+        cleanupBuild();
 
-            cleanupBuild();
+        auto msg =
+            fmt("Cannot build '%s'.\n"
+                "Reason: " ANSI_RED "builder %s" ANSI_NORMAL ".",
+                Magenta(store.printStorePath(drvPath)),
+                statusToString(status));
 
-            auto msg =
-                fmt("Cannot build '%s'.\n"
-                    "Reason: " ANSI_RED "builder %s" ANSI_NORMAL ".",
-                    Magenta(store.printStorePath(drvPath)),
-                    statusToString(status));
+        msg += showKnownOutputs(store, drv);
 
-            msg += showKnownOutputs(store, drv);
+        miscMethods->appendLogTailErrorMsg(msg);
 
-            miscMethods->appendLogTailErrorMsg(msg);
+        if (diskFull)
+            msg += "\nnote: build failure may have been caused by lack of free disk space";
 
-            if (diskFull)
-                msg += "\nnote: build failure may have been caused by lack of free disk space";
-
-            throw BuildError(
-                !derivationType.isSandboxed() || diskFull ? BuildResult::TransientFailure
-                                                          : BuildResult::PermanentFailure,
-                msg);
-        }
-
-        /* Compute the FS closure of the outputs and register them as
-           being valid. */
-        auto builtOutputs = registerOutputs();
-
-        /* Delete unused redirected outputs (when doing hash rewriting). */
-        for (auto & i : redirectedOutputs)
-            deletePath(store.Store::toRealPath(i.second));
-
-        deleteTmpDir(true);
-
-        return std::move(builtOutputs);
-
-    } catch (BuildError & e) {
-        return std::move(e);
+        throw BuildError(
+            !derivationType.isSandboxed() || diskFull ? BuildResult::TransientFailure : BuildResult::PermanentFailure,
+            msg);
     }
+
+    /* Compute the FS closure of the outputs and register them as
+       being valid. */
+    auto builtOutputs = registerOutputs();
+
+    /* Delete unused redirected outputs (when doing hash rewriting). */
+    for (auto & i : redirectedOutputs)
+        deletePath(store.Store::toRealPath(i.second));
+
+    deleteTmpDir(true);
+
+    return builtOutputs;
 }
 
 void DerivationBuilderImpl::cleanupBuild()

src/libstore/unix/build/linux-derivation-builder.cc

@@ -659,7 +659,7 @@ struct ChrootLinuxDerivationBuilder : ChrootDerivationBuilder, LinuxDerivationBu
             throw SysError("setuid failed");
     }
 
-    std::variant<BuildError, SingleDrvOutputs> unprepareBuild() override
+    SingleDrvOutputs unprepareBuild() override
     {
         sandboxMountNamespace = -1;
         sandboxUserNamespace = -1;