worker-protocol: add cancelled-and-hash-mismatch-status feature

src/libcmd/installables.cc

@@ -586,13 +586,18 @@ static void throwBuildErrors(std::vector<KeyedBuildResult> & 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)));
         }
     }
 }

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<std::pair<BuildResult::Failure::Status, std::string_view>, 12> failureStatusStrings{{
+static constexpr std::array<std::pair<BuildResult::Failure::Status, std::string_view>, 13> failureStatusStrings{{
 #define ENUM_ENTRY(e) {BuildResult::Failure::e, #e}
     ENUM_ENTRY(PermanentFailure),
     ENUM_ENTRY(InputRejected),
@@ -104,6 +112,7 @@ static constexpr std::array<std::pair<BuildResult::Failure::Status, std::string_
     ENUM_ENTRY(NotDeterministic),
     ENUM_ENTRY(NoSubstituters),
     ENUM_ENTRY(HashMismatch),
+    ENUM_ENTRY(Cancelled),
 #undef ENUM_ENTRY
 }};
 

src/libstore/common-protocol.cc

@@ -83,58 +83,4 @@ void CommonProto::Serialise<Signature>::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<BuildResultStatus>::read(const StoreDirConfig & store, CommonProto::ReadConn conn)
-{
-    auto rawStatus = readNum<uint8_t>(conn.from);
-
-    if (rawStatus >= std::size(buildResultStatusTable))
-        throw Error("Invalid BuildResult status %d from remote", rawStatus);
-
-    return buildResultStatusTable[rawStatus];
-}
-
-void CommonProto::Serialise<BuildResultStatus>::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

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;
             }

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<BuildError, Error>
      */
     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<typename... Args>
     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}
-
     {
     }
 

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.

src/libstore/include/nix/store/common-protocol.hh

@@ -116,9 +116,6 @@ DECLARE_COMMON_SERIALISER(std::optional<ContentAddress>);
  */
 using BuildResultStatus = std::variant<BuildResultSuccessStatus, BuildResultFailureStatus>;
 
-template<>
-DECLARE_COMMON_SERIALISER(BuildResultStatus);
-
 #undef COMMA_
 
 } // namespace nix

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<>

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<>

src/libstore/legacy-ssh-store.cc

@@ -244,7 +244,7 @@ void LegacySSHStore::buildPaths(
 
     conn->to.flush();
 
-    auto status = CommonProto::Serialise<BuildResultStatus>::read(*this, {conn->from});
+    auto status = ServeProto::Serialise<BuildResultStatus>::read(*this, *conn);
     if (auto * failure = std::get_if<BuildResultFailureStatus>(&status)) {
         std::string errorMsg;
         conn->from >> errorMsg;

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<BuildResultStatus>::read(const StoreDirConfig & store, ServeProto::ReadConn conn)
+{
+    auto rawStatus = readNum<uint64_t>(conn.from);
+
+    if (rawStatus >= std::size(buildResultStatusTable))
+        throw Error("Invalid BuildResult status %d from remote", rawStatus);
+
+    return buildResultStatusTable[rawStatus];
+}
+
+void ServeProto::Serialise<BuildResultStatus>::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<BuildResultFailureStatus>(&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<BuildResult>::read(const StoreDirConfig & store, ServeProto::ReadConn conn)
 {
     BuildResult res;
@@ -24,7 +90,7 @@ BuildResult ServeProto::Serialise<BuildResult>::read(const StoreDirConfig & stor
     std::string errorMsg;
     bool isNonDeterministic = false;
 
-    auto status = ServeProto::Serialise<BuildResultStatus>::read(store, {conn.from});
+    auto status = ServeProto::Serialise<BuildResultStatus>::read(store, conn);
     conn.from >> errorMsg;
 
     if (conn.version >= ServeProto::Version{2, 3})
@@ -100,11 +166,11 @@ void ServeProto::Serialise<BuildResult>::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);
             },
         },

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);