diff --git a/doc/manual/change-authors.yml b/doc/manual/change-authors.yml index 60c0924c7..e96bac80c 100644 --- a/doc/manual/change-authors.yml +++ b/doc/manual/change-authors.yml @@ -85,6 +85,10 @@ kloenk: forgejo: kloenk github: kloenk +lheckemann: + forgejo: lheckemann + github: lheckemann + lovesegfault: github: lovesegfault diff --git a/doc/manual/rl-next/report-differing-outputs.md b/doc/manual/rl-next/report-differing-outputs.md new file mode 100644 index 000000000..db8caf3bc --- /dev/null +++ b/doc/manual/rl-next/report-differing-outputs.md @@ -0,0 +1,22 @@ +--- +synopsis: "Reproducibility check builds now report all differing outputs" +cls: [2069] +category: Improvements +credits: [lheckemann] +--- + +`nix-build --check` allows rerunning the build of an already-built derivation to check that it produces the same output again. + +If a multiple-output derivation with impure behaviour is built with `--check`, only the first output would be shown in the resulting error message (and kept for comparison): + +``` +error: derivation '/nix/store/4spy3nz1661zm15gkybsy1h5f36aliwx-python3.11-test-1.0.0.drv' may not be deterministic: output '/nix/store/ccqcp01zg18wp9iadzmzimqzdi3ll08d-python3.11-test +-1.0.0-dist' differs from '/nix/store/ccqcp01zg18wp9iadzmzimqzdi3ll08d-python3.11-test-1.0.0-dist.check' +``` + +Now, all differing outputs are kept and reported: +``` +error: derivation '4spy3nz1661zm15gkybsy1h5f36aliwx-python3.11-test-1.0.0.drv' may not be deterministic: outputs differ + output differs: output '/nix/store/ccqcp01zg18wp9iadzmzimqzdi3ll08d-python3.11-test-1.0.0-dist' differs from '/nix/store/ccqcp01zg18wp9iadzmzimqzdi3ll08d-python3.11-test-1.0.0-dist.check' + output differs: output '/nix/store/yl59v08356i841c560alb0zmk7q16klb-python3.11-test-1.0.0' differs from '/nix/store/yl59v08356i841c560alb0zmk7q16klb-python3.11-test-1.0.0.check' +``` diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index c8c68f99f..77bfd099d 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -2021,6 +2021,8 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs() OutputPathMap finalOutputs; + std::vector>> nondeterministic; + for (auto & outputName : sortedOutputNames) { auto output = get(drv->outputs, outputName); auto scratchPath = get(scratchOutputs, outputName); @@ -2307,20 +2309,20 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs() buildUser ? buildUser->getGID() : getgid(), finalDestPath, dst, worker.store.printStorePath(drvPath), tmpDir); - throw NotDeterministic("derivation '%s' may not be deterministic: output '%s' differs from '%s'", - worker.store.printStorePath(drvPath), worker.store.toRealPath(finalDestPath), dst); + nondeterministic.push_back(std::make_pair(worker.store.toRealPath(finalDestPath), dst)); } else - throw NotDeterministic("derivation '%s' may not be deterministic: output '%s' differs", - worker.store.printStorePath(drvPath), worker.store.toRealPath(finalDestPath)); + nondeterministic.push_back(std::make_pair(worker.store.toRealPath(finalDestPath), std::nullopt)); } /* Since we verified the build, it's now ultimately trusted. */ - if (!oldInfo.ultimate) { + else if (!oldInfo.ultimate) { oldInfo.ultimate = true; localStore.signPathInfo(oldInfo); localStore.registerValidPaths({{oldInfo.path, oldInfo}}); } + /* Don't register anything, since we already have the + previous versions which we're comparing. */ continue; } @@ -2351,6 +2353,18 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs() } if (buildMode == bmCheck) { + if (!nondeterministic.empty()) { + std::ostringstream msg; + msg << HintFmt("derivation '%s' may not be deterministic: outputs differ", drvPath.to_string()); + for (auto [oldPath, newPath]: nondeterministic) { + if (newPath) { + msg << HintFmt("\n output differs: output '%s' differs from '%s'", oldPath.c_str(), *newPath); + } else { + msg << HintFmt("\n output '%s' differs", oldPath.c_str()); + } + } + throw NotDeterministic(msg.str()); + } /* In case of fixed-output derivations, if there are mismatches on `--check` an error must be thrown as this is also a source for non-determinism. */ diff --git a/tests/functional/check.nix b/tests/functional/check.nix index ddab8eea9..7b18d0b4d 100644 --- a/tests/functional/check.nix +++ b/tests/functional/check.nix @@ -6,10 +6,12 @@ with import ./config.nix; nondeterministic = mkDerivation { inherit checkBuildId; name = "nondeterministic"; + outputs = ["a" "b"]; buildCommand = '' - mkdir $out - date +%s.%N > $out/date + mkdir $a $b + date +%s.%N > $a/date + date +%s.%N > $b/date echo "CHECK_TMPDIR=$TMPDIR" echo "checkBuildId=$checkBuildId" echo "$checkBuildId" > $TMPDIR/checkBuildId @@ -19,10 +21,12 @@ with import ./config.nix; deterministic = mkDerivation { inherit checkBuildId; name = "deterministic"; + outputs = ["a" "b"]; buildCommand = '' - mkdir $out - echo date > $out/date + mkdir $a $b + echo date > $a/date + echo date > $b/date echo "CHECK_TMPDIR=$TMPDIR" echo "checkBuildId=$checkBuildId" echo "$checkBuildId" > $TMPDIR/checkBuildId @@ -32,10 +36,12 @@ with import ./config.nix; failed = mkDerivation { inherit checkBuildId; name = "failed"; + outputs = ["a" "b"]; buildCommand = '' - mkdir $out - echo date > $out/date + mkdir $a $b + echo date > $a/date + echo date > $b/date echo "CHECK_TMPDIR=$TMPDIR" echo "checkBuildId=$checkBuildId" echo "$checkBuildId" > $TMPDIR/checkBuildId diff --git a/tests/functional/check.sh b/tests/functional/check.sh index 38883c5d7..90ab61968 100644 --- a/tests/functional/check.sh +++ b/tests/functional/check.sh @@ -65,6 +65,8 @@ checkBuildTempDirRemoved $TEST_ROOT/log nix-build check.nix -A nondeterministic --argstr checkBuildId $checkBuildId \ --no-out-link --check 2> $TEST_ROOT/log || status=$? grep 'may not be deterministic' $TEST_ROOT/log +# the differences in both outputs should be reported +[[ $(grep -c 'differs' $TEST_ROOT/log) = 2 ]] [ "$status" = "104" ] checkBuildTempDirRemoved $TEST_ROOT/log