From e356d54d7a42f8501db5cf71b9e6baf4c3ff8b93 Mon Sep 17 00:00:00 2001
From: eldritch horrors <pennae@lix.systems>
Date: Fri, 27 Jun 2025 15:38:53 +0200
Subject: [PATCH] libstore: don't delete already valid outputs after build

eagerly consider outputs as not needing deletion during output
registration rather than only doing so after registration. not
waiting for registration to succeed may keep store paths alive
in the file system if registration fails for some reason; that
seem preferrable to the possibility of having another instance
of this bug. since we only leave *good* outputs around there's
not much to worry about except maybe bit of wasted disk space.

fixes #883

Change-Id: I8c22c92e39b9e203f1061278f86cde19dc4474a4
---
 lix/libstore/build/local-derivation-goal.cc | 16 +++++-----------
 tests/functional/linux-sandbox.sh           |  7 +++++++
 tests/functional/regression-fj883.nix       | 15 +++++++++++++++
 3 files changed, 27 insertions(+), 11 deletions(-)
 create mode 100644 tests/functional/regression-fj883.nix

diff --git a/lix/libstore/build/local-derivation-goal.cc b/lix/libstore/build/local-derivation-goal.cc
index 2222d10c6..11a973d06 100644
--- a/lix/libstore/build/local-derivation-goal.cc
+++ b/lix/libstore/build/local-derivation-goal.cc
@@ -1692,6 +1692,11 @@ try {
                before this for loop. */
             if (*scratchPath != finalStorePath)
                 outputRewrites[std::string { scratchPath->hashPart() }] = std::string { finalStorePath.hashPart() };
+            /* Cancel automatic deletion of that output if it was a scratch output that we just
+             * registered. */
+            if (auto cleaner = scratchOutputsCleaner.extract(outputName)) {
+                cleaner.mapped().cancel();
+            }
         };
 
         auto orifu = get(outputReferencesIfUnregistered, outputName);
@@ -2002,10 +2007,6 @@ try {
            the next iteration */
         if (newInfo.ca) {
             TRY_AWAIT(localStore.registerValidPaths({{newInfo.path, newInfo}}));
-            /* Cancel automatic deletion of that output if it was a scratch output. */
-            if (auto cleaner = scratchOutputsCleaner.extract(outputName)) {
-                cleaner.mapped().cancel();
-            }
         }
 
         infos.emplace(outputName, std::move(newInfo));
@@ -2045,13 +2046,6 @@ try {
             infos2.insert_or_assign(newInfo.path, newInfo);
         }
         TRY_AWAIT(localStore.registerValidPaths(infos2));
-
-        /* Cancel automatic deletion of that output if it was a scratch output that we just registered. */
-        for (auto & [outputName, _ ] : infos) {
-            if (auto cleaner = scratchOutputsCleaner.extract(outputName)) {
-                cleaner.mapped().cancel();
-            }
-        }
     }
 
     /* In case of a fixed-output derivation hash mismatch, throw an
diff --git a/tests/functional/linux-sandbox.sh b/tests/functional/linux-sandbox.sh
index 82f363a09..526605e5f 100644
--- a/tests/functional/linux-sandbox.sh
+++ b/tests/functional/linux-sandbox.sh
@@ -81,3 +81,10 @@ testCert present fixed-output "$certsymlink"
 
 # Symlinks should be added in the sandbox directly and not followed
 nix-sandbox-build symlink-derivation.nix
+
+# Regression fj#883: derivations outputs disappearing after rebuild
+# build the derivation for both its outputs and delete one of them.
+# simulates substitution or copying only one output from a builder.
+nix-store --delete $(nix-sandbox-build --no-out-link ./regression-fj883.nix -A base.lib)
+# build a derivation depending on previous one. this should succeed
+nix-sandbox-build --no-out-link ./regression-fj883.nix -A downstream
diff --git a/tests/functional/regression-fj883.nix b/tests/functional/regression-fj883.nix
new file mode 100644
index 000000000..2317145b7
--- /dev/null
+++ b/tests/functional/regression-fj883.nix
@@ -0,0 +1,15 @@
+with import ./config.nix;
+
+rec {
+  base = mkDerivation {
+    name = "base";
+    outputs = [ "out" "lib" ];
+    buildCommand = "echo > $out; echo > $lib";
+  };
+
+  downstream = mkDerivation {
+    name = "downstream";
+    deps = [ base.out base.lib ];
+    buildCommand = "echo $deps > $out";
+  };
+}