From 87077739658b35863fb56131fd7099294c181ee5 Mon Sep 17 00:00:00 2001
From: regnat <rg@regnat.ovh>
Date: Tue, 20 Jul 2021 06:57:56 +0200
Subject: [PATCH] Properly lock the builds of CA derivations
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Make sure that we can’t build the same derivation twice at the same
time.

Fix https://github.com/NixOS/nix/issues/5029
---
 src/libstore/build/derivation-goal.cc | 10 ++++++++--
 tests/ca/concurrent-builds.sh         | 18 ++++++++++++++++++
 tests/ca/racy.nix                     | 15 +++++++++++++++
 tests/local.mk                        |  1 +
 4 files changed, 42 insertions(+), 2 deletions(-)
 create mode 100755 tests/ca/concurrent-builds.sh
 create mode 100644 tests/ca/racy.nix

diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc
index 06f854629..7df597400 100644
--- a/src/libstore/build/derivation-goal.cc
+++ b/src/libstore/build/derivation-goal.cc
@@ -544,7 +544,7 @@ void DerivationGoal::tryToBuild()
     PathSet lockFiles;
     /* FIXME: Should lock something like the drv itself so we don't build same
        CA drv concurrently */
-    if (dynamic_cast<LocalStore *>(&worker.store))
+    if (dynamic_cast<LocalStore *>(&worker.store)) {
         /* If we aren't a local store, we might need to use the local store as
            a build remote, but that would cause a deadlock. */
         /* FIXME: Make it so we can use ourselves as a build remote even if we
@@ -552,9 +552,15 @@ void DerivationGoal::tryToBuild()
         /* FIXME: find some way to lock for scheduling for the other stores so
            a forking daemon with --store still won't farm out redundant builds.
            */
-        for (auto & i : drv->outputsAndOptPaths(worker.store))
+        for (auto & i : drv->outputsAndOptPaths(worker.store)) {
             if (i.second.second)
                 lockFiles.insert(worker.store.Store::toRealPath(*i.second.second));
+            else
+                lockFiles.insert(
+                    worker.store.Store::toRealPath(drvPath) + "!" + i.first
+                );
+        }
+    }
 
     if (!outputLocks.lockPaths(lockFiles, "", false)) {
         if (!actLock)
diff --git a/tests/ca/concurrent-builds.sh b/tests/ca/concurrent-builds.sh
new file mode 100755
index 000000000..68441ec76
--- /dev/null
+++ b/tests/ca/concurrent-builds.sh
@@ -0,0 +1,18 @@
+#!/usr/bin/env bash
+
+# Ensure that we can’t build twice the same derivation concurrently.
+# Regression test for https://github.com/NixOS/nix/issues/5029
+
+source common.sh
+
+sed -i 's/experimental-features .*/& ca-derivations ca-references/' "$NIX_CONF_DIR"/nix.conf
+
+export NIX_TESTS_CA_BY_DEFAULT=1
+
+clearStore
+
+for i in {0..5}; do
+    nix build --no-link --file ./racy.nix &
+done
+
+wait
diff --git a/tests/ca/racy.nix b/tests/ca/racy.nix
new file mode 100644
index 000000000..555a15484
--- /dev/null
+++ b/tests/ca/racy.nix
@@ -0,0 +1,15 @@
+# A derivation that would certainly fail if several builders tried to
+# build it at once.
+
+
+with import ./config.nix;
+
+mkDerivation {
+  name = "simple";
+  buildCommand = ''
+    mkdir $out
+    echo bar >> $out/foo
+    sleep 3
+    [[ "$(cat $out/foo)" == bar ]]
+  '';
+}
diff --git a/tests/local.mk b/tests/local.mk
index ce9725543..e16c5a9b7 100644
--- a/tests/local.mk
+++ b/tests/local.mk
@@ -55,6 +55,7 @@ nix_tests = \
   ca/nix-shell.sh \
   ca/nix-run.sh \
   ca/recursive.sh \
+  ca/concurrent-builds.sh \
   ca/nix-copy.sh
   # parallel.sh