From 328a3bbbd0c8ec4a51b85a813f09a6d2064f993a Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 17 Oct 2025 12:03:18 -0400 Subject: [PATCH 1/2] Regression test for issue #14287 This will currently fail, until the bug is fixed. Co-Authored-By: Sergei Zimmerman (cherry picked from commit 246dbe1c0554deda6eda2fd89855237294dcca5e) --- tests/functional/build-hook-list-paths.sh | 12 ++++++++++++ tests/functional/post-hook.sh | 12 ++++++++++++ 2 files changed, 24 insertions(+) create mode 100755 tests/functional/build-hook-list-paths.sh diff --git a/tests/functional/build-hook-list-paths.sh b/tests/functional/build-hook-list-paths.sh new file mode 100755 index 000000000..03691c2d2 --- /dev/null +++ b/tests/functional/build-hook-list-paths.sh @@ -0,0 +1,12 @@ +#!/bin/sh + +set -x +set -e + +[ -n "$OUT_PATHS" ] +[ -n "$DRV_PATH" ] +[ -n "$HOOK_DEST" ] + +for o in $OUT_PATHS; do + echo "$o" >> "$HOOK_DEST" +done diff --git a/tests/functional/post-hook.sh b/tests/functional/post-hook.sh index 67bb46377..b16d8ab84 100755 --- a/tests/functional/post-hook.sh +++ b/tests/functional/post-hook.sh @@ -29,6 +29,18 @@ nix-build -o "$TEST_ROOT"/result dependencies.nix --post-build-hook "$pushToStor export BUILD_HOOK_ONLY_OUT_PATHS=$([ ! "$NIX_TESTS_CA_BY_DEFAULT" ]) nix-build -o "$TEST_ROOT"/result-mult multiple-outputs.nix -A a.first --post-build-hook "$pushToStore" +if isDaemonNewer "2.33.0pre20251029"; then + # Regression test for issue #14287: `--check` should re-run post build + # hook, even though nothing is getting newly registered. + export HOOK_DEST=$TEST_ROOT/listing + # Needed so the hook will get the above environment variable. + restartDaemon + nix-build -o "$TEST_ROOT"/result-mult multiple-outputs.nix --check -A a.first --post-build-hook "$PWD/build-hook-list-paths.sh" + grepQuiet a-first "$HOOK_DEST" + grepQuiet a-second "$HOOK_DEST" + unset HOOK_DEST +fi + clearStore # Ensure that the remote store contains both the runtime and build-time From 9e4177bc674733ed5cec32c6ffdd12ee3ca16ece Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 17 Oct 2025 13:07:41 -0400 Subject: [PATCH 2/2] Fix issue #14287 The test added in the previous commit now passes. Co-authored-by: Eelco Dolstra (cherry picked from commit de192794c9f5a4dbd4fb6735277140f2161d9d79) --- src/libstore/build/derivation-building-goal.cc | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/libstore/build/derivation-building-goal.cc b/src/libstore/build/derivation-building-goal.cc index 001816ca0..310b2bf84 100644 --- a/src/libstore/build/derivation-building-goal.cc +++ b/src/libstore/build/derivation-building-goal.cc @@ -860,7 +860,15 @@ Goal::Co DerivationBuildingGoal::tryToBuild() { builder.reset(); StorePathSet outputPaths; - for (auto & [_, output] : builtOutputs) { + /* In the check case we install no store objects, and so + `builtOutputs` is empty. However, per issue #14287, there is + an expectation that the post-build hook is still executed. + (This is useful for e.g. logging successful deterministic rebuilds.) + + In order to make that work, in the check case just load the + (preexisting) infos from scratch, rather than relying on what + `DerivationBuilder` returned to us. */ + for (auto & [_, output] : buildMode == bmCheck ? checkPathValidity(initialOutputs).second : builtOutputs) { // for sake of `bmRepair` worker.markContentsGood(output.outPath); outputPaths.insert(output.outPath);