diff --git a/doc/manual/rl-next/mismatch-check-path.md b/doc/manual/rl-next/mismatch-check-path.md new file mode 100644 index 000000000..a0e2ba95d --- /dev/null +++ b/doc/manual/rl-next/mismatch-check-path.md @@ -0,0 +1,14 @@ +--- +synopsis: Better debuggability on fixed-output hash mismatches +issues: [] +cls: [] +category: Improvements +credits: [lheckemann] +--- + +Fixed-output derivation hash mismatch error messages will now include the path that was +produced unexpectedly, and this path will be registered as valid even if `--check` +(`nix-store`, `nix-build`) or `--rebuild` (`nix build`) was passed. This makes comparing +the expected path with the obtained path easier, and is useful for debugging when +upstreams modify previously-published releases or when changes in fixed-output +derivations' dependencies affect their output unexpectedly. diff --git a/lix/libstore/build/local-derivation-goal.cc b/lix/libstore/build/local-derivation-goal.cc index 79e63b149..1577b2476 100644 --- a/lix/libstore/build/local-derivation-goal.cc +++ b/lix/libstore/build/local-derivation-goal.cc @@ -1855,11 +1855,13 @@ try { // XXX: shameless layering violation hack that makes the hash mismatch error at least not utterly worthless auto guessedUrl = getOr(drv->env, "urls", getOr(drv->env, "url", "(unknown)")); delayedException = std::make_exception_ptr( - BuildError("hash mismatch in fixed-output derivation '%s':\n likely URL: %s\n specified: %s\n got: %s", + BuildError("hash mismatch in fixed-output derivation '%s':\n likely URL: %s\n specified: %s\n got: %s\n expected path: %s\n got path: %s", worker.store.printStorePath(drvPath), guessedUrl, wanted.to_string(Base::SRI, true), - got.to_string(Base::SRI, true))); + got.to_string(Base::SRI, true), + worker.store.printStorePath(dof.path(worker.store, drv->name, outputName)), + worker.store.printStorePath(newInfo0.path))); } if (!newInfo0.references.empty()) { std::string references; @@ -1918,9 +1920,10 @@ try { /* Path already exists, need to replace it */ replaceValidPath(worker.store.toRealPath(finalDestPath), actualPath); actualPath = worker.store.toRealPath(finalDestPath); - } else if (buildMode == bmCheck) { - /* Path already exists, and we want to compare, so we leave out - new path in place. */ + } else if (buildMode == bmCheck && TRY_AWAIT(worker.store.isValidPath(newInfo.path))) { + /* Path already exists, and we want to compare, so we + don't replace the previously existing output with + the new one. */ } else if (TRY_AWAIT(worker.store.isValidPath(newInfo.path))) { /* Path already exists because CA path produced by something else. No moving needed. */ @@ -1935,8 +1938,12 @@ try { auto & localStore = getLocalStore(); - if (buildMode == bmCheck) { + // Check determinism and run the diff hook for input-addressed + // paths if we're in check mode. + // TODO: implement this for content-addressed paths too. + if (buildMode == bmCheck && !newInfo.ca) { + // We can only do this if we have a previous output path to compare. if (!TRY_AWAIT(worker.store.isValidPath(newInfo.path))) continue; ValidPathInfo oldInfo(*TRY_AWAIT(worker.store.queryPathInfo(newInfo.path))); if (newInfo.narHash != oldInfo.narHash) { @@ -2007,12 +2014,11 @@ try { } 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. */ - if (delayedException) - std::rethrow_exception(delayedException); - co_return TRY_AWAIT(assertPathValidity()); + /* In case of fixed-output derivations with hash mismatches, + we don't want to rethrow the exception until later so that + the unexpected path is still registered as valid. */ + if (!delayedException) + co_return TRY_AWAIT(assertPathValidity()); } /* Apply output checks. */ diff --git a/tests/functional/check.sh b/tests/functional/check.sh index 653563971..fc63b9e21 100644 --- a/tests/functional/check.sh +++ b/tests/functional/check.sh @@ -136,15 +136,27 @@ nix-build check.nix -A fetchurl --no-out-link --repair [[ $(cat $path) != foo ]] echo 'Hello World' > $TEST_ROOT/dummy -nix-build check.nix -A hashmismatch --no-out-link || status=$? +nix-build check.nix -A hashmismatch --no-out-link 2>mismatch-output || status=$? [ "$status" = "102" ] +obtained=$(grep "got path:" mismatch-output | awk '{ print $3 }') +# The path that actually came out should exist +[ -e "$obtained" ] +# and be registered as valid +nix-store -q --references "$obtained" >/dev/null echo -n > $TEST_ROOT/dummy -nix-build check.nix -A hashmismatch --no-out-link -echo 'Hello World' > $TEST_ROOT/dummy +successful_fod=$(nix-build check.nix -A hashmismatch --no-out-link) -nix-build check.nix -A hashmismatch --no-out-link --check || status=$? +echo 'Hello World 2' > $TEST_ROOT/dummy +nix-build check.nix -A hashmismatch --no-out-link --check 2>mismatch-output || status=$? +cat mismatch-output >&2 [ "$status" = "102" ] +grep -E "expected path:\s+$successful_fod" mismatch-output +obtained=$(grep "got path:" mismatch-output | awk '{ print $3 }') +# The path that actually came out should exist +[ -e "$obtained" ] +# and be registered as valid +nix-store -q --references "$obtained" >/dev/null # Multiple failures with --keep-going nix-build check.nix -A nondeterministic --no-out-link