From 4c04b6c62a5eaed3678b1c06b6a61740bd0d787e Mon Sep 17 00:00:00 2001 From: Amaan Qureshi Date: Thu, 5 Feb 2026 17:24:48 -0500 Subject: [PATCH] worker-protocol: add cancelled-and-hash-mismatch-status feature This commit adds a `cancelled-and-hash-mismatch-status` protocol feature that gates two new `BuildResult` failure statuses: `HashMismatch` (a specific type of `OutputRejected` for hash mismatches) and `Cancelled` (for goals never attempted because another goal failed without `--keep-going`). Goals now default to `Cancelled` status instead of `MiscFailure`. When communicating with older remotes, `HashMismatch` falls back to `OutputRejected` and `Cancelled` falls back to `MiscFailure`. Co-authored-by: Eelco Dolstra --- src/libcmd/installables.cc | 7 +- src/libstore/build-result.cc | 13 +++- src/libstore/common-protocol.cc | 54 ------------- src/libstore/daemon.cc | 5 +- .../include/nix/store/build-result.hh | 24 ++++-- src/libstore/include/nix/store/build/goal.hh | 2 +- .../include/nix/store/common-protocol.hh | 3 - .../include/nix/store/serve-protocol.hh | 2 + .../include/nix/store/worker-protocol.hh | 9 +++ src/libstore/legacy-ssh-store.cc | 2 +- src/libstore/serve-protocol.cc | 72 +++++++++++++++++- src/libstore/worker-protocol-connection.cc | 7 +- src/libstore/worker-protocol.cc | 76 ++++++++++++++++++- src/nix/flake.cc | 13 +++- tests/functional/build.sh | 69 +++++++++-------- tests/functional/check-refs.sh | 8 +- tests/functional/common/functions.sh | 2 +- tests/functional/dyn-drv/failing-outer.sh | 2 +- tests/functional/fetchurl.sh | 2 +- tests/functional/timeout.sh | 2 +- 20 files changed, 254 insertions(+), 120 deletions(-) diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index b9366a27590..77d1145e5e8 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -586,13 +586,18 @@ static void throwBuildErrors(std::vector & buildResults, const throw *failedResult->second; } else { StringSet failedPaths; + /* Compute aggregate exit status from all failures */ + ExitStatusFlags flags{}; for (; failedResult != failed.end(); failedResult++) { if (!failedResult->second->message().empty()) { logError(failedResult->second->info()); } failedPaths.insert(failedResult->first->path.to_string(store)); + auto status = failedResult->second->status; + flags.updateFromStatus(status); } - throw Error("build of %s failed", concatStringsSep(", ", quoteStrings(failedPaths))); + auto exitStatus = flags.failingExitStatus(); + throw Error(exitStatus, "build of %s failed", concatStringsSep(", ", quoteStrings(failedPaths))); } } } diff --git a/src/libstore/build-result.cc b/src/libstore/build-result.cc index f01911bcd00..b10f1e06b23 100644 --- a/src/libstore/build-result.cc +++ b/src/libstore/build-result.cc @@ -4,6 +4,13 @@ namespace nix { +unsigned int BuildError::exitCodeFromStatus(Status status) +{ + ExitStatusFlags flags{}; + flags.updateFromStatus(status); + return flags.failingExitStatus(); +} + void ExitStatusFlags::updateFromStatus(BuildResult::Failure::Status status) { // Allow selecting a subset of enum values @@ -19,8 +26,9 @@ void ExitStatusFlags::updateFromStatus(BuildResult::Failure::Status status) case BuildResult::Failure::NotDeterministic: checkMismatch = true; break; + case BuildResult::Failure::OutputRejected: case BuildResult::Failure::PermanentFailure: - // Also considered a permenant failure, it seems + // Also considered a permanent failure, it seems case BuildResult::Failure::InputRejected: permanentFailure = true; break; @@ -90,7 +98,7 @@ static BuildResult::Success::Status successStatusFromString(std::string_view str throw Error("unknown built result success status '%s'", str); } -static constexpr std::array, 12> failureStatusStrings{{ +static constexpr std::array, 13> failureStatusStrings{{ #define ENUM_ENTRY(e) {BuildResult::Failure::e, #e} ENUM_ENTRY(PermanentFailure), ENUM_ENTRY(InputRejected), @@ -104,6 +112,7 @@ static constexpr std::array::write( conn.to << sig.to_string(); } -/** - * Mapping from protocol wire values to BuildResultStatus. - * - * The array index is the wire value. - * Note: HashMismatch is not in the protocol; it gets converted - * to OutputRejected before serialization. - */ -constexpr static BuildResultStatus buildResultStatusTable[] = { - BuildResultSuccessStatus::Built, // 0 - BuildResultSuccessStatus::Substituted, // 1 - BuildResultSuccessStatus::AlreadyValid, // 2 - BuildResultFailureStatus::PermanentFailure, // 3 - BuildResultFailureStatus::InputRejected, // 4 - BuildResultFailureStatus::OutputRejected, // 5 - BuildResultFailureStatus::TransientFailure, // 6 - BuildResultFailureStatus::CachedFailure, // 7 - BuildResultFailureStatus::TimedOut, // 8 - BuildResultFailureStatus::MiscFailure, // 9 - BuildResultFailureStatus::DependencyFailed, // 10 - BuildResultFailureStatus::LogLimitExceeded, // 11 - BuildResultFailureStatus::NotDeterministic, // 12 - BuildResultSuccessStatus::ResolvesToAlreadyValid, // 13 - BuildResultFailureStatus::NoSubstituters, // 14 -}; - -BuildResultStatus -CommonProto::Serialise::read(const StoreDirConfig & store, CommonProto::ReadConn conn) -{ - auto rawStatus = readNum(conn.from); - - if (rawStatus >= std::size(buildResultStatusTable)) - throw Error("Invalid BuildResult status %d from remote", rawStatus); - - return buildResultStatusTable[rawStatus]; -} - -void CommonProto::Serialise::write( - const StoreDirConfig & store, CommonProto::WriteConn conn, const BuildResultStatus & status) -{ - /* See definition, the protocols don't know about `HashMismatch` - yet, so change it to `OutputRejected`, which they expect - for this case (hash mismatch is a type of output - rejection). */ - if (status == BuildResultStatus{BuildResultFailureStatus::HashMismatch}) { - return write(store, conn, BuildResultFailureStatus::OutputRejected); - } - for (auto && [wire, val] : enumerate(buildResultStatusTable)) - if (val == status) { - conn.to << uint8_t(wire); - return; - } - unreachable(); -} - } // namespace nix diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index a7be521a3d7..61969473375 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -134,7 +134,10 @@ struct TunnelLogger : public Logger to << STDERR_LAST; else { if (clientVersion >= WorkerProto::Version{.number = {1, 26}}) { - to << STDERR_ERROR << *ex; + to << STDERR_ERROR; + if (clientVersion.features.contains(WorkerProto::featureCancelledAndHashMismatchStatus)) + to << ex->info().status; + to << *ex; } else { to << STDERR_ERROR << ex->what() << ex->info().status; } diff --git a/src/libstore/include/nix/store/build-result.hh b/src/libstore/include/nix/store/build-result.hh index c664e6e5b6f..3aa9bc8126b 100644 --- a/src/libstore/include/nix/store/build-result.hh +++ b/src/libstore/include/nix/store/build-result.hh @@ -46,10 +46,15 @@ enum struct BuildResultFailureStatus : uint8_t { LogLimitExceeded, NotDeterministic, NoSubstituters, - /// A certain type of `OutputRejected`. The protocols do not yet - /// know about this one, so change it back to `OutputRejected` - /// before serialization. + /// A certain type of `OutputRejected`. Requires the + /// `cancelled-and-hash-mismatch-status` feature; falls back to + /// `OutputRejected` when communicating with older remotes. HashMismatch, + /// Goal was never attempted because another goal failed (and + /// `--keep-going` wasn't used). Requires the + /// `cancelled-and-hash-mismatch-status` feature; falls back to + /// `MiscFailure` when communicating with older remotes. + Cancelled, }; /** @@ -73,14 +78,22 @@ struct BuildError : public CloneableError */ bool isNonDeterministic = false; +private: + + /** + * Used in the constructors + */ + static unsigned int exitCodeFromStatus(Status status); + public: + /** * Variadic constructor for throwing with format strings. * Delegates to the string constructor after formatting. */ template BuildError(Status status, const Args &... args) - : CloneableError(args...) + : CloneableError(exitCodeFromStatus(status), args...) , status{status} { } @@ -97,10 +110,9 @@ public: * Also used for deserialization. */ BuildError(Args args) - : CloneableError(std::move(args.msg)) + : CloneableError(exitCodeFromStatus(args.status), std::move(args.msg)) , status{args.status} , isNonDeterministic{args.isNonDeterministic} - { } diff --git a/src/libstore/include/nix/store/build/goal.hh b/src/libstore/include/nix/store/build/goal.hh index 263647fc126..9b8790c5c2b 100644 --- a/src/libstore/include/nix/store/build/goal.hh +++ b/src/libstore/include/nix/store/build/goal.hh @@ -118,7 +118,7 @@ public: /** * Build result. */ - BuildResult buildResult; + BuildResult buildResult = {.inner = BuildResult::Failure{BuildResult::Failure::Cancelled, ""}}; /** * Suspend our goal and wait until we get `work`-ed again. diff --git a/src/libstore/include/nix/store/common-protocol.hh b/src/libstore/include/nix/store/common-protocol.hh index 2ef0f795b4e..a10d248a1d8 100644 --- a/src/libstore/include/nix/store/common-protocol.hh +++ b/src/libstore/include/nix/store/common-protocol.hh @@ -116,9 +116,6 @@ DECLARE_COMMON_SERIALISER(std::optional); */ using BuildResultStatus = std::variant; -template<> -DECLARE_COMMON_SERIALISER(BuildResultStatus); - #undef COMMA_ } // namespace nix diff --git a/src/libstore/include/nix/store/serve-protocol.hh b/src/libstore/include/nix/store/serve-protocol.hh index 94a250b2e3c..a33a60343f2 100644 --- a/src/libstore/include/nix/store/serve-protocol.hh +++ b/src/libstore/include/nix/store/serve-protocol.hh @@ -208,6 +208,8 @@ inline std::ostream & operator<<(std::ostream & s, ServeProto::Command op) static void write(const StoreDirConfig & store, ServeProto::WriteConn conn, const T & t); \ }; +template<> +DECLARE_SERVE_SERIALISER(BuildResultStatus); template<> DECLARE_SERVE_SERIALISER(BuildResult); template<> diff --git a/src/libstore/include/nix/store/worker-protocol.hh b/src/libstore/include/nix/store/worker-protocol.hh index 8e20d3c992c..0abd57bffde 100644 --- a/src/libstore/include/nix/store/worker-protocol.hh +++ b/src/libstore/include/nix/store/worker-protocol.hh @@ -124,6 +124,13 @@ struct WorkerProto */ static constexpr std::string_view featureRealisationWithPath = "realisation-with-path-not-hash"; + /** + * Feature for transmitting HashMismatch and Cancelled statuses. + * Falls back to OutputRejected / MiscFailure when communicating + * with older remotes that lack this feature. + */ + static constexpr std::string_view featureCancelledAndHashMismatchStatus = "cancelled-and-hash-mismatch-status"; + /** * A unidirectional read connection, to be used by the read half of the * canonical serializers below. @@ -311,6 +318,8 @@ inline std::ostream & operator<<(std::ostream & s, WorkerProto::Op op) static void write(const StoreDirConfig & store, WorkerProto::WriteConn conn, const T & t); \ }; +template<> +DECLARE_WORKER_SERIALISER(BuildResultStatus); template<> DECLARE_WORKER_SERIALISER(DerivedPath); template<> diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index b1781be457f..63b09430214 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -244,7 +244,7 @@ void LegacySSHStore::buildPaths( conn->to.flush(); - auto status = CommonProto::Serialise::read(*this, {conn->from}); + auto status = ServeProto::Serialise::read(*this, *conn); if (auto * failure = std::get_if(&status)) { std::string errorMsg; conn->from >> errorMsg; diff --git a/src/libstore/serve-protocol.cc b/src/libstore/serve-protocol.cc index bddd17da8b6..61c94f3002b 100644 --- a/src/libstore/serve-protocol.cc +++ b/src/libstore/serve-protocol.cc @@ -15,6 +15,72 @@ namespace nix { /* protocol-specific definitions */ +/** + * Mapping from protocol wire values to BuildResultStatus. + * + * The array index is the wire value. ServeProto does not support + * HashMismatch or Cancelled; those are converted before writing. + */ +constexpr static BuildResultStatus buildResultStatusTable[] = { + BuildResultSuccessStatus::Built, // 0 + BuildResultSuccessStatus::Substituted, // 1 + BuildResultSuccessStatus::AlreadyValid, // 2 + BuildResultFailureStatus::PermanentFailure, // 3 + BuildResultFailureStatus::InputRejected, // 4 + BuildResultFailureStatus::OutputRejected, // 5 + BuildResultFailureStatus::TransientFailure, // 6 + BuildResultFailureStatus::CachedFailure, // 7 + BuildResultFailureStatus::TimedOut, // 8 + BuildResultFailureStatus::MiscFailure, // 9 + BuildResultFailureStatus::DependencyFailed, // 10 + BuildResultFailureStatus::LogLimitExceeded, // 11 + BuildResultFailureStatus::NotDeterministic, // 12 + BuildResultSuccessStatus::ResolvesToAlreadyValid, // 13 + BuildResultFailureStatus::NoSubstituters, // 14 +}; + +BuildResultStatus +ServeProto::Serialise::read(const StoreDirConfig & store, ServeProto::ReadConn conn) +{ + auto rawStatus = readNum(conn.from); + + if (rawStatus >= std::size(buildResultStatusTable)) + throw Error("Invalid BuildResult status %d from remote", rawStatus); + + return buildResultStatusTable[rawStatus]; +} + +void ServeProto::Serialise::write( + const StoreDirConfig & store, ServeProto::WriteConn conn, const BuildResultStatus & status) +{ + auto effective = status; + + /* ServeProto doesn't have feature negotiation, so convert new + statuses that old remotes don't understand. */ + if (auto * f = std::get_if(&effective)) { +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wswitch-enum" + switch (*f) { + case BuildResultFailureStatus::HashMismatch: + effective = BuildResultFailureStatus::OutputRejected; + break; + case BuildResultFailureStatus::Cancelled: + effective = BuildResultFailureStatus::MiscFailure; + break; + default: + break; + } +#pragma GCC diagnostic pop + } + + for (auto && [wire, val] : enumerate(buildResultStatusTable)) + if (val == effective) { + conn.to << uint64_t(wire); + return; + } + unreachable(); +} + BuildResult ServeProto::Serialise::read(const StoreDirConfig & store, ServeProto::ReadConn conn) { BuildResult res; @@ -24,7 +90,7 @@ BuildResult ServeProto::Serialise::read(const StoreDirConfig & stor std::string errorMsg; bool isNonDeterministic = false; - auto status = ServeProto::Serialise::read(store, {conn.from}); + auto status = ServeProto::Serialise::read(store, conn); conn.from >> errorMsg; if (conn.version >= ServeProto::Version{2, 3}) @@ -100,11 +166,11 @@ void ServeProto::Serialise::write( std::visit( overloaded{ [&](const BuildResult::Failure & failure) { - ServeProto::write(store, {conn.to}, BuildResultStatus{failure.status}); + ServeProto::write(store, conn, BuildResultStatus{failure.status}); common(failure.message(), failure.isNonDeterministic, decltype(BuildResult::Success::builtOutputs){}); }, [&](const BuildResult::Success & success) { - ServeProto::write(store, {conn.to}, BuildResultStatus{success.status}); + ServeProto::write(store, conn, BuildResultStatus{success.status}); common(/*errorMsg=*/"", /*isNonDeterministic=*/false, success.builtOutputs); }, }, diff --git a/src/libstore/worker-protocol-connection.cc b/src/libstore/worker-protocol-connection.cc index dcc698293a3..b910e716e37 100644 --- a/src/libstore/worker-protocol-connection.cc +++ b/src/libstore/worker-protocol-connection.cc @@ -63,7 +63,12 @@ WorkerProto::BasicClientConnection::processStderrReturn(Sink * sink, Source * so else if (msg == STDERR_ERROR) { if (protoVersion >= WorkerProto::Version{.number = {1, 26}}) { - ex = std::make_exception_ptr(readError(from)); + unsigned int exitStatus = 1; + if (protoVersion.features.contains(WorkerProto::featureCancelledAndHashMismatchStatus)) + exitStatus = readInt(from); + auto e = readError(from); + e.withExitStatus(exitStatus); + ex = std::make_exception_ptr(std::move(e)); } else { auto error = readString(from); unsigned int status = readInt(from); diff --git a/src/libstore/worker-protocol.cc b/src/libstore/worker-protocol.cc index 3da966a6201..0a4ceccd6e0 100644 --- a/src/libstore/worker-protocol.cc +++ b/src/libstore/worker-protocol.cc @@ -19,13 +19,12 @@ const WorkerProto::Version WorkerProto::latest = { .number = { .major = 1, - .minor = 38, + .minor = 39, }, .features = { - std::string{ - WorkerProto::featureRealisationWithPath, - }, + std::string{WorkerProto::featureCancelledAndHashMismatchStatus}, + std::string{WorkerProto::featureRealisationWithPath}, }, }; @@ -54,6 +53,75 @@ std::partial_ordering WorkerProto::Version::operator<=>(const WorkerProto::Versi /* protocol-specific definitions */ +/** + * Mapping from protocol wire values to BuildResultStatus. + * + * The array index is the wire value. Indices 15 and 16 require the + * `cancelled-and-hash-mismatch-status` feature. + */ +constexpr static BuildResultStatus buildResultStatusTable[] = { + BuildResultSuccessStatus::Built, // 0 + BuildResultSuccessStatus::Substituted, // 1 + BuildResultSuccessStatus::AlreadyValid, // 2 + BuildResultFailureStatus::PermanentFailure, // 3 + BuildResultFailureStatus::InputRejected, // 4 + BuildResultFailureStatus::OutputRejected, // 5 + BuildResultFailureStatus::TransientFailure, // 6 + BuildResultFailureStatus::CachedFailure, // 7 + BuildResultFailureStatus::TimedOut, // 8 + BuildResultFailureStatus::MiscFailure, // 9 + BuildResultFailureStatus::DependencyFailed, // 10 + BuildResultFailureStatus::LogLimitExceeded, // 11 + BuildResultFailureStatus::NotDeterministic, // 12 + BuildResultSuccessStatus::ResolvesToAlreadyValid, // 13 + BuildResultFailureStatus::NoSubstituters, // 14 + BuildResultFailureStatus::HashMismatch, // 15 + BuildResultFailureStatus::Cancelled, // 16 +}; + +BuildResultStatus +WorkerProto::Serialise::read(const StoreDirConfig & store, WorkerProto::ReadConn conn) +{ + auto rawStatus = readNum(conn.from); + + if (rawStatus >= std::size(buildResultStatusTable)) + throw Error("Invalid BuildResult status %d from remote", rawStatus); + + return buildResultStatusTable[rawStatus]; +} + +void WorkerProto::Serialise::write( + const StoreDirConfig & store, WorkerProto::WriteConn conn, const BuildResultStatus & status) +{ + auto effective = status; + + /* If the remote doesn't support the cancelled-and-hash-mismatch-status + feature, convert new statuses for backwards compatibility. */ + if (auto * f = std::get_if(&effective); + f && !conn.version.features.contains(WorkerProto::featureCancelledAndHashMismatchStatus)) { +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wswitch-enum" + switch (*f) { + case BuildResultFailureStatus::HashMismatch: + effective = BuildResultFailureStatus::OutputRejected; + break; + case BuildResultFailureStatus::Cancelled: + effective = BuildResultFailureStatus::MiscFailure; + break; + default: + break; + } +#pragma GCC diagnostic pop + } + + for (auto && [wire, val] : enumerate(buildResultStatusTable)) + if (val == effective) { + conn.to << uint64_t(wire); + return; + } + unreachable(); +} + BuildMode WorkerProto::Serialise::read(const StoreDirConfig & store, WorkerProto::ReadConn conn) { auto temp = readNum(conn.from); diff --git a/src/nix/flake.cc b/src/nix/flake.cc index df2a3a9114a..4fe804596c3 100644 --- a/src/nix/flake.cc +++ b/src/nix/flake.cc @@ -18,6 +18,7 @@ #include "nix/cmd/markdown.hh" #include "nix/util/users.hh" #include "nix/fetchers/fetch-to-store.hh" +#include "nix/store/build-result.hh" #include "nix/store/local-fs-store.hh" #include "nix/store/globals.hh" @@ -357,6 +358,7 @@ struct CmdFlakeCheck : FlakeCommand auto localSystem = std::string(settings.thisSystem.get()); bool hasErrors = false; + ExitStatusFlags exitStatusFlags{}; auto reportError = [&](const Error & e) { try { throw e; @@ -816,10 +818,12 @@ struct CmdFlakeCheck : FlakeCommand // Report build failures with attribute paths for (auto & result : results) { if (auto * failure = result.tryGetFailure()) { + exitStatusFlags.updateFromStatus(failure->status); auto it = attrPathsByDrv.find(result.path); if (it != attrPathsByDrv.end() && !it->second.empty()) { for (auto & attrPath : it->second) { reportError(Error( + exitStatusFlags.failingExitStatus(), "failed to build attribute '%s', build of '%s' failed: %s", attrPath.to_string(*state), result.path.to_string(*store), @@ -827,14 +831,17 @@ struct CmdFlakeCheck : FlakeCommand } } else { // Derivation has no attribute path (e.g., a build dependency) - reportError( - Error("build of '%s' failed: %s", result.path.to_string(*store), failure->message())); + reportError(Error( + exitStatusFlags.failingExitStatus(), + "build of '%s' failed: %s", + result.path.to_string(*store), + failure->message())); } } } } if (hasErrors) - throw Error("some errors were encountered during the evaluation"); + throw Error(exitStatusFlags.failingExitStatus(), "some errors were encountered during the evaluation"); logger->log(lvlInfo, ANSI_GREEN "all checks passed!" ANSI_NORMAL); diff --git a/tests/functional/build.sh b/tests/functional/build.sh index 0e76f949f55..e47328b51e5 100755 --- a/tests/functional/build.sh +++ b/tests/functional/build.sh @@ -162,29 +162,39 @@ if isDaemonNewer "2.34pre"; then # With the fix, cancelled goals are not reported as failures. # Use -j1 so only x1 starts and fails; x2, x3, x4 are cancelled. out="$(nix build -f fod-failing.nix -j1 -L 2>&1)" && status=0 || status=$? - test "$status" = 1 + test "$status" = 102 # hash mismatch # Only the hash mismatch error for x1. Cancelled goals not reported. test "$(<<<"$out" grep -cE '^error:')" = 1 # Regression test: error messages should not be empty (end with just "failed:") <<<"$out" grepQuietInverse -E "^error:.*failed: *$" else out="$(nix build -f fod-failing.nix -L 2>&1)" && status=0 || status=$? + # Old daemons lack the feature flag to send proper exit codes, so we get 1. test "$status" = 1 # At minimum, check that x1 is reported as failing <<<"$out" grepQuiet -E "error:.*-x1" fi -<<<"$out" grepQuiet -E "hash mismatch in fixed-output derivation '.*-x1\\.drv'" -<<<"$out" grepQuiet -vE "hash mismatch in fixed-output derivation '.*-x3\\.drv'" -<<<"$out" grepQuiet -vE "hash mismatch in fixed-output derivation '.*-x2\\.drv'" +# Detailed error format checks only work with new daemons (buildPathsWithResults). +# Old daemons return a single combined error via buildPaths. +if isDaemonNewer "2.34pre"; then + <<<"$out" grepQuiet -E "hash mismatch in fixed-output derivation '.*-x1\\.drv'" + <<<"$out" grepQuiet -vE "hash mismatch in fixed-output derivation '.*-x3\\.drv'" + <<<"$out" grepQuiet -vE "hash mismatch in fixed-output derivation '.*-x2\\.drv'" +fi out="$(nix build -f fod-failing.nix -L x1 x2 x3 --keep-going 2>&1)" && status=0 || status=$? -test "$status" = 1 -# three "hash mismatch" errors - for each failing fod, one "build of ... failed" -test "$(<<<"$out" grep -cE '^error:')" = 4 -<<<"$out" grepQuiet -E "hash mismatch in fixed-output derivation '.*-x1\\.drv'" -<<<"$out" grepQuiet -E "hash mismatch in fixed-output derivation '.*-x3\\.drv'" -<<<"$out" grepQuiet -E "hash mismatch in fixed-output derivation '.*-x2\\.drv'" -<<<"$out" grepQuiet -E "error: build of '.*-x[1-3]\\.drv\\^out', '.*-x[1-3]\\.drv\\^out', '.*-x[1-3]\\.drv\\^out' failed" +# All three FODs fail with hash mismatch; aggregate exit status reflects this +if isDaemonNewer "2.34pre"; then + test "$status" = 102 # hash mismatch + # three "hash mismatch" errors + one "build of ... failed" + test "$(<<<"$out" grep -cE '^error:')" = 4 + <<<"$out" grepQuiet -E "hash mismatch in fixed-output derivation '.*-x1\\.drv'" + <<<"$out" grepQuiet -E "hash mismatch in fixed-output derivation '.*-x3\\.drv'" + <<<"$out" grepQuiet -E "hash mismatch in fixed-output derivation '.*-x2\\.drv'" + <<<"$out" grepQuiet -E "error: build of '.*-x[1-3]\\.drv\\^out', '.*-x[1-3]\\.drv\\^out', '.*-x[1-3]\\.drv\\^out' failed" +else + test "$status" = 1 # old daemons lack feature flag, exit code defaults to 1 +fi out="$(nix build -f fod-failing.nix -L x4 2>&1)" && status=0 || status=$? test "$status" = 1 @@ -222,27 +232,23 @@ fi # Only fast-fail should be reported as a failure. # Uses fifo for synchronization to ensure deterministic behavior. # Requires -j2 so slow and fast-fail run concurrently (fifo deadlocks if serialized). -if isDaemonNewer "2.34pre" && canUseSandbox; then +# Skipped on NixOS: requires extra-sandbox-paths which untrusted users cannot set. +if isDaemonNewer "2.34pre" && canUseSandbox && ! isTestOnNixOS; then fifoDir="$TEST_ROOT/cancelled-builds-fifo" mkdir -p "$fifoDir" mkfifo "$fifoDir/fifo" chmod a+rw "$fifoDir/fifo" - # When using a separate test store, we need sandbox-paths to access - # the system store (where bash/coreutils live). On NixOS, the test - # uses the system store directly, so this isn't needed (and would - # conflict with input paths). - sandboxPathsArg=() - if ! isTestOnNixOS; then - sandboxPathsArg=(--option sandbox-paths "/nix/store") - fi + sandboxArgs=( + --option sandbox true + --option sandbox-paths "/nix/store" + --option sandbox-build-dir /build-tmp + --option extra-sandbox-paths "/cancelled-builds-fifo=$fifoDir" + ) out="$(nix flake check ./cancelled-builds --impure -L -j2 \ - --option sandbox true \ - "${sandboxPathsArg[@]}" \ - --option sandbox-build-dir /build-tmp \ - --option extra-sandbox-paths "/cancelled-builds-fifo=$fifoDir" \ + "${sandboxArgs[@]}" \ 2>&1)" && status=0 || status=$? rm -rf "$fifoDir" - test "$status" = 1 + test "$status" = 100 # build failure # The error should be for fast-fail, not for cancelled goals <<<"$out" grepQuiet -E "Cannot build.*fast-fail" # Cancelled goals should NOT appear in error messages (but may appear in "will be built" list) @@ -256,23 +262,16 @@ if isDaemonNewer "2.34pre" && canUseSandbox; then mkdir -p "$fifoDir" mkfifo "$fifoDir/fifo" chmod a+rw "$fifoDir/fifo" - sandboxPathsArg=() - if ! isTestOnNixOS; then - sandboxPathsArg=(--option sandbox-paths "/nix/store") - fi system=$(nix eval --raw --impure --expr builtins.currentSystem) out="$(nix build --impure -L -j2 \ - --option sandbox true \ - "${sandboxPathsArg[@]}" \ - --option sandbox-build-dir /build-tmp \ - --option extra-sandbox-paths "/cancelled-builds-fifo=$fifoDir" \ + "${sandboxArgs[@]}" \ "./cancelled-builds#checks.$system.slow" \ "./cancelled-builds#checks.$system.depends-on-slow" \ "./cancelled-builds#checks.$system.fast-fail" \ "./cancelled-builds#checks.$system.depends-on-fail" \ 2>&1)" && status=0 || status=$? rm -rf "$fifoDir" - test "$status" = 1 + test "$status" = 100 # build failure # The error should be for fast-fail, not for cancelled goals <<<"$out" grepQuiet -E "Cannot build.*fast-fail" # Cancelled goals should NOT appear in error messages @@ -288,7 +287,7 @@ fi # "local builds are disabled" instead of the misleading # "required system or feature not available". if isDaemonNewer "2.34pre"; then - expectStderr 1 nix build --impure --max-jobs 0 --expr \ + expectStderr 100 nix build --impure --max-jobs 0 --expr \ 'derivation { name = "test-maxjobs"; builder = "/bin/sh"; args = ["-c" "exit 0"]; system = builtins.currentSystem; }' \ --no-link \ | grepQuiet "local builds are disabled" diff --git a/tests/functional/check-refs.sh b/tests/functional/check-refs.sh index 16bcac1582e..bbfd64750c9 100755 --- a/tests/functional/check-refs.sh +++ b/tests/functional/check-refs.sh @@ -63,7 +63,13 @@ fi if isDaemonNewer "2.28pre20241225"; then # test12 should fail (syntactically invalid). - expectStderr 1 nix-build -vvv -o "$RESULT" check-refs.nix -A test12 >"$TEST_ROOT/test12.stderr" + # New daemons send the proper exit code (100 = build failure); + # old daemons lack the feature flag and default to exit code 1. + if isDaemonNewer "2.34pre"; then + expectStderr 100 nix-build -vvv -o "$RESULT" check-refs.nix -A test12 >"$TEST_ROOT/test12.stderr" + else + expectStderr 1 nix-build -vvv -o "$RESULT" check-refs.nix -A test12 >"$TEST_ROOT/test12.stderr" + fi if isDaemonNewer "2.33pre20251110"; then grepQuiet -F \ "output check for 'lib' contains output name 'dev', but this is not a valid output of this derivation. (Valid outputs are [lib, out].)" \ diff --git a/tests/functional/common/functions.sh b/tests/functional/common/functions.sh index 771bbca785b..3aa947972ea 100644 --- a/tests/functional/common/functions.sh +++ b/tests/functional/common/functions.sh @@ -222,7 +222,7 @@ assertStderr() { } needLocalStore() { - if [[ "$NIX_REMOTE" == "daemon" ]]; then + if [[ "$NIX_REMOTE" == "daemon" ]] || isTestOnNixOS; then skipTest "Can't run through the daemon ($1)" fi } diff --git a/tests/functional/dyn-drv/failing-outer.sh b/tests/functional/dyn-drv/failing-outer.sh index 709f79619ea..857b678535c 100644 --- a/tests/functional/dyn-drv/failing-outer.sh +++ b/tests/functional/dyn-drv/failing-outer.sh @@ -6,7 +6,7 @@ source common.sh requireDaemonNewerThan "2.30pre20250515" expected=100 -if [[ -v NIX_DAEMON_PACKAGE ]]; then expected=1; fi # work around the daemon not returning a 100 status correctly +if [[ -v NIX_DAEMON_PACKAGE ]] && ! isDaemonNewer "2.34pre"; then expected=1; fi # work around old daemons not returning a 100 status correctly expectStderr "$expected" nix-build ./text-hashed-output.nix -A failingWrapper --no-out-link \ | grepQuiet "build of resolved derivation '.*use-dynamic-drv-in-non-dynamic-drv-wrong.drv' failed" diff --git a/tests/functional/fetchurl.sh b/tests/functional/fetchurl.sh index c25ac321668..d3d995db851 100755 --- a/tests/functional/fetchurl.sh +++ b/tests/functional/fetchurl.sh @@ -86,5 +86,5 @@ test -L "$outPath/symlink" # Make sure that *not* passing a outputHash fails. requireDaemonNewerThan "2.20" expected=100 -if [[ -v NIX_DAEMON_PACKAGE ]]; then expected=1; fi # work around the daemon not returning a 100 status correctly +if [[ -v NIX_DAEMON_PACKAGE ]] && ! isDaemonNewer "2.34pre"; then expected=1; fi # work around old daemons not returning a 100 status correctly expectStderr $expected nix-build --expr '{ url }: builtins.derivation { name = "nix-cache-info"; system = "x86_64-linux"; builder = "builtin:fetchurl"; inherit url; outputHashMode = "flat"; }' --argstr url "file://$narxz" 2>&1 | grep 'must be a fixed-output or impure derivation' diff --git a/tests/functional/timeout.sh b/tests/functional/timeout.sh index 1bd23118f1a..79dc3b9cd7d 100755 --- a/tests/functional/timeout.sh +++ b/tests/functional/timeout.sh @@ -17,4 +17,4 @@ expectStderr 101 nix-build timeout.nix -A silent --max-silent-time 2 | grepQuiet expectStderr 100 nix-build timeout.nix -A closeLog | grepQuiet "builder failed due to signal" -expectStderr 1 nix build -f timeout.nix silent --max-silent-time 2 | grepQuiet "timed out after 2 seconds" +expectStderr 101 nix build -f timeout.nix silent --max-silent-time 2 | grepQuiet "timed out after 2 seconds"