From 5a8423720989ba845696d567713c04fdb291c35d Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 9 May 2025 00:32:41 +0200 Subject: [PATCH] Improve build failure error messages They're now laid out in a more readable way, and they shows the output paths (if known). --- src/libstore/build/derivation-goal.cc | 35 ++++++++++++++++--- .../store/build/derivation-building-misc.hh | 6 ++++ src/libstore/unix/build/derivation-builder.cc | 6 +++- tests/functional/build.sh | 15 ++++++-- 4 files changed, 54 insertions(+), 8 deletions(-) diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 344d1c7ea..d1a2dfd41 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -354,6 +354,22 @@ struct value_comparison }; +std::string showKnownOutputs(Store & store, const Derivation & drv) +{ + std::string msg; + StorePathSet expectedOutputPaths; + for (auto & i : drv.outputsAndOptPaths(store)) + if (i.second.second) + expectedOutputPaths.insert(*i.second.second); + if (!expectedOutputPaths.empty()) { + msg += "\nOutput paths:"; + for (auto & p : expectedOutputPaths) + msg += fmt("\n %s", Magenta(store.printStorePath(p))); + } + return msg; +} + + /* At least one of the output paths could not be produced using a substitute. So we have to build instead. */ Goal::Co DerivationGoal::gaveUpOnSubstitution() @@ -430,9 +446,14 @@ Goal::Co DerivationGoal::gaveUpOnSubstitution() if (nrFailed != 0) { if (!useDerivation) throw Error("some dependencies of '%s' are missing", worker.store.printStorePath(drvPath)); - co_return done(BuildResult::DependencyFailed, {}, Error( - "%s dependencies of derivation '%s' failed to build", - nrFailed, worker.store.printStorePath(drvPath))); + auto msg = fmt( + "Cannot build '%s'.\n" + "Reason: " ANSI_RED "%d %s failed" ANSI_NORMAL ".", + Magenta(worker.store.printStorePath(drvPath)), + nrFailed, + nrFailed == 1 ? "dependency" : "dependencies"); + msg += showKnownOutputs(worker.store, *drv); + co_return done(BuildResult::DependencyFailed, {}, Error(msg)); } if (retrySubstitution == RetrySubstitution::YesNeed) { @@ -1033,7 +1054,7 @@ void runPostBuildHook( void DerivationGoal::appendLogTailErrorMsg(std::string & msg) { if (!logger->isVerbose() && !logTail.empty()) { - msg += fmt(";\nlast %d log lines:\n", logTail.size()); + msg += fmt("\nLast %d log lines:\n", logTail.size()); for (auto & line : logTail) { msg += "> "; msg += line; @@ -1090,10 +1111,14 @@ Goal::Co DerivationGoal::hookDone() /* Check the exit status. */ if (!statusOk(status)) { - auto msg = fmt("builder for '%s' %s", + auto msg = fmt( + "Cannot build '%s'.\n" + "Reason: " ANSI_RED "builder %s" ANSI_NORMAL ".", Magenta(worker.store.printStorePath(drvPath)), statusToString(status)); + msg += showKnownOutputs(worker.store, *drv); + appendLogTailErrorMsg(msg); outputLocks.unlock(); diff --git a/src/libstore/include/nix/store/build/derivation-building-misc.hh b/src/libstore/include/nix/store/build/derivation-building-misc.hh index caf94844d..915d891d7 100644 --- a/src/libstore/include/nix/store/build/derivation-building-misc.hh +++ b/src/libstore/include/nix/store/build/derivation-building-misc.hh @@ -9,6 +9,7 @@ namespace nix { class Store; +struct Derivation; /** * Unless we are repairing, we don't both to test validity and just assume it, @@ -49,4 +50,9 @@ struct InitialOutput void runPostBuildHook(Store & store, Logger & logger, const StorePath & drvPath, const StorePathSet & outputPaths); +/** + * Format the known outputs of a derivation for use in error messages. + */ +std::string showKnownOutputs(Store & store, const Derivation & drv); + } diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index 4bde9750d..59c0be91c 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -585,10 +585,14 @@ std::variant, SingleDrvOutputs> Derivation diskFull |= cleanupDecideWhetherDiskFull(); - auto msg = fmt("builder for '%s' %s", + auto msg = fmt( + "Cannot build '%s'.\n" + "Reason: " ANSI_RED "builder %s" ANSI_NORMAL ".", Magenta(store.printStorePath(drvPath)), statusToString(status)); + msg += showKnownOutputs(store, drv); + miscMethods->appendLogTailErrorMsg(msg); if (diskFull) diff --git a/tests/functional/build.sh b/tests/functional/build.sh index 3f65a7c2c..0a19ff7da 100755 --- a/tests/functional/build.sh +++ b/tests/functional/build.sh @@ -179,12 +179,23 @@ test "$(<<<"$out" grep -cE '^error:')" = 4 out="$(nix build -f fod-failing.nix -L x4 2>&1)" && status=0 || status=$? test "$status" = 1 test "$(<<<"$out" grep -cE '^error:')" = 2 -<<<"$out" grepQuiet -E "error: 1 dependencies of derivation '.*-x4\\.drv' failed to build" + +if isDaemonNewer "2.29pre"; then + <<<"$out" grepQuiet -E "error: Cannot build '.*-x4\\.drv'" + <<<"$out" grepQuiet -E "Reason: 1 dependency failed." +else + <<<"$out" grepQuiet -E "error: 1 dependencies of derivation '.*-x4\\.drv' failed to build" +fi <<<"$out" grepQuiet -E "hash mismatch in fixed-output derivation '.*-x2\\.drv'" out="$(nix build -f fod-failing.nix -L x4 --keep-going 2>&1)" && status=0 || status=$? test "$status" = 1 test "$(<<<"$out" grep -cE '^error:')" = 3 -<<<"$out" grepQuiet -E "error: 2 dependencies of derivation '.*-x4\\.drv' failed to build" +if isDaemonNewer "2.29pre"; then + <<<"$out" grepQuiet -E "error: Cannot build '.*-x4\\.drv'" + <<<"$out" grepQuiet -E "Reason: 2 dependencies failed." +else + <<<"$out" grepQuiet -E "error: 2 dependencies of derivation '.*-x4\\.drv' failed to build" +fi <<<"$out" grepQuiet -vE "hash mismatch in fixed-output derivation '.*-x3\\.drv'" <<<"$out" grepQuiet -vE "hash mismatch in fixed-output derivation '.*-x2\\.drv'"