From 99456a0c7e4df832b58d671e6a1b89f6110285de Mon Sep 17 00:00:00 2001 From: eldritch horrors 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 220ed5d26..10e572433 100644 --- a/lix/libstore/build/local-derivation-goal.cc +++ b/lix/libstore/build/local-derivation-goal.cc @@ -2119,6 +2119,11 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs() 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); @@ -2433,10 +2438,6 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs() the next iteration */ if (newInfo.ca) { 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)); @@ -2477,13 +2478,6 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs() infos2.insert_or_assign(newInfo.path, newInfo); } 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"; + }; +}