From 50edbc4ddf1f32fbf30ecaa7222beadf10947f5e Mon Sep 17 00:00:00 2001
From: Maximilian Bosch <maximilian@mbosch.me>
Date: Mon, 23 Aug 2021 23:47:29 +0200
Subject: [PATCH] nix-store --serve: pass on `settings.keepFailed` from SSH
 store

When doing e.g.

    nix-build -A package --keep-failed --option \
      builders \
      'ssh://mfhydra?remote-store=/home/bosch/store x86_64-linux - 10 4 big-parallel'

this doesn't work properly because this build-setting is ignored.

I changed this behavior by passing the `settings.keepFailed` through the
serve-protocol to remote machines to make sure that I can introspect the
build-directory (which is particularly helpful when I have to look at a
`config.log` from a failed build for instance).
---
 src/libstore/legacy-ssh-store.cc |  4 ++++
 src/libstore/serve-protocol.hh   |  2 +-
 src/nix-store/nix-store.cc       |  3 +++
 tests/build-remote.sh            | 13 +++++++++++++
 tests/failing.nix                | 22 ++++++++++++++++++++++
 5 files changed, 43 insertions(+), 1 deletion(-)
 create mode 100644 tests/failing.nix

diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc
index 45eed5707..7da3b36b5 100644
--- a/src/libstore/legacy-ssh-store.cc
+++ b/src/libstore/legacy-ssh-store.cc
@@ -237,6 +237,10 @@ private:
             conn.to
                 << settings.buildRepeat
                 << settings.enforceDeterminism;
+
+        if (GET_PROTOCOL_MINOR(conn.remoteVersion) >= 7) {
+            conn.to << ((int) settings.keepFailed);
+        }
     }
 
 public:
diff --git a/src/libstore/serve-protocol.hh b/src/libstore/serve-protocol.hh
index 02d0810cc..3f76baa82 100644
--- a/src/libstore/serve-protocol.hh
+++ b/src/libstore/serve-protocol.hh
@@ -5,7 +5,7 @@ namespace nix {
 #define SERVE_MAGIC_1 0x390c9deb
 #define SERVE_MAGIC_2 0x5452eecb
 
-#define SERVE_PROTOCOL_VERSION (2 << 8 | 6)
+#define SERVE_PROTOCOL_VERSION (2 << 8 | 7)
 #define GET_PROTOCOL_MAJOR(x) ((x) & 0xff00)
 #define GET_PROTOCOL_MINOR(x) ((x) & 0x00ff)
 
diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc
index b327793e7..f0ce0368a 100644
--- a/src/nix-store/nix-store.cc
+++ b/src/nix-store/nix-store.cc
@@ -801,6 +801,9 @@ static void opServe(Strings opFlags, Strings opArgs)
             settings.enforceDeterminism = readInt(in);
             settings.runDiffHook = true;
         }
+        if (GET_PROTOCOL_MINOR(clientVersion) >= 7) {
+            settings.keepFailed = (bool) readInt(in);
+        }
         settings.printRepeatedBuilds = false;
     };
 
diff --git a/tests/build-remote.sh b/tests/build-remote.sh
index 27d85a83d..806c6d261 100644
--- a/tests/build-remote.sh
+++ b/tests/build-remote.sh
@@ -53,3 +53,16 @@ nix path-info --store $TEST_ROOT/machine3 --all \
   | grep -v builder-build-remote-input-1.sh \
   | grep -v builder-build-remote-input-2.sh \
   | grep builder-build-remote-input-3.sh
+
+# Behavior of keep-failed
+out="$(nix-build 2>&1 failing.nix \
+  --builders "$(join_by '; ' "${builders[@]}")"  \
+  --keep-failed \
+  --store $TEST_ROOT/machine0 \
+  -j0 \
+  --arg busybox $busybox)" || true
+
+[[ "$out" =~ .*"note: keeping build directory".* ]]
+
+build_dir="$(grep "note: keeping build" <<< "$out" | sed -E "s/^(.*)note: keeping build directory '(.*)'(.*)$/\2/")"
+[[ "foo" = $(<"$build_dir"/bar) ]]
diff --git a/tests/failing.nix b/tests/failing.nix
new file mode 100644
index 000000000..2a0350d4d
--- /dev/null
+++ b/tests/failing.nix
@@ -0,0 +1,22 @@
+{ busybox }:
+with import ./config.nix;
+let
+
+  mkDerivation = args:
+    derivation ({
+      inherit system;
+      builder = busybox;
+      args = ["sh" "-e" args.builder or (builtins.toFile "builder-${args.name}.sh" "if [ -e .attrs.sh ]; then source .attrs.sh; fi; eval \"$buildCommand\"")];
+    } // removeAttrs args ["builder" "meta"])
+    // { meta = args.meta or {}; };
+in
+{
+
+  failing = mkDerivation {
+    name = "failing";
+    buildCommand = ''
+      echo foo > bar
+      exit 1
+    '';
+  };
+}