From 12f5d27363316df8e04af1e2e376c39588e12057 Mon Sep 17 00:00:00 2001 From: Artemis Tosini Date: Sun, 12 May 2024 21:09:26 +0000 Subject: [PATCH] libstore: Start creating LocalDerivationGoal subclasses LocalDerivationGoal includes a large number of low-level sandboxing primitives for Darwin and Linux, intermingled with ifdefs. Start creating platform-specific classes to make it easier to add new platforms and review platform-specific code. This change only creates support infrastructure and moves two function, more functions will be moved in future changes. Change-Id: I9fc29fa2a7345107d4fc96c46fa90b4eabf6bb89 --- src/libstore/build/local-derivation-goal.cc | 49 ++++----------------- src/libstore/build/local-derivation-goal.hh | 35 ++++++++++++++- src/libstore/build/worker.cc | 8 ++-- src/libstore/platform.cc | 40 +++++++++++++++++ src/libstore/platform/darwin.cc | 26 +++++++++++ src/libstore/platform/darwin.hh | 16 +++++++ src/libstore/platform/fallback.hh | 11 +++++ src/libstore/platform/linux.cc | 14 ++++++ src/libstore/platform/linux.hh | 13 ++++++ 9 files changed, 165 insertions(+), 47 deletions(-) diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index bae821266..4e616c838 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -51,9 +51,6 @@ #endif #if __APPLE__ -#include -#include - /* This definition is undocumented but depended upon by all major browsers. */ extern "C" int sandbox_init_with_parameters(const char *profile, uint64_t flags, const char *const parameters[], char **errorbuf); #endif @@ -161,19 +158,7 @@ void LocalDerivationGoal::killChild() void LocalDerivationGoal::killSandbox(bool getStats) { - if (cgroup) { - #if __linux__ - auto stats = destroyCgroup(*cgroup); - if (getStats) { - buildResult.cpuUser = stats.cpuUser; - buildResult.cpuSystem = stats.cpuSystem; - } - #else - abort(); - #endif - } - - else if (buildUser) { + if (buildUser) { auto uid = buildUser->getUID(); assert(uid != 0); killUser(uid); @@ -2177,31 +2162,8 @@ void LocalDerivationGoal::runChild() } } -#if __APPLE__ - posix_spawnattr_t attrp; - - if (posix_spawnattr_init(&attrp)) - throw SysError("failed to initialize builder"); - - if (posix_spawnattr_setflags(&attrp, POSIX_SPAWN_SETEXEC)) - throw SysError("failed to initialize builder"); - - if (drv->platform == "aarch64-darwin") { - // Unset kern.curproc_arch_affinity so we can escape Rosetta - int affinity = 0; - sysctlbyname("kern.curproc_arch_affinity", NULL, NULL, &affinity, sizeof(affinity)); - - cpu_type_t cpu = CPU_TYPE_ARM64; - posix_spawnattr_setbinpref_np(&attrp, 1, &cpu, NULL); - } else if (drv->platform == "x86_64-darwin") { - cpu_type_t cpu = CPU_TYPE_X86_64; - posix_spawnattr_setbinpref_np(&attrp, 1, &cpu, NULL); - } - - posix_spawn(NULL, drv->builder.c_str(), NULL, &attrp, stringsToCharPtrs(args).data(), stringsToCharPtrs(envStrs).data()); -#else - execve(drv->builder.c_str(), stringsToCharPtrs(args).data(), stringsToCharPtrs(envStrs).data()); -#endif + execBuilder(drv->builder, args, envStrs); + // execBuilder should not return throw SysError("executing '%1%'", drv->builder); @@ -2217,6 +2179,11 @@ void LocalDerivationGoal::runChild() } } +void LocalDerivationGoal::execBuilder(std::string builder, Strings args, Strings envStrs) +{ + execve(builder.c_str(), stringsToCharPtrs(args).data(), stringsToCharPtrs(envStrs).data()); +} + SingleDrvOutputs LocalDerivationGoal::registerOutputs() { diff --git a/src/libstore/build/local-derivation-goal.hh b/src/libstore/build/local-derivation-goal.hh index f3a83d42f..91329ca35 100644 --- a/src/libstore/build/local-derivation-goal.hh +++ b/src/libstore/build/local-derivation-goal.hh @@ -178,7 +178,28 @@ struct LocalDerivationGoal : public DerivationGoal friend struct RestrictedStore; - using DerivationGoal::DerivationGoal; + /** + * Create a LocalDerivationGoal without an on-disk .drv file, + * possibly a platform-specific subclass + */ + static std::shared_ptr makeLocalDerivationGoal( + const StorePath & drvPath, + const OutputsSpec & wantedOutputs, + Worker & worker, + BuildMode buildMode + ); + + /** + * Create a LocalDerivationGoal for an on-disk .drv file, + * possibly a platform-specific subclass + */ + static std::shared_ptr makeLocalDerivationGoal( + const StorePath & drvPath, + const BasicDerivation & drv, + const OutputsSpec & wantedOutputs, + Worker & worker, + BuildMode buildMode + ); virtual ~LocalDerivationGoal() noexcept(false) override; @@ -282,7 +303,7 @@ struct LocalDerivationGoal : public DerivationGoal * Kill any processes running under the build user UID or in the * cgroup of the build. */ - void killSandbox(bool getStats); + virtual void killSandbox(bool getStats); /** * Create alternative path calculated from but distinct from the @@ -299,6 +320,16 @@ struct LocalDerivationGoal : public DerivationGoal * rewrites caught everything */ StorePath makeFallbackPath(OutputNameView outputName); + +protected: + using DerivationGoal::DerivationGoal; + + /** + * Execute the builder, replacing the current process. + * Generally this means an `execve` call. + */ + virtual void execBuilder(std::string builder, Strings args, Strings envStrs); + }; } diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index 04be0da99..a7a298c34 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -65,8 +65,8 @@ std::shared_ptr Worker::makeDerivationGoal(const StorePath & drv { return makeDerivationGoalCommon(drvPath, wantedOutputs, [&]() -> std::shared_ptr { return !dynamic_cast(&store) - ? std::make_shared(drvPath, wantedOutputs, *this, buildMode) - : std::make_shared(drvPath, wantedOutputs, *this, buildMode); + ? std::make_shared(drvPath, wantedOutputs, *this, buildMode) + : LocalDerivationGoal::makeLocalDerivationGoal(drvPath, wantedOutputs, *this, buildMode); }); } @@ -76,8 +76,8 @@ std::shared_ptr Worker::makeBasicDerivationGoal(const StorePath { return makeDerivationGoalCommon(drvPath, wantedOutputs, [&]() -> std::shared_ptr { return !dynamic_cast(&store) - ? std::make_shared(drvPath, drv, wantedOutputs, *this, buildMode) - : std::make_shared(drvPath, drv, wantedOutputs, *this, buildMode); + ? std::make_shared(drvPath, drv, wantedOutputs, *this, buildMode) + : LocalDerivationGoal::makeLocalDerivationGoal(drvPath, drv, wantedOutputs, *this, buildMode); }); } diff --git a/src/libstore/platform.cc b/src/libstore/platform.cc index acdedab99..d10d33f0e 100644 --- a/src/libstore/platform.cc +++ b/src/libstore/platform.cc @@ -1,4 +1,5 @@ #include "local-store.hh" +#include "build/local-derivation-goal.hh" #if __linux__ #include "platform/linux.hh" @@ -19,4 +20,43 @@ std::shared_ptr LocalStore::makeLocalStore(const Params & params) return std::shared_ptr(new FallbackLocalStore(params)); #endif } + +std::shared_ptr LocalDerivationGoal::makeLocalDerivationGoal( + const StorePath & drvPath, + const OutputsSpec & wantedOutputs, + Worker & worker, + BuildMode buildMode +) +{ +#if __linux__ + return std::make_shared(drvPath, wantedOutputs, worker, buildMode); +#elif __APPLE__ + return std::make_shared(drvPath, wantedOutputs, worker, buildMode); +#else + return std::make_shared(drvPath, wantedOutputs, worker, buildMode); +#endif +} + +std::shared_ptr LocalDerivationGoal::makeLocalDerivationGoal( + const StorePath & drvPath, + const BasicDerivation & drv, + const OutputsSpec & wantedOutputs, + Worker & worker, + BuildMode buildMode +) +{ +#if __linux__ + return std::make_shared( + drvPath, drv, wantedOutputs, worker, buildMode + ); +#elif __APPLE__ + return std::make_shared( + drvPath, drv, wantedOutputs, worker, buildMode + ); +#else + return std::make_shared( + drvPath, drv, wantedOutputs, worker, buildMode + ); +#endif +} } diff --git a/src/libstore/platform/darwin.cc b/src/libstore/platform/darwin.cc index bbb81784c..83b4b4183 100644 --- a/src/libstore/platform/darwin.cc +++ b/src/libstore/platform/darwin.cc @@ -6,6 +6,7 @@ #include #include #include +#include #include @@ -220,4 +221,29 @@ void DarwinLocalStore::findPlatformRoots(UncheckedRoots & unchecked) } } } + +void DarwinLocalDerivationGoal::execBuilder(std::string builder, Strings args, Strings envStrs) +{ + posix_spawnattr_t attrp; + + if (posix_spawnattr_init(&attrp)) + throw SysError("failed to initialize builder"); + + if (posix_spawnattr_setflags(&attrp, POSIX_SPAWN_SETEXEC)) + throw SysError("failed to initialize builder"); + + if (drv->platform == "aarch64-darwin") { + // Unset kern.curproc_arch_affinity so we can escape Rosetta + int affinity = 0; + sysctlbyname("kern.curproc_arch_affinity", NULL, NULL, &affinity, sizeof(affinity)); + + cpu_type_t cpu = CPU_TYPE_ARM64; + posix_spawnattr_setbinpref_np(&attrp, 1, &cpu, NULL); + } else if (drv->platform == "x86_64-darwin") { + cpu_type_t cpu = CPU_TYPE_X86_64; + posix_spawnattr_setbinpref_np(&attrp, 1, &cpu, NULL); + } + + posix_spawn(NULL, builder.c_str(), NULL, &attrp, stringsToCharPtrs(args).data(), stringsToCharPtrs(envStrs).data()); +} } diff --git a/src/libstore/platform/darwin.hh b/src/libstore/platform/darwin.hh index b7170aa05..0ac7077fb 100644 --- a/src/libstore/platform/darwin.hh +++ b/src/libstore/platform/darwin.hh @@ -1,6 +1,7 @@ #pragma once ///@file +#include "build/local-derivation-goal.hh" #include "gc-store.hh" #include "local-store.hh" @@ -32,4 +33,19 @@ private: void findPlatformRoots(UncheckedRoots & unchecked) override; }; +/** + * Darwin-specific implementation of LocalDerivationGoal + */ +class DarwinLocalDerivationGoal : public LocalDerivationGoal +{ +public: + using LocalDerivationGoal::LocalDerivationGoal; + +private: + /** + * Set process flags to enter or leave rosetta, then execute the builder + */ + void execBuilder(std::string builder, Strings args, Strings envStrs) override; +}; + } diff --git a/src/libstore/platform/fallback.hh b/src/libstore/platform/fallback.hh index fd27edbe6..3b77536bb 100644 --- a/src/libstore/platform/fallback.hh +++ b/src/libstore/platform/fallback.hh @@ -1,6 +1,7 @@ #pragma once ///@file +#include "build/local-derivation-goal.hh" #include "local-store.hh" namespace nix { @@ -28,4 +29,14 @@ public: } }; +/** + * Fallback platform implementation of LocalDerivationGoal + * Exists so we can make LocalDerivationGoal constructor protected + */ +class FallbackLocalDerivationGoal : public LocalDerivationGoal +{ +public: + using LocalDerivationGoal::LocalDerivationGoal; +}; + } diff --git a/src/libstore/platform/linux.cc b/src/libstore/platform/linux.cc index a34608894..6b94c01cc 100644 --- a/src/libstore/platform/linux.cc +++ b/src/libstore/platform/linux.cc @@ -1,3 +1,4 @@ +#include "cgroup.hh" #include "gc-store.hh" #include "signals.hh" #include "platform/linux.hh" @@ -114,4 +115,17 @@ void LinuxLocalStore::findPlatformRoots(UncheckedRoots & unchecked) readFileRoots("/proc/sys/kernel/fbsplash", unchecked); readFileRoots("/proc/sys/kernel/poweroff_cmd", unchecked); } + +void LinuxLocalDerivationGoal::killSandbox(bool getStats) +{ + if (cgroup) { + auto stats = destroyCgroup(*cgroup); + if (getStats) { + buildResult.cpuUser = stats.cpuUser; + buildResult.cpuSystem = stats.cpuSystem; + } + } else { + LocalDerivationGoal::killSandbox(getStats); + } +} } diff --git a/src/libstore/platform/linux.hh b/src/libstore/platform/linux.hh index 8b97e17c5..2cad001ea 100644 --- a/src/libstore/platform/linux.hh +++ b/src/libstore/platform/linux.hh @@ -1,6 +1,7 @@ #pragma once ///@file +#include "build/local-derivation-goal.hh" #include "gc-store.hh" #include "local-store.hh" @@ -32,4 +33,16 @@ private: void findPlatformRoots(UncheckedRoots & unchecked) override; }; +/** + * Linux-specific implementation of LocalDerivationGoal + */ +class LinuxLocalDerivationGoal : public LocalDerivationGoal +{ +public: + using LocalDerivationGoal::LocalDerivationGoal; + +private: + void killSandbox(bool getStatus) override; +}; + }