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

@@ -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

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

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";
+  };
+}