Merge pull request #4158 from hercules-ci/issue-3964-substitution-loop

Fix substitution loop #3964, #3534

3 files changed, 42 insertions, 3 deletions

diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc
index fda05f0e9..1c9217537 100644
--- a/src/libstore/build/derivation-goal.cc
+++ b/src/libstore/build/derivation-goal.cc
@@ -330,8 +330,13 @@ void DerivationGoal::outputsSubstitutionTried()
 
     /*  If the substitutes form an incomplete closure, then we should
         build the dependencies of this derivation, but after that, we
-        can still use the substitutes for this derivation itself. */
-    if (nrIncompleteClosure > 0) retrySubstitution = true;
+        can still use the substitutes for this derivation itself.
+
+        If the nrIncompleteClosure != nrFailed, we have another issue as well.
+        In particular, it may be the case that the hole in the closure is
+        an output of the current derivation, which causes a loop if retried.
+     */
+    if (nrIncompleteClosure > 0 && nrIncompleteClosure == nrFailed) retrySubstitution = true;
 
     nrFailed = nrNoSubstituters = nrIncompleteClosure = 0;
 
diff --git a/src/libstore/build/goal.hh b/src/libstore/build/goal.hh
index 360c160ce..0781a9d38 100644
--- a/src/libstore/build/goal.hh
+++ b/src/libstore/build/goal.hh
@@ -46,7 +46,7 @@ struct Goal : public std::enable_shared_from_this<Goal>
     unsigned int nrNoSubstituters;
 
     /* Number of substitution goals we are/were waiting for that
-       failed because othey had unsubstitutable references. */
+       failed because they had unsubstitutable references. */
     unsigned int nrIncompleteClosure;
 
     /* Name of this goal for debugging purposes. */
diff --git a/tests/binary-cache.sh b/tests/binary-cache.sh
index fe4ddec8d..e14cf882e 100644
--- a/tests/binary-cache.sh
+++ b/tests/binary-cache.sh
@@ -239,3 +239,37 @@ nix copy --to "file://$cacheDir?index-debug-info=1&compression=none" $outPath
 diff -u \
     <(cat $cacheDir/debuginfo/02623eda209c26a59b1a8638ff7752f6b945c26b.debug | jq -S) \
     <(echo '{"archive":"../nar/100vxs724qr46phz8m24iswmg9p3785hsyagz0kchf6q6gf06sw6.nar","member":"lib/debug/.build-id/02/623eda209c26a59b1a8638ff7752f6b945c26b.debug"}' | jq -S)
+
+# Test against issue https://github.com/NixOS/nix/issues/3964
+#
+expr='
+  with import ./config.nix;
+  mkDerivation {
+    name = "multi-output";
+    buildCommand = "mkdir -p $out; echo foo > $doc; echo $doc > $out/docref";
+    outputs = ["out" "doc"];
+  }
+'
+outPath=$(nix-build --no-out-link -E "$expr")
+docPath=$(nix-store -q --references $outPath)
+
+# $ nix-store -q --tree $outPath
+# ...-multi-output
+# +---...-multi-output-doc
+
+nix copy --to "file://$cacheDir" $outPath
+( echo $outPath $docPath
+  find $cacheDir
+) >/tmp/blurb
+
+hashpart() {
+  basename "$1" | cut -c1-32
+}
+
+# break the closure of out by removing doc
+rm $cacheDir/$(hashpart $docPath).narinfo
+
+nix-store --delete $outPath $docPath
+# -vvv is the level that logs during the loop
+timeout 60 nix-build -E "$expr" --option substituters "file://$cacheDir" \
+  --option trusted-binary-caches "file://$cacheDir"  --no-require-sigs