diff --git a/src/libstore/build/derivation-building-goal.cc b/src/libstore/build/derivation-building-goal.cc index 89dfcc6d9..af7c25e52 100644 --- a/src/libstore/build/derivation-building-goal.cc +++ b/src/libstore/build/derivation-building-goal.cc @@ -633,11 +633,6 @@ Goal::Co DerivationBuildingGoal::tryToBuild() { goal.closeLogFile(); } - - void appendLogTailErrorMsg(std::string & msg) override - { - goal.appendLogTailErrorMsg(msg); - } }; auto * localStoreP = dynamic_cast(&worker.store); @@ -725,6 +720,9 @@ Goal::Co DerivationBuildingGoal::tryToBuild() SingleDrvOutputs builtOutputs; try { builtOutputs = builder->unprepareBuild(); + } catch (BuilderFailureError & e) { + outputLocks.unlock(); + co_return doneFailure(fixupBuilderFailureErrorMessage(std::move(e))); } catch (BuildError & e) { outputLocks.unlock(); // Allow selecting a subset of enum values @@ -835,8 +833,16 @@ static void runPostBuildHook( }); } -void DerivationBuildingGoal::appendLogTailErrorMsg(std::string & msg) +BuildError DerivationBuildingGoal::fixupBuilderFailureErrorMessage(BuilderFailureError e) { + auto msg = + fmt("Cannot build '%s'.\n" + "Reason: " ANSI_RED "builder %s" ANSI_NORMAL ".", + Magenta(worker.store.printStorePath(drvPath)), + statusToString(e.builderStatus)); + + msg += showKnownOutputs(worker.store, *drv); + if (!logger->isVerbose() && !logTail.empty()) { msg += fmt("\nLast %d log lines:\n", logTail.size()); for (auto & line : logTail) { @@ -853,6 +859,10 @@ void DerivationBuildingGoal::appendLogTailErrorMsg(std::string & msg) nixLogCommand, worker.store.printStorePath(drvPath)); } + + msg += e.extraMsgAfter; + + return BuildError{e.status, msg}; } Goal::Co DerivationBuildingGoal::hookDone() @@ -893,21 +903,13 @@ Goal::Co DerivationBuildingGoal::hookDone() /* Check the exit status. */ if (!statusOk(status)) { - 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); + auto e = fixupBuilderFailureErrorMessage({BuildResult::MiscFailure, status, ""}); outputLocks.unlock(); /* TODO (once again) support fine-grained error codes, see issue #12641. */ - co_return doneFailure(BuildError{BuildResult::MiscFailure, msg}); + co_return doneFailure(std::move(e)); } /* Compute the FS closure of the outputs and register them as diff --git a/src/libstore/include/nix/store/build/derivation-builder.hh b/src/libstore/include/nix/store/build/derivation-builder.hh index 0338911c4..45fbba3f5 100644 --- a/src/libstore/include/nix/store/build/derivation-builder.hh +++ b/src/libstore/include/nix/store/build/derivation-builder.hh @@ -12,6 +12,29 @@ namespace nix { +/** + * Denotes a build failure that stemmed from the builder exiting with a + * failing exist status. + */ +struct BuilderFailureError : BuildError +{ + int builderStatus; + + std::string extraMsgAfter; + + BuilderFailureError(BuildResult::Status status, int builderStatus, std::string extraMsgAfter) + : BuildError{ + status, + /* No message for now, because the caller will make for + us, with extra context */ + "", + } + , builderStatus{std::move(builderStatus)} + , extraMsgAfter{std::move(extraMsgAfter)} + { + } +}; + /** * Stuff we need to pass to initChild(). */ @@ -94,8 +117,6 @@ struct DerivationBuilderCallbacks */ virtual void closeLogFile() = 0; - virtual void appendLogTailErrorMsg(std::string & msg) = 0; - /** * Hook up `builderOut` to some mechanism to ingest the log * diff --git a/src/libstore/include/nix/store/build/derivation-building-goal.hh b/src/libstore/include/nix/store/build/derivation-building-goal.hh index 38f0fc7bf..162cf14ad 100644 --- a/src/libstore/include/nix/store/build/derivation-building-goal.hh +++ b/src/libstore/include/nix/store/build/derivation-building-goal.hh @@ -14,6 +14,7 @@ namespace nix { using std::map; +struct BuilderFailureError; #ifndef _WIN32 // TODO enable build hook on Windows struct HookInstance; struct DerivationBuilder; @@ -174,7 +175,7 @@ struct DerivationBuildingGoal : public Goal Done doneFailure(BuildError ex); - void appendLogTailErrorMsg(std::string & msg); + BuildError fixupBuilderFailureErrorMessage(BuilderFailureError msg); JobCategory jobCategory() const override { diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index 62864557a..80f73801d 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -535,26 +535,16 @@ SingleDrvOutputs DerivationBuilderImpl::unprepareBuild() /* Check the exit status. */ if (!statusOk(status)) { + /* Check *before* cleaning up. */ bool diskFull = decideWhetherDiskFull(); cleanupBuild(false); - 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) - msg += "\nnote: build failure may have been caused by lack of free disk space"; - - throw BuildError( + throw BuilderFailureError{ !derivationType.isSandboxed() || diskFull ? BuildResult::TransientFailure : BuildResult::PermanentFailure, - msg); + status, + diskFull ? "\nnote: build failure may have been caused by lack of free disk space" : "", + }; } /* Compute the FS closure of the outputs and register them as