From 6758e65612b990805d3d7d2039cd92647730e900 Mon Sep 17 00:00:00 2001
From: regnat <rg@regnat.ovh>
Date: Wed, 9 Dec 2020 09:44:07 +0100
Subject: [PATCH 1/3] Revert "Re-query for the derivation outputs in the
 post-build-hook"

This reverts commit 1b1e0760335832c87516b9103b670b34662d5daf.

Using `queryPartialDerivationOutputMap` assumes that the derivation
exists locally which isn't the case for remote builders.
---
 src/libstore/build/derivation-goal.cc | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc
index fdf777c27..1db85bd37 100644
--- a/src/libstore/build/derivation-goal.cc
+++ b/src/libstore/build/derivation-goal.cc
@@ -899,10 +899,8 @@ void DerivationGoal::buildDone()
                 Logger::Fields{worker.store.printStorePath(drvPath)});
             PushActivity pact(act.id);
             StorePathSet outputPaths;
-            for (auto& [_, maybeOutPath] :
-                 worker.store.queryPartialDerivationOutputMap(drvPath)) {
-                if (maybeOutPath)
-                    outputPaths.insert(*maybeOutPath);
+            for (auto i : drv->outputs) {
+                outputPaths.insert(finalOutputs.at(i.first));
             }
             std::map<std::string, std::string> hookEnvironment = getEnv();
 

From ee7c94fa1b74b43f7a719373f5f566d09b147126 Mon Sep 17 00:00:00 2001
From: regnat <rg@regnat.ovh>
Date: Wed, 9 Dec 2020 10:37:39 +0100
Subject: [PATCH 2/3] Test the post-build-hook with remote builders

Regression test for #4245
---
 tests/build-remote-input-addressed.sh | 28 +++++++++++++++++++++++++++
 tests/build-remote.sh                 |  3 +++
 2 files changed, 31 insertions(+)

diff --git a/tests/build-remote-input-addressed.sh b/tests/build-remote-input-addressed.sh
index b34caa061..49d15c389 100644
--- a/tests/build-remote-input-addressed.sh
+++ b/tests/build-remote-input-addressed.sh
@@ -3,3 +3,31 @@ source common.sh
 file=build-hook.nix
 
 source build-remote.sh
+
+# Add a `post-build-hook` option to the nix conf.
+# This hook will be executed both for the local machine and the remote builders
+# (because they share the same config).
+registerBuildHook () {
+    # Dummy post-build-hook just to ensure that it's executed correctly.
+    # (we can't reuse the one from `$PWD/push-to-store.sh` because of
+    # https://github.com/NixOS/nix/issues/4341)
+    cat <<EOF > $TEST_ROOT/post-build-hook.sh
+#!/bin/sh
+
+echo "Post hook ran successfully"
+# Add an empty line to a counter file, just to check that this hook ran properly
+echo "" >> $TEST_ROOT/post-hook-counter
+EOF
+    chmod +x $TEST_ROOT/post-build-hook.sh
+    rm -f $TEST_ROOT/post-hook-counter
+
+    echo "post-build-hook = $TEST_ROOT/post-build-hook.sh" >> $NIX_CONF_DIR/nix.conf
+}
+
+registerBuildHook
+source build-remote.sh
+
+# `build-hook.nix` has four derivations to build, and the hook runs twice for
+# each derivation (once on the builder and once on the host), so the counter
+# should contain eight lines now
+[[ $(cat $TEST_ROOT/post-hook-counter | wc -l) -eq 8 ]]
diff --git a/tests/build-remote.sh b/tests/build-remote.sh
index ca6d1de09..04848e4b5 100644
--- a/tests/build-remote.sh
+++ b/tests/build-remote.sh
@@ -14,6 +14,9 @@ builders=(
   "ssh-ng://localhost?remote-store=$TEST_ROOT/machine3?system-features=baz - - 1 1 baz"
 )
 
+chmod -R +w $TEST_ROOT/machine* || true
+rm -rf $TEST_ROOT/machine* || true
+
 # Note: ssh://localhost bypasses ssh, directly invoking nix-store as a
 # child process. This allows us to test LegacySSHStore::buildDerivation().
 # ssh-ng://... likewise allows us to test RemoteStore::buildDerivation().

From c87267c2a4621a42d6bfcdb7944cd546f194787a Mon Sep 17 00:00:00 2001
From: regnat <rg@regnat.ovh>
Date: Wed, 9 Dec 2020 10:38:04 +0100
Subject: [PATCH 3/3] Store the final drv outputs in memory when building
 remotely
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The `DerivationGoal` has a variable storing the “final” derivation
output paths that is used (amongst other things) to fill the environment
for the post build hook. However this variable wasn't set when the
build-hook is used, causing a crash when both hooks are used together.

Fix this by setting this variable (from the informations in the db) after a run
of the post build hook.
---
 src/libstore/build/derivation-goal.cc | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc
index 1db85bd37..de58d9f06 100644
--- a/src/libstore/build/derivation-goal.cc
+++ b/src/libstore/build/derivation-goal.cc
@@ -2872,6 +2872,8 @@ void DerivationGoal::registerOutputs()
         for (auto & i : drv->outputsAndOptPaths(worker.store)) {
             if (!i.second.second || !worker.store.isValidPath(*i.second.second))
                 allValid = false;
+            else
+                finalOutputs.insert_or_assign(i.first, *i.second.second);
         }
         if (allValid) return;
     }