-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
External derivation builders #14145
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
External derivation builders #14145
Changes from all commits
584ef0f
73e4c40
6c0d677
68bd2e4
e9c5d72
e7e2ac9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1372,6 +1372,77 @@ public: | |
Default is 0, which disables the warning. | ||
Set it to 1 to warn on all paths. | ||
)"}; | ||
|
||
struct ExternalBuilder | ||
{ | ||
std::vector<std::string> systems; | ||
Path program; | ||
std::vector<std::string> args; | ||
}; | ||
|
||
using ExternalBuilders = std::vector<ExternalBuilder>; | ||
|
||
Setting<ExternalBuilders> externalBuilders{ | ||
this, | ||
{}, | ||
"external-builders", | ||
R"( | ||
Helper programs that execute derivations. | ||
|
||
The program is passed a JSON document that describes the build environment as the final argument. | ||
The JSON document looks like this: | ||
|
||
{ | ||
"args": [ | ||
"-e", | ||
"/nix/store/vj1c3wf9…-source-stdenv.sh", | ||
"/nix/store/shkw4qm9…-default-builder.sh" | ||
], | ||
"builder": "/nix/store/s1qkj0ph…-bash-5.2p37/bin/bash", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The following are not needed?
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It shouldn't need those, but I did add the input closure and the (scratch) output paths so the external builder knows exactly what paths it may need to copy/mount into/out of its build environment. |
||
"env": { | ||
"HOME": "/homeless-shelter", | ||
"builder": "/nix/store/s1qkj0ph…-bash-5.2p37/bin/bash", | ||
"nativeBuildInputs": "/nix/store/l31j72f1…-version-check-hook", | ||
"out": "/nix/store/2yx2prgx…-hello-2.12.2" | ||
… | ||
}, | ||
"inputPaths": [ | ||
"/nix/store/14dciax3…-glibc-2.32-54-dev", | ||
"/nix/store/1azs5s8z…-gettext-0.21", | ||
… | ||
], | ||
"outputs": { | ||
"out": "/nix/store/2yx2prgx…-hello-2.12.2" | ||
}, | ||
"realStoreDir": "/nix/store", | ||
"storeDir": "/nix/store", | ||
"system": "aarch64-linux", | ||
"tmpDir": "/private/tmp/nix-build-hello-2.12.2.drv-0/build", | ||
"tmpDirInSandbox": "/build", | ||
"topTmpDir": "/private/tmp/nix-build-hello-2.12.2.drv-0", | ||
"version": 1 | ||
} | ||
)", | ||
{}, // aliases | ||
true, // document default | ||
// NOTE(cole-h): even though we can make the experimental feature required here, the errors | ||
// are not as good (it just becomes a warning if you try to use this setting without the | ||
// experimental feature) | ||
// | ||
// With this commented out: | ||
// | ||
// error: experimental Nix feature 'external-builders' is disabled; add '--extra-experimental-features | ||
// external-builders' to enable it | ||
// | ||
// With this uncommented: | ||
// | ||
// warning: Ignoring setting 'external-builders' because experimental feature 'external-builders' is not enabled | ||
// error: Cannot build '/nix/store/vwsp4qd8…-opentofu-1.10.2.drv'. | ||
// Reason: required system or feature not available | ||
// Required system: 'aarch64-linux' with features {} | ||
// Current system: 'aarch64-darwin' with features {apple-virt, benchmark, big-parallel, nixos-test} | ||
// Xp::ExternalBuilders | ||
}; | ||
}; | ||
|
||
// FIXME: don't use a global variable. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -229,6 +229,12 @@ class DerivationBuilderImpl : public DerivationBuilder, public DerivationBuilder | |
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: | ||
|
@@ -666,21 +672,8 @@ static bool checkNotWorldWritable(std::filesystem::path path) | |
return true; | ||
} | ||
|
||
std::optional<Descriptor> DerivationBuilderImpl::startBuild() | ||
void DerivationBuilderImpl::checkSystem() | ||
{ | ||
if (useBuildUsers()) { | ||
if (!buildUser) | ||
buildUser = getBuildUser(); | ||
|
||
if (!buildUser) | ||
return std::nullopt; | ||
} | ||
|
||
/* Make sure that no other processes are executing under the | ||
sandbox uids. This must be done before any chownToBuilder() | ||
calls. */ | ||
prepareUser(); | ||
|
||
/* Right platform? */ | ||
if (!drvOptions.canBuildLocally(store, drv)) { | ||
auto msg = | ||
|
@@ -704,6 +697,24 @@ std::optional<Descriptor> DerivationBuilderImpl::startBuild() | |
|
||
throw BuildError(BuildResult::Failure::InputRejected, msg); | ||
} | ||
} | ||
|
||
std::optional<Descriptor> DerivationBuilderImpl::startBuild() | ||
{ | ||
if (useBuildUsers()) { | ||
if (!buildUser) | ||
buildUser = getBuildUser(); | ||
|
||
if (!buildUser) | ||
return std::nullopt; | ||
} | ||
|
||
checkSystem(); | ||
|
||
/* Make sure that no other processes are executing under the | ||
sandbox uids. This must be done before any chownToBuilder() | ||
calls. */ | ||
prepareUser(); | ||
|
||
auto buildDir = store.config->getBuildDir(); | ||
|
||
|
@@ -1904,12 +1915,16 @@ StorePath DerivationBuilderImpl::makeFallbackPath(const StorePath & path) | |
#include "chroot-derivation-builder.cc" | ||
#include "linux-derivation-builder.cc" | ||
#include "darwin-derivation-builder.cc" | ||
#include "external-derivation-builder.cc" | ||
|
||
namespace nix { | ||
|
||
std::unique_ptr<DerivationBuilder> makeDerivationBuilder( | ||
LocalStore & store, std::unique_ptr<DerivationBuilderCallbacks> miscMethods, DerivationBuilderParams params) | ||
{ | ||
if (auto builder = ExternalDerivationBuilder::newIfSupported(store, miscMethods, params)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this filter out builtin derivations? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Currently it doesn't. But that's the same for the build hook IIRC. |
||
return builder; | ||
|
||
bool useSandbox = false; | ||
|
||
/* Are we doing a sandboxed build? */ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,123 @@ | ||
namespace nix { | ||
|
||
struct ExternalDerivationBuilder : DerivationBuilderImpl | ||
{ | ||
Settings::ExternalBuilder externalBuilder; | ||
|
||
ExternalDerivationBuilder( | ||
LocalStore & store, | ||
std::unique_ptr<DerivationBuilderCallbacks> miscMethods, | ||
DerivationBuilderParams params, | ||
Settings::ExternalBuilder externalBuilder) | ||
: DerivationBuilderImpl(store, std::move(miscMethods), std::move(params)) | ||
, externalBuilder(std::move(externalBuilder)) | ||
{ | ||
experimentalFeatureSettings.require(Xp::ExternalBuilders); | ||
} | ||
|
||
static std::unique_ptr<ExternalDerivationBuilder> newIfSupported( | ||
LocalStore & store, std::unique_ptr<DerivationBuilderCallbacks> & miscMethods, DerivationBuilderParams & params) | ||
{ | ||
for (auto & handler : settings.externalBuilders.get()) { | ||
for (auto & system : handler.systems) | ||
if (params.drv.platform == system) | ||
return std::make_unique<ExternalDerivationBuilder>( | ||
store, std::move(miscMethods), std::move(params), handler); | ||
} | ||
return {}; | ||
} | ||
|
||
Path tmpDirInSandbox() override | ||
{ | ||
/* In a sandbox, for determinism, always use the same temporary | ||
directory. */ | ||
return "/build"; | ||
} | ||
|
||
void setBuildTmpDir() override | ||
{ | ||
tmpDir = topTmpDir + "/build"; | ||
createDir(tmpDir, 0700); | ||
} | ||
|
||
void checkSystem() override {} | ||
|
||
void startChild() override | ||
{ | ||
if (drvOptions.getRequiredSystemFeatures(drv).count("recursive-nix")) | ||
throw Error("'recursive-nix' is not supported yet by external derivation builders"); | ||
|
||
auto json = nlohmann::json::object(); | ||
|
||
json.emplace("version", 1); | ||
json.emplace("builder", drv.builder); | ||
{ | ||
auto l = nlohmann::json::array(); | ||
for (auto & i : drv.args) | ||
l.push_back(rewriteStrings(i, inputRewrites)); | ||
json.emplace("args", std::move(l)); | ||
} | ||
{ | ||
auto j = nlohmann::json::object(); | ||
for (auto & [name, value] : env) | ||
j.emplace(name, rewriteStrings(value, inputRewrites)); | ||
json.emplace("env", std::move(j)); | ||
} | ||
xokdvium marked this conversation as resolved.
Show resolved
Hide resolved
|
||
json.emplace("topTmpDir", topTmpDir); | ||
json.emplace("tmpDir", tmpDir); | ||
json.emplace("tmpDirInSandbox", tmpDirInSandbox()); | ||
json.emplace("storeDir", store.storeDir); | ||
json.emplace("realStoreDir", store.config->realStoreDir.get()); | ||
json.emplace("system", drv.platform); | ||
{ | ||
auto l = nlohmann::json::array(); | ||
for (auto & i : inputPaths) | ||
l.push_back(store.printStorePath(i)); | ||
json.emplace("inputPaths", std::move(l)); | ||
} | ||
{ | ||
auto l = nlohmann::json::object(); | ||
for (auto & i : scratchOutputs) | ||
l.emplace(i.first, store.printStorePath(i.second)); | ||
json.emplace("outputs", std::move(l)); | ||
} | ||
|
||
// TODO(cole-h): writing this to stdin is too much effort right now, if we want to revisit | ||
// that, see this comment by Eelco about how to make it not suck: | ||
// https://github.com/DeterminateSystems/nix-src/pull/141#discussion_r2205493257 | ||
auto jsonFile = std::filesystem::path{topTmpDir} / "build.json"; | ||
writeFile(jsonFile, json.dump()); | ||
|
||
pid = startProcess([&]() { | ||
openSlave(); | ||
try { | ||
commonChildInit(); | ||
|
||
Strings args = {externalBuilder.program}; | ||
xokdvium marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
if (!externalBuilder.args.empty()) { | ||
args.insert(args.end(), externalBuilder.args.begin(), externalBuilder.args.end()); | ||
} | ||
|
||
args.insert(args.end(), jsonFile); | ||
|
||
if (chdir(tmpDir.c_str()) == -1) | ||
throw SysError("changing into '%1%'", tmpDir); | ||
|
||
chownToBuilder(topTmpDir); | ||
|
||
setUser(); | ||
|
||
debug("executing external builder: %s", concatStringsSep(" ", args)); | ||
execv(externalBuilder.program.c_str(), stringsToCharPtrs(args).data()); | ||
|
||
throw SysError("executing '%s'", externalBuilder.program); | ||
} catch (...) { | ||
handleChildException(true); | ||
_exit(1); | ||
} | ||
}); | ||
} | ||
}; | ||
|
||
} // namespace nix |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
#!/usr/bin/env bash | ||
|
||
source common.sh | ||
|
||
TODO_NixOS | ||
|
||
needLocalStore "'--external-builders' can’t be used with the daemon" | ||
|
||
expr="$TEST_ROOT/expr.nix" | ||
cat > "$expr" <<EOF | ||
with import ${config_nix}; | ||
mkDerivation { | ||
name = "external"; | ||
system = "x68_46-xunil"; | ||
buildCommand = '' | ||
echo xyzzy | ||
printf foo > \$out | ||
''; | ||
} | ||
EOF | ||
|
||
external_builder="$TEST_ROOT/external-builder.sh" | ||
cat > "$external_builder" <<EOF | ||
#! $SHELL -e | ||
|
||
PATH=$PATH | ||
|
||
[[ "\$1" = bla ]] | ||
|
||
system="\$(jq -r .system < "\$2")" | ||
builder="\$(jq -r .builder < "\$2")" | ||
args="\$(jq -r '.args | join(" ")' < "\$2")" | ||
export buildCommand="\$(jq -r .env.buildCommand < "\$2")" | ||
export out="\$(jq -r .env.out < "\$2")" | ||
[[ \$system = x68_46-xunil ]] | ||
|
||
printf "\2\n" | ||
|
||
# In a real external builder, we would now call something like qemu to emulate the system. | ||
"\$builder" \$args | ||
|
||
printf bar >> \$out | ||
EOF | ||
chmod +x "$external_builder" | ||
|
||
nix build -L --file "$expr" --out-link "$TEST_ROOT/result" \ | ||
--extra-experimental-features external-builders \ | ||
--external-builders "[{\"systems\": [\"x68_46-xunil\"], \"args\": [\"bla\"], \"program\": \"$external_builder\"}]" | ||
|
||
[[ $(cat "$TEST_ROOT/result") = foobar ]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a privileged nix option, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, every setting is privileged unless it's specifically exempted in
daemon.cc
.