Consolidate logic choosing where we can/should build a bit

I want to separate "policy" from "mechanism".

Now the logic to decide how to build (a policy choice, though with some
hard constraints) is all in derivation building goal, and all in the
same spot. build hook, external builder, or local builder --- the choice
between all three is made in the same spot --- pure policy.

Now, if you want to use the external deriation builder, you simply
provide the `ExternalBuilder` you wish to use, and there is no
additional checking --- pure mechanism. It is the responsibility of the
caller to choose an external builder that works for the derivation in
question.

Also, `checkSystem()` was the only thing throwing `BuildError` from
`startBuilder`. Now that that is gone, we can now remove the
`try...catch` around that.

src/libstore/build/derivation-builder.cc

@@ -0,0 +1,27 @@
+#include "nix/util/json-utils.hh"
+#include "nix/store/build/derivation-builder.hh"
+
+namespace nlohmann {
+
+using namespace nix;
+
+ExternalBuilder adl_serializer<ExternalBuilder>::from_json(const json & json)
+{
+    auto obj = getObject(json);
+    return {
+        .systems = valueAt(obj, "systems"),
+        .program = valueAt(obj, "program"),
+        .args = valueAt(obj, "args"),
+    };
+}
+
+void adl_serializer<ExternalBuilder>::to_json(json & json, const ExternalBuilder & eb)
+{
+    json = {
+        {"systems", eb.systems},
+        {"program", eb.program},
+        {"args", eb.args},
+    };
+}
+
+} // namespace nlohmann

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

@@ -491,6 +491,8 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
 
     bool useHook;
 
+    const ExternalBuilder * externalBuilder = nullptr;
+
     while (true) {
         trace("trying to build");
 
@@ -584,7 +586,42 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
                 co_await waitForAWhile();
                 continue;
             case rpDecline:
-                /* We should do it ourselves. */
+                /* We should do it ourselves.
+
+                   Now that we've decided we can't / won't do a remote build, check
+                   that we can in fact build locally. First see if there is an
+                   external builder for a "semi-local build". If there is, prefer to
+                   use that. If there is not, then check if we can do a "true" local
+                   build. */
+
+                externalBuilder = settings.findExternalDerivationBuilderIfSupported(*drv);
+
+                if (!externalBuilder && !drvOptions->canBuildLocally(worker.store, *drv)) {
+                    auto msg =
+                        fmt("Cannot build '%s'.\n"
+                            "Reason: " ANSI_RED "required system or feature not available" ANSI_NORMAL
+                            "\n"
+                            "Required system: '%s' with features {%s}\n"
+                            "Current system: '%s' with features {%s}",
+                            Magenta(worker.store.printStorePath(drvPath)),
+                            Magenta(drv->platform),
+                            concatStringsSep(", ", drvOptions->getRequiredSystemFeatures(*drv)),
+                            Magenta(settings.thisSystem),
+                            concatStringsSep<StringSet>(", ", worker.store.Store::config.systemFeatures));
+
+                    // since aarch64-darwin has Rosetta 2, this user can actually run x86_64-darwin on their hardware -
+                    // we should tell them to run the command to install Darwin 2
+                    if (drv->platform == "x86_64-darwin" && settings.thisSystem == "aarch64-darwin")
+                        msg += fmt(
+                            "\nNote: run `%s` to run programs for x86_64-darwin",
+                            Magenta(
+                                "/usr/sbin/softwareupdate --install-rosetta && launchctl stop org.nixos.nix-daemon"));
+
+                    builder.reset();
+                    outputLocks.unlock();
+                    worker.permanentFailure = true;
+                    co_return doneFailure({BuildResult::Failure::InputRejected, std::move(msg)});
+                }
                 useHook = false;
                 break;
             }
@@ -771,12 +808,7 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
                 co_return doneFailure(std::move(e));
             }
 
-            /* If we have to wait and retry (see below), then `builder` will
+            DerivationBuilderParams params{
-               already be created, so we don't need to create it again. */
-            builder = makeDerivationBuilder(
-                *localStoreP,
-                std::make_unique<DerivationBuildingGoalCallbacks>(*this, builder),
-                DerivationBuilderParams{
                 .drvPath = drvPath,
                 .buildResult = buildResult,
                 .drv = *drv,
@@ -787,20 +819,24 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
                 .defaultPathsInChroot = std::move(defaultPathsInChroot),
                 .systemFeatures = worker.store.config.systemFeatures.get(),
                 .desugaredEnv = std::move(desugaredEnv),
-                });
+            };
+
+            /* If we have to wait and retry (see below), then `builder` will
+               already be created, so we don't need to create it again. */
+            builder = externalBuilder ? makeExternalDerivationBuilder(
+                                            *localStoreP,
+                                            std::make_unique<DerivationBuildingGoalCallbacks>(*this, builder),
+                                            std::move(params),
+                                            *externalBuilder)
+                                      : makeDerivationBuilder(
+                                            *localStoreP,
+                                            std::make_unique<DerivationBuildingGoalCallbacks>(*this, builder),
+                                            std::move(params));
         }
 
-        std::optional<Descriptor> builderOutOpt;
+        if (auto builderOutOpt = builder->startBuild()) {
-        try {
+            builderOut = *std::move(builderOutOpt);
-            /* Okay, we have to build. */
+        } else {
-            builderOutOpt = builder->startBuild();
-        } catch (BuildError & e) {
-            builder.reset();
-            outputLocks.unlock();
-            worker.permanentFailure = true;
-            co_return doneFailure(std::move(e)); // InputRejected
-        }
-        if (!builderOutOpt) {
             if (!actLock)
                 actLock = std::make_unique<Activity>(
                     *logger,
@@ -809,9 +845,7 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
                     fmt("waiting for a free build user ID for '%s'", Magenta(worker.store.printStorePath(drvPath))));
             co_await waitForAWhile();
             continue;
-        } else {
+        }
-            builderOut = *std::move(builderOutOpt);
-        };
 
         break;
     }

src/libstore/globals.cc

@@ -258,6 +258,15 @@ Path Settings::getDefaultSSLCertFile()
     return "";
 }
 
+const ExternalBuilder * Settings::findExternalDerivationBuilderIfSupported(const Derivation & drv)
+{
+    if (auto it = std::ranges::find_if(
+            externalBuilders.get(), [&](const auto & handler) { return handler.systems.contains(drv.platform); });
+        it != externalBuilders.get().end())
+        return &*it;
+    return nullptr;
+}
+
 std::string nixVersion = PACKAGE_VERSION;
 
 NLOHMANN_JSON_SERIALIZE_ENUM(
@@ -379,8 +388,6 @@ unsigned int MaxBuildJobsSetting::parse(const std::string & str) const
     }
 }
 
-NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE(Settings::ExternalBuilder, systems, program, args);
-
 template<>
 Settings::ExternalBuilders BaseSetting<Settings::ExternalBuilders>::parse(const std::string & str) const
 {

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

@@ -1,12 +1,15 @@
 #pragma once
 ///@file
 
+#include <nlohmann/json_fwd.hpp>
+
 #include "nix/store/build-result.hh"
 #include "nix/store/derivation-options.hh"
 #include "nix/store/build/derivation-building-misc.hh"
 #include "nix/store/derivations.hh"
 #include "nix/store/parsed-derivations.hh"
 #include "nix/util/processes.hh"
+#include "nix/util/json-impls.hh"
 #include "nix/store/restricted-store.hh"
 #include "nix/store/build/derivation-env-desugar.hh"
 
@@ -179,9 +182,28 @@ struct DerivationBuilder : RestrictionContext
     virtual bool killChild() = 0;
 };
 
+struct ExternalBuilder
+{
+    StringSet systems;
+    Path program;
+    std::vector<std::string> args;
+};
+
 #ifndef _WIN32 // TODO enable `DerivationBuilder` on Windows
 std::unique_ptr<DerivationBuilder> makeDerivationBuilder(
     LocalStore & store, std::unique_ptr<DerivationBuilderCallbacks> miscMethods, DerivationBuilderParams params);
+
+/**
+ * @param handler Must be chosen such that it supports the given
+ * derivation.
+ */
+std::unique_ptr<DerivationBuilder> makeExternalDerivationBuilder(
+    LocalStore & store,
+    std::unique_ptr<DerivationBuilderCallbacks> miscMethods,
+    DerivationBuilderParams params,
+    const ExternalBuilder & handler);
 #endif
 
 } // namespace nix
+
+JSON_IMPL(nix::ExternalBuilder)

src/libstore/include/nix/store/globals.hh

@@ -1373,13 +1373,6 @@ public:
           Set it to 1 to warn on all paths.
         )"};
 
-    struct ExternalBuilder
-    {
-        StringSet systems;
-        Path program;
-        std::vector<std::string> args;
-    };
-
     using ExternalBuilders = std::vector<ExternalBuilder>;
 
     Setting<ExternalBuilders> externalBuilders{
@@ -1443,6 +1436,12 @@ public:
         //        Current system: 'aarch64-darwin' with features {apple-virt, benchmark, big-parallel, nixos-test}
         // Xp::ExternalBuilders
     };
+
+    /**
+     * Finds the first external derivation builder that supports this
+     * derivation, or else returns a null pointer.
+     */
+    const ExternalBuilder * findExternalDerivationBuilderIfSupported(const Derivation & drv);
 };
 
 // FIXME: don't use a global variable.

src/libstore/meson.build

@@ -298,6 +298,7 @@ sources = files(
   'aws-creds.cc',
   'binary-cache-store.cc',
   'build-result.cc',
+  'build/derivation-builder.cc',
   'build/derivation-building-goal.cc',
   'build/derivation-check.cc',
   'build/derivation-env-desugar.cc',

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

@@ -229,12 +229,6 @@ protected:
         return acquireUserLock(1, false);
     }
 
-    /**
-     * Throw an exception if we can't do this derivation because of
-     * missing system features.
-     */
-    virtual void checkSystem();
-
     /**
      * Return the paths that should be made available in the sandbox.
      * This includes:
@@ -672,33 +666,6 @@ static bool checkNotWorldWritable(std::filesystem::path path)
     return true;
 }
 
-void DerivationBuilderImpl::checkSystem()
-{
-    /* Right platform? */
-    if (!drvOptions.canBuildLocally(store, drv)) {
-        auto msg =
-            fmt("Cannot build '%s'.\n"
-                "Reason: " ANSI_RED "required system or feature not available" ANSI_NORMAL
-                "\n"
-                "Required system: '%s' with features {%s}\n"
-                "Current system: '%s' with features {%s}",
-                Magenta(store.printStorePath(drvPath)),
-                Magenta(drv.platform),
-                concatStringsSep(", ", drvOptions.getRequiredSystemFeatures(drv)),
-                Magenta(settings.thisSystem),
-                concatStringsSep<StringSet>(", ", store.Store::config.systemFeatures));
-
-        // since aarch64-darwin has Rosetta 2, this user can actually run x86_64-darwin on their hardware - we should
-        // tell them to run the command to install Darwin 2
-        if (drv.platform == "x86_64-darwin" && settings.thisSystem == "aarch64-darwin")
-            msg +=
-                fmt("\nNote: run `%s` to run programs for x86_64-darwin",
-                    Magenta("/usr/sbin/softwareupdate --install-rosetta && launchctl stop org.nixos.nix-daemon"));
-
-        throw BuildError(BuildResult::Failure::InputRejected, msg);
-    }
-}
-
 std::optional<Descriptor> DerivationBuilderImpl::startBuild()
 {
     if (useBuildUsers()) {
@@ -709,8 +676,6 @@ std::optional<Descriptor> DerivationBuilderImpl::startBuild()
             return std::nullopt;
     }
 
-    checkSystem();
-
     /* Make sure that no other processes are executing under the
        sandbox uids. This must be done before any chownToBuilder()
        calls. */
@@ -1922,9 +1887,6 @@ namespace nix {
 std::unique_ptr<DerivationBuilder> makeDerivationBuilder(
     LocalStore & store, std::unique_ptr<DerivationBuilderCallbacks> miscMethods, DerivationBuilderParams params)
 {
-    if (auto builder = ExternalDerivationBuilder::newIfSupported(store, miscMethods, params))
-        return builder;
-
     bool useSandbox = false;
 
     /* Are we doing a sandboxed build? */

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

@@ -2,32 +2,19 @@ namespace nix {
 
 struct ExternalDerivationBuilder : DerivationBuilderImpl
 {
-    Settings::ExternalBuilder externalBuilder;
+    ExternalBuilder externalBuilder;
 
     ExternalDerivationBuilder(
         LocalStore & store,
         std::unique_ptr<DerivationBuilderCallbacks> miscMethods,
         DerivationBuilderParams params,
-        Settings::ExternalBuilder externalBuilder)
+        ExternalBuilder externalBuilder)
         : DerivationBuilderImpl(store, std::move(miscMethods), std::move(params))
         , externalBuilder(std::move(externalBuilder))
     {
         experimentalFeatureSettings.require(Xp::ExternalBuilders);
     }
 
-    static std::unique_ptr<ExternalDerivationBuilder> newIfSupported(
-        LocalStore & store, std::unique_ptr<DerivationBuilderCallbacks> & miscMethods, DerivationBuilderParams & params)
-    {
-        if (auto it = std::ranges::find_if(
-                settings.externalBuilders.get(),
-                [&](const auto & handler) { return handler.systems.contains(params.drv.platform); });
-            it != settings.externalBuilders.get().end()) {
-            return std::make_unique<ExternalDerivationBuilder>(
-                store, std::move(miscMethods), std::move(params), *it);
-        }
-        return {};
-    }
-
     Path tmpDirInSandbox() override
     {
         /* In a sandbox, for determinism, always use the same temporary
@@ -41,8 +28,6 @@ struct ExternalDerivationBuilder : DerivationBuilderImpl
         createDir(tmpDir, 0700);
     }
 
-    void checkSystem() override {}
-
     void startChild() override
     {
         if (drvOptions.getRequiredSystemFeatures(drv).count("recursive-nix"))
@@ -121,4 +106,13 @@ struct ExternalDerivationBuilder : DerivationBuilderImpl
     }
 };
 
+std::unique_ptr<DerivationBuilder> makeExternalDerivationBuilder(
+    LocalStore & store,
+    std::unique_ptr<DerivationBuilderCallbacks> miscMethods,
+    DerivationBuilderParams params,
+    const ExternalBuilder & handler)
+{
+    return std::make_unique<ExternalDerivationBuilder>(store, std::move(miscMethods), std::move(params), handler);
+}
+
 } // namespace nix