From 7068cbf0106fec9cd2cf1535b9cc8753e5c9f5e0 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Wed, 21 Jan 2026 15:59:30 +0100 Subject: [PATCH] libstore: extract env and args rewriting from child this really doesn't have to be here, it doesn't help very much. doing it in the parent is cheap enough to not care and sandbox setup is not async yet *anyway*, so we would not even notice if the old way was any faster. Change-Id: I5a3a99af0fa5928e9a42f9c6589d98ff38b8c775 --- lix/libstore/build/local-derivation-goal.cc | 39 ++++++++++++--------- lix/libstore/build/local-derivation-goal.hh | 10 ++++-- lix/libstore/platform/linux.cc | 7 ++-- lix/libstore/platform/linux.hh | 2 ++ 4 files changed, 38 insertions(+), 20 deletions(-) diff --git a/lix/libstore/build/local-derivation-goal.cc b/lix/libstore/build/local-derivation-goal.cc index 159657e79..078920a33 100644 --- a/lix/libstore/build/local-derivation-goal.cc +++ b/lix/libstore/build/local-derivation-goal.cc @@ -860,8 +860,23 @@ try { setupConfiguredCertificateAuthority(); } + /* Fill in the environment. */ + Strings envStrs; + for (auto & i : env) { + envStrs.push_back(rewriteStrings(i.first + "=" + i.second, inputRewrites)); + } + + /* Fill in the arguments. */ + Strings args; + + args.push_back(std::string(baseNameOf(drv->builder))); + + for (auto & i : drv->args) { + args.push_back(rewriteStrings(i, inputRewrites)); + } + /* Fork a child to build the package. */ - pid = startChild(netrcData, caFileData, std::move(builderOut)); + pid = startChild(netrcData, caFileData, envStrs, args, std::move(builderOut)); /* parent */ pid.setSeparatePG(true); @@ -900,6 +915,8 @@ try { Pid LocalDerivationGoal::startChild( const std::optional & netrcData, const std::optional & caFileData, + const Strings & envStrs, + const Strings & args, AutoCloseFD logPTY ) { @@ -908,7 +925,7 @@ Pid LocalDerivationGoal::startChild( throw SysError("failed to redirect build output to log file"); } closeOnExec(STDERR_FILENO, false); - runChild(netrcData, caFileData); + runChild(netrcData, caFileData, envStrs, args); }); } @@ -1144,7 +1161,10 @@ void LocalDerivationGoal::chownToBuilder(const AutoCloseFD & fd) } void LocalDerivationGoal::runChild( - const std::optional & netrcData, const std::optional & caFileData + const std::optional & netrcData, + const std::optional & caFileData, + const Strings & envStrs, + const Strings & args ) { /* Warning: in the child we should absolutely not make any SQLite @@ -1174,11 +1194,6 @@ void LocalDerivationGoal::runChild( // FIXME: set other limits to deterministic values? - /* Fill in the environment. */ - Strings envStrs; - for (auto & i : env) - envStrs.push_back(rewriteStrings(i.first + "=" + i.second, inputRewrites)); - /* If we are running in `build-users' mode, then switch to the user we allocated above. Make sure that we drop all root privileges. Note that above we have closed all file @@ -1205,14 +1220,6 @@ void LocalDerivationGoal::runChild( finishChildSetup(); - /* Fill in the arguments. */ - Strings args; - - args.push_back(std::string(baseNameOf(drv->builder))); - - for (auto & i : drv->args) - args.push_back(rewriteStrings(i, inputRewrites)); - /* Indicate that we managed to set up the build environment. */ writeFull(STDERR_FILENO, std::string("\2\n")); diff --git a/lix/libstore/build/local-derivation-goal.hh b/lix/libstore/build/local-derivation-goal.hh index 870122306..31fc2d3a2 100644 --- a/lix/libstore/build/local-derivation-goal.hh +++ b/lix/libstore/build/local-derivation-goal.hh @@ -214,8 +214,12 @@ struct LocalDerivationGoal : public DerivationGoal /** * Run the builder's process. */ - void - runChild(const std::optional & netrcData, const std::optional & caFileData); + void runChild( + const std::optional & netrcData, + const std::optional & caFileData, + const Strings & envStrs, + const Strings & args + ); /** * Check that the derivation outputs all exist and register them @@ -315,6 +319,8 @@ protected: virtual Pid startChild( const std::optional & netrcData, const std::optional & caFileData, + const Strings & envStrs, + const Strings & args, AutoCloseFD logPTY ); diff --git a/lix/libstore/platform/linux.cc b/lix/libstore/platform/linux.cc index c6c94ed63..50cf9d295 100644 --- a/lix/libstore/platform/linux.cc +++ b/lix/libstore/platform/linux.cc @@ -1241,6 +1241,8 @@ bool LinuxLocalDerivationGoal::prepareChildSetup() Pid LinuxLocalDerivationGoal::startChild( const std::optional & netrcData, const std::optional & caFileData, + const Strings & envStrs, + const Strings & args, AutoCloseFD logPTY ) { @@ -1253,7 +1255,7 @@ Pid LinuxLocalDerivationGoal::startChild( // If we're not sandboxing no need to faff about, use the fallback if (!useChroot) { - return LocalDerivationGoal::startChild(netrcData, caFileData, std::move(logPTY)); + return LocalDerivationGoal::startChild(netrcData, caFileData, envStrs, args, std::move(logPTY)); } /* Set up private namespaces for the build: @@ -1329,7 +1331,8 @@ Pid LinuxLocalDerivationGoal::startChild( options.cloneFlags |= CLONE_NEWUSER; } - pid_t child = startProcess([&]() { runChild(netrcData, caFileData); }, options).release(); + pid_t child = + startProcess([&]() { runChild(netrcData, caFileData, envStrs, args); }, options).release(); writeFull(sendPid.writeSide.get(), fmt("%d\n", child)); _exit(0); diff --git a/lix/libstore/platform/linux.hh b/lix/libstore/platform/linux.hh index 4f6c92a75..29e44315c 100644 --- a/lix/libstore/platform/linux.hh +++ b/lix/libstore/platform/linux.hh @@ -76,6 +76,8 @@ private: Pid startChild( const std::optional & netrcData, const std::optional & caFileData, + const Strings & envStrs, + const Strings & args, AutoCloseFD logPTY ) override;