From 37fbfffd8e23cf9ca4038e6c4145583a811e91aa Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 23 Oct 2017 20:43:04 +0200 Subject: [PATCH] Pass all settings to build-remote This ensures that command line flags such as --builders get passed correctly. --- src/build-remote/build-remote.cc | 59 +++++++++++++++++--------------- src/libstore/build.cc | 28 +++++++++------ src/libutil/serialise.hh | 22 +++++++++++- tests/build-hook.hook.sh | 23 ------------- tests/build-hook.sh | 10 ------ tests/build-remote.sh | 13 +++---- tests/local.mk | 2 +- 7 files changed, 75 insertions(+), 82 deletions(-) delete mode 100755 tests/build-hook.hook.sh delete mode 100644 tests/build-hook.sh diff --git a/src/build-remote/build-remote.cc b/src/build-remote/build-remote.cc index ea002cdcc..f023aedb1 100644 --- a/src/build-remote/build-remote.cc +++ b/src/build-remote/build-remote.cc @@ -46,16 +46,23 @@ int main (int argc, char * * argv) unsetenv("DISPLAY"); unsetenv("SSH_ASKPASS"); - if (argc != 6) + if (argc != 2) throw UsageError("called without required arguments"); - auto store = openStore().cast(); + verbosity = (Verbosity) std::stoll(argv[1]); - auto localSystem = argv[1]; - settings.maxSilentTime = std::stoll(argv[2]); - settings.buildTimeout = std::stoll(argv[3]); - verbosity = (Verbosity) std::stoll(argv[4]); - settings.builders = argv[5]; + FdSource source(STDIN_FILENO); + + /* Read the parent's settings. */ + while (readInt(source)) { + auto name = readString(source); + auto value = readString(source); + settings.set(name, value); + } + + settings.maxBuildJobs.set("1"); // hack to make tests with local?root= work + + auto store = openStore().cast(); /* It would be more appropriate to use $XDG_RUNTIME_DIR, since that gets cleared on reboot, but it wouldn't work on macOS. */ @@ -74,18 +81,20 @@ int main (int argc, char * * argv) string drvPath; string storeUri; - for (string line; getline(cin, line);) { - auto tokens = tokenizeString>(line); - auto sz = tokens.size(); - if (sz != 3 && sz != 4) - throw Error("invalid build hook line '%1%'", line); - auto amWilling = tokens[0] == "1"; - auto neededSystem = tokens[1]; - drvPath = tokens[2]; - auto requiredFeatures = sz == 3 ? - std::set{} : - tokenizeString>(tokens[3], ","); - auto canBuildLocally = amWilling && (neededSystem == localSystem); + + while (true) { + + try { + auto s = readString(source); + if (s != "try") return; + } catch (EndOfFile &) { return; } + + auto amWilling = readInt(source); + auto neededSystem = readString(source); + source >> drvPath; + auto requiredFeatures = readStrings>(source); + + auto canBuildLocally = amWilling && (neededSystem == settings.thisSystem); /* Error ignored here, will be caught later */ mkdir(currentLoad.c_str(), 0777); @@ -100,7 +109,7 @@ int main (int argc, char * * argv) Machine * bestMachine = nullptr; unsigned long long bestLoad = 0; for (auto & m : machines) { - debug("considering building on '%s'", m.storeUri); + debug("considering building on remote machine '%s'", m.storeUri); if (m.enabled && std::find(m.systemTypes.begin(), m.systemTypes.end(), @@ -184,15 +193,9 @@ int main (int argc, char * * argv) connected: std::cerr << "# accept\n"; - string line; - if (!getline(cin, line)) - throw Error("hook caller didn't send inputs"); - auto inputs = tokenizeString(line); - if (!getline(cin, line)) - throw Error("hook caller didn't send outputs"); - - auto outputs = tokenizeString(line); + auto inputs = readStrings(source); + auto outputs = readStrings(source); AutoCloseFD uploadLock = openLockFile(currentLoad + "/" + escapeUri(storeUri) + ".upload-lock", true); diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 88c516546..057ad2bdf 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -606,6 +606,8 @@ struct HookInstance /* The process ID of the hook. */ Pid pid; + FdSink sink; + HookInstance(); ~HookInstance(); @@ -642,11 +644,7 @@ HookInstance::HookInstance() Strings args = { baseNameOf(settings.buildHook), - settings.thisSystem, - std::to_string(settings.maxSilentTime), - std::to_string(settings.buildTimeout), std::to_string(verbosity), - settings.builders }; execv(settings.buildHook.get().c_str(), stringsToCharPtrs(args).data()); @@ -657,6 +655,11 @@ HookInstance::HookInstance() pid.setSeparatePG(true); fromHook.writeSide = -1; toHook.readSide = -1; + + sink = FdSink(toHook.writeSide.get()); + for (auto & setting : settings.getSettings()) + sink << 1 << setting.first << setting.second; + sink << 0; } @@ -1633,9 +1636,13 @@ HookReply DerivationGoal::tryBuildHook() for (auto & i : features) checkStoreName(i); /* !!! abuse */ /* Send the request to the hook. */ - writeLine(worker.hook->toHook.writeSide.get(), (format("%1% %2% %3% %4%") - % (worker.getNrLocalBuilds() < settings.maxBuildJobs ? "1" : "0") - % drv->platform % drvPath % concatStringsSep(",", features)).str()); + worker.hook->sink + << "try" + << (worker.getNrLocalBuilds() < settings.maxBuildJobs ? 1 : 0) + << drv->platform + << drvPath + << features; + worker.hook->sink.flush(); /* Read the first line of input, which should be a word indicating whether the hook wishes to perform the build. */ @@ -1680,12 +1687,13 @@ HookReply DerivationGoal::tryBuildHook() /* Tell the hook all the inputs that have to be copied to the remote system. */ - writeLine(hook->toHook.writeSide.get(), concatStringsSep(" ", inputPaths)); + hook->sink << inputPaths; /* Tell the hooks the missing outputs that have to be copied back from the remote system. */ - writeLine(hook->toHook.writeSide.get(), concatStringsSep(" ", missingPaths)); + hook->sink << missingPaths; + hook->sink = FdSink(); hook->toHook.writeSide = -1; /* Create the log file and pipe. */ @@ -3986,7 +3994,7 @@ void Worker::run(const Goals & _topGoals) else { if (awake.empty() && 0 == settings.maxBuildJobs) throw Error( "unable to start any build; either increase '--max-jobs' " - "or enable distributed builds"); + "or enable remote builds"); assert(!awake.empty()); } } diff --git a/src/libutil/serialise.hh b/src/libutil/serialise.hh index 70b193941..2ea5b6354 100644 --- a/src/libutil/serialise.hh +++ b/src/libutil/serialise.hh @@ -92,7 +92,17 @@ struct FdSink : BufferedSink FdSink() : fd(-1) { } FdSink(int fd) : fd(fd) { } FdSink(FdSink&&) = default; - FdSink& operator=(FdSink&&) = default; + + FdSink& operator=(FdSink && s) + { + flush(); + fd = s.fd; + s.fd = -1; + warn = s.warn; + written = s.written; + return *this; + } + ~FdSink(); void write(const unsigned char * data, size_t len) override; @@ -112,6 +122,16 @@ struct FdSource : BufferedSource FdSource() : fd(-1) { } FdSource(int fd) : fd(fd) { } + FdSource(FdSource&&) = default; + + FdSource& operator=(FdSource && s) + { + fd = s.fd; + s.fd = -1; + read = s.read; + return *this; + } + size_t readUnbuffered(unsigned char * data, size_t len) override; bool good() override; private: diff --git a/tests/build-hook.hook.sh b/tests/build-hook.hook.sh deleted file mode 100755 index c7472eab7..000000000 --- a/tests/build-hook.hook.sh +++ /dev/null @@ -1,23 +0,0 @@ -#! /bin/sh - -#set -x - -while read x y drv rest; do - - echo "HOOK for $drv" >&2 - - outPath=`sed 's/Derive(\[("out",\"\([^\"]*\)\".*/\1/' $drv` - - echo "output path is $outPath" >&2 - - if `echo $outPath | grep -q input-1`; then - echo "# accept" >&2 - read inputs - read outputs - mkdir $outPath - echo "BAR" > $outPath/foo - else - echo "# decline" >&2 - fi - -done diff --git a/tests/build-hook.sh b/tests/build-hook.sh deleted file mode 100644 index 2005c7ceb..000000000 --- a/tests/build-hook.sh +++ /dev/null @@ -1,10 +0,0 @@ -source common.sh - -clearStore - -outPath=$(nix-build build-hook.nix --no-out-link --option build-hook $(pwd)/build-hook.hook.sh) - -echo "output path is $outPath" - -text=$(cat "$outPath"/foobar) -if test "$text" != "BARBAR"; then exit 1; fi diff --git a/tests/build-remote.sh b/tests/build-remote.sh index e27ce7e25..603b57c59 100644 --- a/tests/build-remote.sh +++ b/tests/build-remote.sh @@ -9,16 +9,11 @@ chmod -R u+w $TEST_ROOT/store0 || true chmod -R u+w $TEST_ROOT/store1 || true rm -rf $TEST_ROOT/store0 $TEST_ROOT/store1 -# FIXME: --option is not passed to build-remote, so have to create a config file. -export NIX_CONF_DIR=$TEST_ROOT/etc2 -mkdir -p $NIX_CONF_DIR -echo " -sandbox-paths = /nix/store -sandbox-build-dir = /build-tmp -" > $NIX_CONF_DIR/nix.conf +nix build -f build-hook.nix -o $TEST_ROOT/result --max-jobs 0 \ + --sandbox-paths /nix/store --sandbox-build-dir /build-tmp \ + --builders "local?root=$TEST_ROOT/store0; local?root=$TEST_ROOT/store1 - - 1 1 foo" -outPath=$(nix-build build-hook.nix --no-out-link -j0 \ - --option builders "local?root=$TEST_ROOT/store0; local?root=$TEST_ROOT/store1 - - 1 1 foo") +outPath=$TEST_ROOT/result cat $outPath/foobar | grep FOOBAR diff --git a/tests/local.mk b/tests/local.mk index 7d99a0fc7..19d6f1893 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -3,7 +3,7 @@ check: nix_tests = \ init.sh hash.sh lang.sh add.sh simple.sh dependencies.sh \ - build-hook.sh gc.sh gc-concurrent.sh \ + gc.sh gc-concurrent.sh \ referrers.sh user-envs.sh logging.sh nix-build.sh misc.sh fixed.sh \ gc-runtime.sh check-refs.sh filter-source.sh \ remote-store.sh export.sh export-graph.sh \