From a63ac8d98b005937e0b65389c9a40dd953b90888 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 29 Aug 2025 15:07:01 -0400 Subject: [PATCH] Inline `DerivationBuildingGoal::hookDone` --- .../build/derivation-building-goal.cc | 146 +++++++++--------- .../store/build/derivation-building-goal.hh | 1 - 2 files changed, 71 insertions(+), 76 deletions(-) diff --git a/src/libstore/build/derivation-building-goal.cc b/src/libstore/build/derivation-building-goal.cc index 510304653..5493845a5 100644 --- a/src/libstore/build/derivation-building-goal.cc +++ b/src/libstore/build/derivation-building-goal.cc @@ -584,7 +584,77 @@ Goal::Co DerivationBuildingGoal::tryToBuild() buildResult.startTime = time(0); // inexact started(); co_await Suspend{}; - co_return hookDone(); + +#ifndef _WIN32 + assert(hook); +#endif + + trace("hook build done"); + + /* Since we got an EOF on the logger pipe, the builder is presumed + to have terminated. In fact, the builder could also have + simply have closed its end of the pipe, so just to be sure, + kill it. */ + int status = +#ifndef _WIN32 // TODO enable build hook on Windows + hook->pid.kill(); +#else + 0; +#endif + + debug("build hook for '%s' finished", worker.store.printStorePath(drvPath)); + + buildResult.timesBuilt++; + buildResult.stopTime = time(0); + + /* So the child is gone now. */ + worker.childTerminated(this); + + /* Close the read side of the logger pipe. */ +#ifndef _WIN32 // TODO enable build hook on Windows + hook->builderOut.readSide.close(); + hook->fromHook.readSide.close(); +#endif + + /* Close the log file. */ + closeLogFile(); + + /* Check the exit status. */ + if (!statusOk(status)) { + auto e = fixupBuilderFailureErrorMessage({BuildResult::MiscFailure, status, ""}); + + outputLocks.unlock(); + + /* TODO (once again) support fine-grained error codes, see issue #12641. */ + + co_return doneFailure(std::move(e)); + } + + /* Compute the FS closure of the outputs and register them as + being valid. */ + auto builtOutputs = + /* When using a build hook, the build hook can register the output + as valid (by doing `nix-store --import'). If so we don't have + to do anything here. + + We can only early return when the outputs are known a priori. For + floating content-addressing derivations this isn't the case. + */ + assertPathValidity(); + + StorePathSet outputPaths; + for (auto & [_, output] : builtOutputs) + outputPaths.insert(output.outPath); + runPostBuildHook(worker.store, *logger, drvPath, outputPaths); + + /* It is now safe to delete the lock files, since all future + lockers will see that the output paths are valid; they will + not create new lock files with the same names as the old + (unlinked) lock files. */ + outputLocks.setDeletion(true); + outputLocks.unlock(); + + co_return doneSuccess(BuildResult::Built, std::move(builtOutputs)); } co_await yield(); @@ -885,80 +955,6 @@ BuildError DerivationBuildingGoal::fixupBuilderFailureErrorMessage(BuilderFailur return BuildError{e.status, msg}; } -Goal::Co DerivationBuildingGoal::hookDone() -{ -#ifndef _WIN32 - assert(hook); -#endif - - trace("hook build done"); - - /* Since we got an EOF on the logger pipe, the builder is presumed - to have terminated. In fact, the builder could also have - simply have closed its end of the pipe, so just to be sure, - kill it. */ - int status = -#ifndef _WIN32 // TODO enable build hook on Windows - hook->pid.kill(); -#else - 0; -#endif - - debug("build hook for '%s' finished", worker.store.printStorePath(drvPath)); - - buildResult.timesBuilt++; - buildResult.stopTime = time(0); - - /* So the child is gone now. */ - worker.childTerminated(this); - - /* Close the read side of the logger pipe. */ -#ifndef _WIN32 // TODO enable build hook on Windows - hook->builderOut.readSide.close(); - hook->fromHook.readSide.close(); -#endif - - /* Close the log file. */ - closeLogFile(); - - /* Check the exit status. */ - if (!statusOk(status)) { - auto e = fixupBuilderFailureErrorMessage({BuildResult::MiscFailure, status, ""}); - - outputLocks.unlock(); - - /* TODO (once again) support fine-grained error codes, see issue #12641. */ - - co_return doneFailure(std::move(e)); - } - - /* Compute the FS closure of the outputs and register them as - being valid. */ - auto builtOutputs = - /* When using a build hook, the build hook can register the output - as valid (by doing `nix-store --import'). If so we don't have - to do anything here. - - We can only early return when the outputs are known a priori. For - floating content-addressing derivations this isn't the case. - */ - assertPathValidity(); - - StorePathSet outputPaths; - for (auto & [_, output] : builtOutputs) - outputPaths.insert(output.outPath); - runPostBuildHook(worker.store, *logger, drvPath, outputPaths); - - /* It is now safe to delete the lock files, since all future - lockers will see that the output paths are valid; they will - not create new lock files with the same names as the old - (unlinked) lock files. */ - outputLocks.setDeletion(true); - outputLocks.unlock(); - - co_return doneSuccess(BuildResult::Built, std::move(builtOutputs)); -} - HookReply DerivationBuildingGoal::tryBuildHook() { #ifdef _WIN32 // TODO enable build hook on Windows 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 dd8b27dc2..041abfad2 100644 --- a/src/libstore/include/nix/store/build/derivation-building-goal.hh +++ b/src/libstore/include/nix/store/build/derivation-building-goal.hh @@ -112,7 +112,6 @@ struct DerivationBuildingGoal : public Goal */ Co gaveUpOnSubstitution(); Co tryToBuild(); - Co hookDone(); /** * Is the build hook willing to perform the build?