From 21d7cb8e1c07e834313decc561dceb25a04ebf71 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Tue, 14 Oct 2025 15:31:15 +0200 Subject: [PATCH] libstore: initialize build hooks via rpc this removes another file descriptor from the build hook. we are now using only rpc to communicate between build hook and daemon, setting the stage for a future world in which we don't even need build hooks Change-Id: I706d9d3f2a420abd42fc7762c205931a45d3c8c5 --- lix/legacy/build-remote.cc | 53 ++++++++++++++++++-------- lix/libstore/build/derivation-goal.cc | 2 +- lix/libstore/build/hook-instance.capnp | 5 ++- lix/libstore/build/hook-instance.cc | 29 ++++++++------ lix/libstore/build/hook-instance.hh | 17 ++++++--- lix/libutil/types-rpc.hh | 39 +++++++++++++++++++ lix/libutil/types.capnp | 10 +++++ 7 files changed, 120 insertions(+), 35 deletions(-) diff --git a/lix/legacy/build-remote.cc b/lix/legacy/build-remote.cc index 9fbd3ae8f..2af4542ff 100644 --- a/lix/legacy/build-remote.cc +++ b/lix/legacy/build-remote.cc @@ -6,11 +6,14 @@ #include "lix/libutil/logging.hh" #include "lix/libutil/rpc.hh" #include "lix/libutil/types-rpc.hh" +#include "lix/libutil/types.hh" #include #include #include #include +#include #include +#include #include #include #include @@ -44,8 +47,9 @@ namespace { struct Instance final : rpc::build_remote::HookInstance::Server { unsigned int maxBuildJobs; + bool initialized = false; - Instance(unsigned int maxBuildJobs) : maxBuildJobs(maxBuildJobs) {} + kj::Promise init(InitContext context) override; kj::Promise build(BuildContext context) override; }; @@ -375,30 +379,47 @@ static int main_build_remote(AsyncIoRoot & aio, std::string programName, Strings verbosity = (Verbosity) std::stoll(argv.front()); - FdSource source(STDIN_FILENO); - - /* Read the parent's settings. */ - while (readNum(source)) { - auto name = readString(source); - auto value = readString(source); - settings.set(name, value); - } - - auto maxBuildJobs = settings.maxBuildJobs; - settings.maxBuildJobs.set("1"); // hack to make tests with local?root= work - - initPlugins(); - auto conn = aio.kj.lowLevelProvider->wrapUnixSocketFd(1); - capnp::TwoPartyServer srv(kj::heap(maxBuildJobs)); + capnp::TwoPartyServer srv(kj::heap()); srv.accept(*conn, 1).wait(aio.kj.waitScope); return 0; } } +kj::Promise Instance::init(InitContext context) +{ + try { + if (initialized) { + throw Error("build hook can only be initialized once"); + } + + /* Read the parent's settings. */ + for (const auto & [name, value] : rpc::to(context.getParams().getSettings())) { + settings.set(name, value); + } + + maxBuildJobs = settings.maxBuildJobs; + settings.maxBuildJobs.set("1"); // hack to make tests with local?root= work + + initPlugins(); + + initialized = true; + + context.getResults().initResult().setGood(); + } catch (...) { + RPC_FILL(context.getResults(), initResult, std::current_exception()); + } + + return kj::READY_NOW; +} + kj::Promise Instance::build(BuildContext context) { try { + if (!initialized) { + throw Error("build hook not fully initialized"); + } + // FIXME this does not open a daemon connection for historical reasons. // we may create a lot of build hook instances, and having each of them // also create a daemon instance is inefficient and wasteful. in future diff --git a/lix/libstore/build/derivation-goal.cc b/lix/libstore/build/derivation-goal.cc index f1be57b3b..892dde182 100644 --- a/lix/libstore/build/derivation-goal.cc +++ b/lix/libstore/build/derivation-goal.cc @@ -1118,7 +1118,7 @@ try { // close the rpc connection to have the hook exit hook->rpc = nullptr; - hook->client = std::nullopt; + hook->client = nullptr; hook->conn = nullptr; if (auto error = TRY_AWAIT(output)) { diff --git a/lix/libstore/build/hook-instance.capnp b/lix/libstore/build/hook-instance.capnp index 4e8707d73..cada46b39 100644 --- a/lix/libstore/build/hook-instance.capnp +++ b/lix/libstore/build/hook-instance.capnp @@ -31,7 +31,10 @@ interface HookInstance { } } - build @0 ( + init @0 ( + settings :Types.Settings + ) -> (result :Types.ResultV); + build @1 ( amWilling :Bool, neededSystem :Data, drvPath :StoreTypes.StorePath, diff --git a/lix/libstore/build/hook-instance.cc b/lix/libstore/build/hook-instance.cc index 1c9478642..06551d753 100644 --- a/lix/libstore/build/hook-instance.cc +++ b/lix/libstore/build/hook-instance.cc @@ -4,7 +4,10 @@ #include "lix/libutil/file-system.hh" #include "lix/libstore/globals.hh" #include "lix/libstore/build/hook-instance.hh" +#include "lix/libutil/rpc.hh" #include "lix/libutil/strings.hh" +#include "lix/libutil/types-rpc.hh" // IWYU pragma: keep +#include namespace nix { @@ -33,9 +36,6 @@ try { fromHook_.create(); /* Create the communication pipes. */ - Pipe toHook_; - toHook_.create(); - auto [selfRPC, hookRPC] = SocketPair::stream(); printMsg(lvlChatty, "running build hook: %s", concatMapStringsSep(" ", args, shellEscape)); @@ -50,9 +50,6 @@ try { if (chdir("/") == -1) throw SysError("changing into /"); /* Dup the communication pipes. */ - if (dup2(toHook_.readSide.get(), STDIN_FILENO) == -1) { - throw SysError("dupping to-hook read side"); - } if (dup2(hookRPC.get(), STDOUT_FILENO) == -1) { throw SysError("dupping to-hook read side"); } @@ -66,15 +63,23 @@ try { std::map settings; globalConfig.getSettings(settings, true); - FdSink sink(toHook_.writeSide.get()); - for (auto & setting : settings) { - sink << 1 << setting.first << setting.second.value; + + auto conn = AIO().lowLevelProvider.wrapUnixSocketFd(kj::AutoCloseFd(selfRPC.release())); + auto client = std::make_unique(*conn, 1); + auto rpc = client->bootstrap().castAs(); + + { + auto initReq = rpc.initRequest(); + RPC_FILL(initReq, initSettings, settings); + TRY_AWAIT_RPC(initReq.send()); } - sink << 0; - sink.flush(); co_return std::make_unique( - std::move(fromHook_.readSide), std::move(selfRPC), std::move(pid) + std::move(fromHook_.readSide), + std::move(conn), + std::move(client), + std::move(rpc), + std::move(pid) ); } catch (...) { co_return result::current_exception(); diff --git a/lix/libstore/build/hook-instance.hh b/lix/libstore/build/hook-instance.hh index 40f4990c1..fabc99b39 100644 --- a/lix/libstore/build/hook-instance.hh +++ b/lix/libstore/build/hook-instance.hh @@ -7,6 +7,7 @@ #include "lix/libutil/serialise.hh" #include #include +#include #include namespace nix { @@ -19,7 +20,7 @@ struct HookInstance AutoCloseFD fromHook; kj::Own conn; - std::optional client; + std::unique_ptr client; rpc::build_remote::HookInstance::Client rpc; /** @@ -31,11 +32,17 @@ struct HookInstance static kj::Promise>> create(); - HookInstance(AutoCloseFD fromHook, AutoCloseFD rpc, Pid pid) + HookInstance( + AutoCloseFD fromHook, + kj::Own conn, + std::unique_ptr client, + rpc::build_remote::HookInstance::Client rpc, + Pid pid + ) : fromHook(std::move(fromHook)) - , conn(AIO().lowLevelProvider.wrapUnixSocketFd(kj::AutoCloseFd(rpc.release()))) - , client(std::in_place, *this->conn, 1) - , rpc(client->bootstrap().castAs()) + , conn(std::move(conn)) + , client(std::move(client)) + , rpc(std::move(rpc)) , pid(std::move(pid)) { } diff --git a/lix/libutil/types-rpc.hh b/lix/libutil/types-rpc.hh index b5e331a55..ad5e4e092 100644 --- a/lix/libutil/types-rpc.hh +++ b/lix/libutil/types-rpc.hh @@ -2,6 +2,7 @@ ///@file RPC helper functions for `types.hh` #include "error.hh" +#include "lix/libutil/config.hh" #include "lix/libutil/result.hh" #include "lix/libutil/types.capnp.h" #include "rpc.hh" @@ -127,4 +128,42 @@ struct Fill, std::exception_ptr> detail::makeBadResult(rb, e); } }; + +template<> +struct Fill> +{ + static void fill( + Settings::Setting::Builder sb, + const std::pair & s, + auto &&... args + ) + { + LIX_RPC_FILL(sb, setName, s.first); + LIX_RPC_FILL(sb, setValue, s.second.value); + } +}; + +template<> +struct Fill> +{ + static void fill( + Settings::Builder sb, const std::map & s, auto &&... args + ) + { + LIX_RPC_FILL(sb, initMap, s); + } +}; + +template<> +struct Convert> +{ + static std::map convert(const Settings::Reader & t, auto &&...) + { + std::map result; + for (const auto & s : t.getMap()) { + result[rpc::to(s.getName())] = rpc::to(s.getValue()); + } + return result; + } +}; } diff --git a/lix/libutil/types.capnp b/lix/libutil/types.capnp index 0b25d5eef..ef82cea76 100644 --- a/lix/libutil/types.capnp +++ b/lix/libutil/types.capnp @@ -34,3 +34,13 @@ struct ResultV { bad @1 :Error; } } + +struct Settings { + struct Setting { + name @0 :Data; + value @1 :Data; + } + + # actually a map, but will treat it as a last-value-wins list of pairs for now. + map @0 :List(Setting); +}