diff --git a/lix/libstore/build/derivation-goal.cc b/lix/libstore/build/derivation-goal.cc index 7eb93292d..0349b503c 100644 --- a/lix/libstore/build/derivation-goal.cc +++ b/lix/libstore/build/derivation-goal.cc @@ -904,7 +904,8 @@ void runPostBuildHook( proc.getStdout()->drainInto(sink); } -kj::Promise> DerivationGoal::buildDone() noexcept +kj::Promise> DerivationGoal::buildDone(std::shared_ptr remoteError +) noexcept try { trace("build done"); @@ -917,7 +918,21 @@ try { to have terminated. In fact, the builder could also have simply have closed its end of the pipe, so just to be sure, kill it. */ - int status = getChildStatus(); + int rawStatus = getChildStatus(); + const auto [exited, exitCode, exitMsg] = [&]() -> std::tuple { + // override exit status with 1 if we received an exception via rpc for + // historical reasons: the build hook used to turn build errors into a + // log line and an `exit(1)` previously, now it returns the full error + if (remoteError) { + return {true, 1, "failed on remote builder"}; + } else { + if (WIFEXITED(rawStatus)) { + return {true, WEXITSTATUS(rawStatus), statusToString(rawStatus)}; + } else { + return {false, -1, statusToString(rawStatus)}; + } + } + }(); debug("builder process for '%s' finished", worker.store.printStorePath(drvPath)); @@ -933,11 +948,13 @@ try { cleanupPostChildKill(); if (buildResult.cpuUser && buildResult.cpuSystem) { - debug("builder for '%s' terminated with status %d, user CPU %.3fs, system CPU %.3fs", + debug( + "builder for '%s' terminated with status %d, user CPU %.3fs, system CPU %.3fs", worker.store.printStorePath(drvPath), - status, + rawStatus, ((double) buildResult.cpuUser->count()) / 1000000, - ((double) buildResult.cpuSystem->count()) / 1000000); + ((double) buildResult.cpuSystem->count()) / 1000000 + ); } bool diskFull = false; @@ -945,13 +962,12 @@ try { try { /* Check the exit status. */ - if (!statusOk(status)) { + if (!exited || exitCode != 0) { diskFull |= cleanupDecideWhetherDiskFull(); - auto msg = fmt("builder for '%s' %s", - Magenta(worker.store.printStorePath(drvPath)), - statusToString(status)); + auto msg = + fmt("builder for '%s' %s", Magenta(worker.store.printStorePath(drvPath)), exitMsg); if (!logger->isVerbose() && !logTail.empty()) { msg += fmt(";\nlast %d log lines:\n", logTail.size()); @@ -1002,19 +1018,21 @@ try { BuildResult::Status st = BuildResult::MiscFailure; - if (hook && WIFEXITED(status) && WEXITSTATUS(status) == 101) + if (hook && exited && exitCode == 101) { st = BuildResult::TimedOut; - - else if (hook && (!WIFEXITED(status) || WEXITSTATUS(status) != 100)) { } - else { + else if (hook && (!exited || exitCode != 100)) + { + } + + else + { assert(derivationType); - st = - dynamic_cast(&e) ? BuildResult::NotDeterministic : - statusOk(status) ? BuildResult::OutputRejected : - !derivationType->isSandboxed() || diskFull ? BuildResult::TransientFailure : - BuildResult::PermanentFailure; + st = dynamic_cast(&e) ? BuildResult::NotDeterministic + : exited && exitCode == 0 ? BuildResult::OutputRejected + : !derivationType->isSandboxed() || diskFull ? BuildResult::TransientFailure + : BuildResult::PermanentFailure; } co_return done(st, {}, std::move(e)); @@ -1118,7 +1136,7 @@ try { buildResult.startTime = time(0); // inexact started(); - TRY_AWAIT_RPC(runPromise); + auto result = co_await runPromise; // close the rpc connection to have the hook exit hook->rpc = nullptr; @@ -1128,7 +1146,13 @@ try { if (auto error = TRY_AWAIT(output)) { co_return HookResult::Accept{std::move(*error)}; } - co_return HookResult::Accept{TRY_AWAIT(buildDone())}; + + std::shared_ptr remoteError; + if (result.getResult().isBad()) { + remoteError = std::make_shared(from(result.getResult().getBad())); + logger->logEI(remoteError->info()); + } + co_return HookResult::Accept{TRY_AWAIT(buildDone(remoteError))}; } catch (...) { co_return result::current_exception(); } diff --git a/lix/libstore/build/derivation-goal.hh b/lix/libstore/build/derivation-goal.hh index 12c685cf1..c78a1d1a7 100644 --- a/lix/libstore/build/derivation-goal.hh +++ b/lix/libstore/build/derivation-goal.hh @@ -273,7 +273,8 @@ struct DerivationGoal : public Goal kj::Promise> inputsRealised() noexcept; kj::Promise> tryToBuild() noexcept; virtual kj::Promise> tryLocalBuild() noexcept; - kj::Promise> buildDone() noexcept; + kj::Promise> + buildDone(std::shared_ptr remoteError = nullptr) noexcept; /** * Is the build hook willing to perform the build? diff --git a/tests/functional/build-remote.sh b/tests/functional/build-remote.sh index 509978291..653dbf70c 100644 --- a/tests/functional/build-remote.sh +++ b/tests/functional/build-remote.sh @@ -78,3 +78,25 @@ out="$(nix-build 2>&1 failing.nix \ build_dir="$(grep "note: keeping build" <<< "$out" | sed -E "s/^(.*)note: keeping build directory '(.*)'(.*)$/\2/")" [[ "foo" = $(<"$build_dir"/b/bar) ]] + +# regression fj#928: --keep-going doesn't keep going with remote builders +output="$(nix-build 2>&1 \ + --store $TEST_ROOT/machine0 \ + --builders "ssh-ng://localhost?remote-store=$TEST_ROOT/machine3 - - 1 1" \ + --keep-going \ + --max-jobs 0 \ + --expr ' + let + fail = n: derivation { + name = n; + system = builtins.currentSystem; + builder = "/bin/sh"; + args = [ "-c" "false" ]; + }; + in { + a = fail "a"; + b = fail "b"; + } + ' || true)" +grep 'a.drv. failed on remote builder' <<<"$output" +grep 'b.drv. failed on remote builder' <<<"$output"