From b56dd21c311b1ad1e19bfb1180a0b5f94834b85d Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 10 Oct 2025 17:18:40 -0400 Subject: [PATCH 1/3] `Settings::ExternalBuilder::systems` make set Nothing cares about the order, actually. --- src/libstore/include/nix/store/globals.hh | 2 +- src/libstore/unix/build/external-derivation-builder.cc | 7 +++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/libstore/include/nix/store/globals.hh b/src/libstore/include/nix/store/globals.hh index 1b59bd6fc..be3561848 100644 --- a/src/libstore/include/nix/store/globals.hh +++ b/src/libstore/include/nix/store/globals.hh @@ -1375,7 +1375,7 @@ public: struct ExternalBuilder { - std::vector systems; + StringSet systems; Path program; std::vector args; }; diff --git a/src/libstore/unix/build/external-derivation-builder.cc b/src/libstore/unix/build/external-derivation-builder.cc index 71cfd1a62..f20badb85 100644 --- a/src/libstore/unix/build/external-derivation-builder.cc +++ b/src/libstore/unix/build/external-derivation-builder.cc @@ -19,10 +19,9 @@ struct ExternalDerivationBuilder : DerivationBuilderImpl LocalStore & store, std::unique_ptr & miscMethods, DerivationBuilderParams & params) { for (auto & handler : settings.externalBuilders.get()) { - for (auto & system : handler.systems) - if (params.drv.platform == system) - return std::make_unique( - store, std::move(miscMethods), std::move(params), handler); + if (handler.systems.contains(params.drv.platform)) + return std::make_unique( + store, std::move(miscMethods), std::move(params), handler); } return {}; } From 2ff59ec3e0fc093dcd0064bc5df21c5d62ea2445 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 10 Oct 2025 17:27:41 -0400 Subject: [PATCH 2/3] Use `std::ranges::find_if` for finding external builders Co-authored-by: Sergei Zimmerman --- src/libstore/unix/build/external-derivation-builder.cc | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/libstore/unix/build/external-derivation-builder.cc b/src/libstore/unix/build/external-derivation-builder.cc index f20badb85..ebcaad525 100644 --- a/src/libstore/unix/build/external-derivation-builder.cc +++ b/src/libstore/unix/build/external-derivation-builder.cc @@ -18,10 +18,12 @@ struct ExternalDerivationBuilder : DerivationBuilderImpl static std::unique_ptr newIfSupported( LocalStore & store, std::unique_ptr & miscMethods, DerivationBuilderParams & params) { - for (auto & handler : settings.externalBuilders.get()) { - if (handler.systems.contains(params.drv.platform)) - return std::make_unique( - store, std::move(miscMethods), std::move(params), handler); + 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( + store, std::move(miscMethods), std::move(params), *it); } return {}; } From b57caaa1a273323b596097ab5509797b38e2e272 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sat, 16 Aug 2025 14:25:28 -0400 Subject: [PATCH 3/3] 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 | 27 ++++++ .../build/derivation-building-goal.cc | 94 +++++++++++++------ src/libstore/globals.cc | 11 ++- .../nix/store/build/derivation-builder.hh | 22 +++++ src/libstore/include/nix/store/globals.hh | 13 ++- src/libstore/meson.build | 1 + src/libstore/unix/build/derivation-builder.cc | 38 -------- .../unix/build/external-derivation-builder.cc | 28 +++--- 8 files changed, 140 insertions(+), 94 deletions(-) create mode 100644 src/libstore/build/derivation-builder.cc diff --git a/src/libstore/build/derivation-builder.cc b/src/libstore/build/derivation-builder.cc new file mode 100644 index 000000000..39ac40175 --- /dev/null +++ b/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::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::to_json(json & json, const ExternalBuilder & eb) +{ + json = { + {"systems", eb.systems}, + {"program", eb.program}, + {"args", eb.args}, + }; +} + +} // namespace nlohmann diff --git a/src/libstore/build/derivation-building-goal.cc b/src/libstore/build/derivation-building-goal.cc index 001816ca0..e8ee945d9 100644 --- a/src/libstore/build/derivation-building-goal.cc +++ b/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(", ", 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,36 +808,35 @@ Goal::Co DerivationBuildingGoal::tryToBuild() co_return doneFailure(std::move(e)); } + DerivationBuilderParams params{ + .drvPath = drvPath, + .buildResult = buildResult, + .drv = *drv, + .drvOptions = *drvOptions, + .inputPaths = inputPaths, + .initialOutputs = initialOutputs, + .buildMode = buildMode, + .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 = makeDerivationBuilder( - *localStoreP, - std::make_unique(*this, builder), - DerivationBuilderParams{ - .drvPath = drvPath, - .buildResult = buildResult, - .drv = *drv, - .drvOptions = *drvOptions, - .inputPaths = inputPaths, - .initialOutputs = initialOutputs, - .buildMode = buildMode, - .defaultPathsInChroot = std::move(defaultPathsInChroot), - .systemFeatures = worker.store.config.systemFeatures.get(), - .desugaredEnv = std::move(desugaredEnv), - }); + builder = externalBuilder ? makeExternalDerivationBuilder( + *localStoreP, + std::make_unique(*this, builder), + std::move(params), + *externalBuilder) + : makeDerivationBuilder( + *localStoreP, + std::make_unique(*this, builder), + std::move(params)); } - std::optional builderOutOpt; - try { - /* Okay, we have to build. */ - builderOutOpt = builder->startBuild(); - } catch (BuildError & e) { - builder.reset(); - outputLocks.unlock(); - worker.permanentFailure = true; - co_return doneFailure(std::move(e)); // InputRejected - } - if (!builderOutOpt) { + if (auto builderOutOpt = builder->startBuild()) { + builderOut = *std::move(builderOutOpt); + } else { if (!actLock) actLock = std::make_unique( *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; } diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc index 58a649fc5..4fdb820a9 100644 --- a/src/libstore/globals.cc +++ b/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::parse(const std::string & str) const { diff --git a/src/libstore/include/nix/store/build/derivation-builder.hh b/src/libstore/include/nix/store/build/derivation-builder.hh index 63ef2b665..5fad26e83 100644 --- a/src/libstore/include/nix/store/build/derivation-builder.hh +++ b/src/libstore/include/nix/store/build/derivation-builder.hh @@ -1,12 +1,15 @@ #pragma once ///@file +#include + #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 args; +}; + #ifndef _WIN32 // TODO enable `DerivationBuilder` on Windows std::unique_ptr makeDerivationBuilder( LocalStore & store, std::unique_ptr miscMethods, DerivationBuilderParams params); + +/** + * @param handler Must be chosen such that it supports the given + * derivation. + */ +std::unique_ptr makeExternalDerivationBuilder( + LocalStore & store, + std::unique_ptr miscMethods, + DerivationBuilderParams params, + const ExternalBuilder & handler); #endif } // namespace nix + +JSON_IMPL(nix::ExternalBuilder) diff --git a/src/libstore/include/nix/store/globals.hh b/src/libstore/include/nix/store/globals.hh index be3561848..14647c05f 100644 --- a/src/libstore/include/nix/store/globals.hh +++ b/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 args; - }; - using ExternalBuilders = std::vector; Setting 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. diff --git a/src/libstore/meson.build b/src/libstore/meson.build index a3502c2e0..728de2dfd 100644 --- a/src/libstore/meson.build +++ b/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', diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index 5bdd843bd..0158505a5 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/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(", ", 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 DerivationBuilderImpl::startBuild() { if (useBuildUsers()) { @@ -709,8 +676,6 @@ std::optional 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 makeDerivationBuilder( LocalStore & store, std::unique_ptr miscMethods, DerivationBuilderParams params) { - if (auto builder = ExternalDerivationBuilder::newIfSupported(store, miscMethods, params)) - return builder; - bool useSandbox = false; /* Are we doing a sandboxed build? */ diff --git a/src/libstore/unix/build/external-derivation-builder.cc b/src/libstore/unix/build/external-derivation-builder.cc index ebcaad525..7ddb6e093 100644 --- a/src/libstore/unix/build/external-derivation-builder.cc +++ b/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 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 newIfSupported( - LocalStore & store, std::unique_ptr & 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( - 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 makeExternalDerivationBuilder( + LocalStore & store, + std::unique_ptr miscMethods, + DerivationBuilderParams params, + const ExternalBuilder & handler) +{ + return std::make_unique(store, std::move(miscMethods), std::move(params), handler); +} + } // namespace nix