From 049c7b33690b716799280f241824c582d260051f Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Sun, 25 Jan 2026 15:34:59 +0100 Subject: [PATCH] libstore: move the builtin builders to our new executable this means that builtinFetchurl runs in a real process now, and thus we no longer need its workarounds for running in a forked process. forking dropped the signal handler thread and broke the curl state via sharing, neither of which happens any more now. we can run fetchurl builtins and their actions straight from the main thread of our builder now, and the temporary files and settings overrides we did are now also unnecessary. Change-Id: I738171bc120ffcd541b7ff1424fed7924c2cdc1d --- lix/legacy/builtin-builder.cc | 25 ++++++- lix/libstore/build/local-derivation-goal.cc | 83 ++++++++++----------- lix/libstore/builtins.hh | 5 +- lix/libstore/builtins/fetchurl.cc | 55 +++++--------- 4 files changed, 86 insertions(+), 82 deletions(-) diff --git a/lix/legacy/builtin-builder.cc b/lix/legacy/builtin-builder.cc index 85276c729..78f10055c 100644 --- a/lix/legacy/builtin-builder.cc +++ b/lix/legacy/builtin-builder.cc @@ -62,7 +62,30 @@ static int main_builtin_builder(AsyncIoRoot & aio, std::string programName, Stri const auto builder = getAttr("builder"); - throw Error("unknown builtin builder %s", builder); + if (builder == "builtin:fetchurl") { + const auto outputHashMode = getAttr("outputHashMode"); + const auto hash = outputHashMode == "flat" ? [&] -> std::optional { + const auto ht = parseHashTypeOpt(getAttr("outputHashAlgo")); + return newHashAllowEmpty(getAttr("outputHash"), ht); + }() + : std::nullopt; + BuiltinFetchurl{ + .storePath = getAttr("out"), + .mainUrl = getAttr("url"), + .unpack = getOr(env, "unpack", "0") == "1", + .executable = getOr(env, "executable", "0") == "1", + .hash = hash, + } + .run(aio); + } else if (builder == "builtin:buildenv") { + builtinBuildenv(getAttr("out"), tokenizeString(getAttr("derivations")), getAttr("manifest")); + } else if (builder == "builtin:unpack-channel") { + builtinUnpackChannel(getAttr("out"), getAttr("channelName"), getAttr("src")); + } else { + throw Error("unknown builtin builder %s", builder); + } + + return 0; } void registerLegacyBuiltinBuilder() diff --git a/lix/libstore/build/local-derivation-goal.cc b/lix/libstore/build/local-derivation-goal.cc index 0d39c1286..74d31c6a2 100644 --- a/lix/libstore/build/local-derivation-goal.cc +++ b/lix/libstore/build/local-derivation-goal.cc @@ -72,6 +72,8 @@ #include #include +using std::literals::operator""sv; + namespace nix { static kj::Promise> handleDiffHook( @@ -1230,51 +1232,48 @@ void LocalDerivationGoal::runChild( /* Execute the program. This should not return. */ if (drv->isBuiltin()) { - try { - logger = makeJSONLogger(*logger); + Strings args{ + "builtin-builder", + }; - BasicDerivation & drv2(*drv); - for (auto & e : drv2.env) - e.second = rewriteStrings(e.second, inputRewrites); + std::map overriddenSettings; + settings.getSettings(overriddenSettings, true); - auto getAttr = [&](const std::string & name) { - auto i = drv2.env.find(name); - if (i == drv2.env.end()) { - throw Error("attribute '%s' missing", name); - } - return i->second; - }; - - if (drv->builder == "builtin:fetchurl") { - const auto hash = getAttr("outputHashMode") == "flat" ? [&] -> std::optional { - const auto ht = parseHashTypeOpt(getAttr("outputHashAlgo")); - return newHashAllowEmpty(getAttr("outputHash"), ht); - }() - : std::nullopt; - BuiltinFetchurl{ - .storePath = getAttr("out"), - .mainUrl = getAttr("url"), - .unpack = getOr(drv2.env, "unpack", "") == "1", - .executable = getOr(drv2.env, "executable", "") == "1", - .hash = hash, - .netrcData = netrcData, - .caFileData = caFileData, - } - .run(); - } else if (drv->builder == "builtin:buildenv") { - builtinBuildenv( - getAttr("out"), tokenizeString(getAttr("derivations")), getAttr("manifest") - ); - } else if (drv->builder == "builtin:unpack-channel") { - builtinUnpackChannel(getAttr("out"), getAttr("channelName"), getAttr("src")); - } else { - throw Error("unsupported builtin builder '%1%'", drv->builder.substr(8)); - } - _exit(0); - } catch (std::exception & e) { // NOLINT(lix-foreign-exceptions) - writeFull(STDERR_FILENO, e.what() + std::string("\n")); - _exit(1); + if (!netrcData.empty()) { + auto path = tmpDirInSandbox + "/netrc"; + overriddenSettings[settings.netrcFile.name].value = path; + writeFile(path, netrcData, 0600); } + if (!caFileData.empty()) { + auto path = tmpDirInSandbox + "/cafile"; + overriddenSettings[settings.caFile.name].value = path; + writeFile(path, caFileData, 0600); + } + + for (const auto & [setting, value] : overriddenSettings) { + args.push_back("--" + setting); + args.push_back(escapeNul(value.value)); + } + + args.push_back("--"); + + for (const auto & [k, v] : drv->env) { + args.push_back("--" + escapeNul(k)); + args.push_back(rewriteStrings(escapeNul(v), inputRewrites)); + } + + // we pass on the *entire* daemon env to the builtin builder for historical + // reasons (libcurl inspects some env vars to modify how it behaves, and we + // are not fully sure yet that we pass through all of them to sandboxes. we + // should change this eventually. TODO: investigate curl env var use first) + Strings envs; + for (auto & [k, v] : getEnv()) { + envs.emplace_back(k + "=" + v); + } + + execBuilder(LIX_LIBEXEC_DIR "/builtin-builder", std::move(args), envs); + + throw Error("builtin builder '%1%' failed to exec", drv->builder.substr(8)); } execBuilder(drv->builder, args, envStrs); diff --git a/lix/libstore/builtins.hh b/lix/libstore/builtins.hh index 7b847120b..530210a08 100644 --- a/lix/libstore/builtins.hh +++ b/lix/libstore/builtins.hh @@ -1,6 +1,7 @@ #pragma once ///@file +#include "lix/libutil/async.hh" #include "lix/libutil/hash.hh" #include "lix/libutil/types.hh" @@ -14,10 +15,8 @@ struct BuiltinFetchurl bool unpack; bool executable; std::optional hash; - std::string netrcData; - std::string caFileData; - void run(); + void run(AsyncIoRoot & aio); }; void builtinUnpackChannel(const Path & out, const std::string & channelName, const std::string & src); diff --git a/lix/libstore/builtins/fetchurl.cc b/lix/libstore/builtins/fetchurl.cc index 9a5184e19..0fc116dac 100644 --- a/lix/libstore/builtins/fetchurl.cc +++ b/lix/libstore/builtins/fetchurl.cc @@ -9,22 +9,9 @@ namespace nix { -void BuiltinFetchurl::run() +void BuiltinFetchurl::run(AsyncIoRoot & aio) { - /* Make the host's netrc data available. Too bad curl requires - this to be stored in a file. It would be nice if we could just - pass a pointer to the data. */ - if (netrcData != "") { - settings.netrcFile.override("netrc"); - writeFile(settings.netrcFile, netrcData, 0600); - } - - settings.caFile.override("ca-certificates.crt"); - writeFile(settings.caFile, caFileData, 0600); - - /* Note: have to use a fresh fileTransfer here because we're in - a forked process. */ - auto fileTransfer = makeFileTransfer(); + auto fileTransfer = getFileTransfer(); // we also have to run the remainder of this function in a fresh thread so // we can have an aio root. the existing root on the current thread is not @@ -47,31 +34,27 @@ void BuiltinFetchurl::run() } }; - std::async(std::launch::async, [&] { - AsyncIoRoot aio; - - /* Try the hashed mirrors first. */ - if (hash) { - for (auto hashedMirror : settings.hashedMirrors.get()) { - try { - if (!hashedMirror.ends_with("/")) { - hashedMirror += '/'; - } - fetch( - aio, - hashedMirror + printHashType(hash->type) + "/" - + hash->to_string(HashFormat::Base16, false) - ); - return; - } catch (Error & e) { - debug("%1%", Uncolored(e.what())); + /* Try the hashed mirrors first. */ + if (hash) { + for (auto hashedMirror : settings.hashedMirrors.get()) { + try { + if (!hashedMirror.ends_with("/")) { + hashedMirror += '/'; } + fetch( + aio, + hashedMirror + printHashType(hash->type) + "/" + + hash->to_string(HashFormat::Base16, false) + ); + return; + } catch (Error & e) { + debug("%1%", Uncolored(e.what())); } } + } - /* Otherwise try the specified URL. */ - fetch(aio, mainUrl); - }).get(); + /* Otherwise try the specified URL. */ + fetch(aio, mainUrl); } }