From a30bf96349604442265561ba305cb24793a09c79 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 29 Aug 2025 15:26:15 -0400 Subject: [PATCH] `DerivationBuildingGoal::initialOutputs` make local variable Also inline `assertPathValidity` in the process. --- .../build/derivation-building-goal.cc | 33 ++++++++++--------- .../store/build/derivation-building-goal.hh | 12 ++----- 2 files changed, 20 insertions(+), 25 deletions(-) diff --git a/src/libstore/build/derivation-building-goal.cc b/src/libstore/build/derivation-building-goal.cc index 95f0ee9d5..072bbfa93 100644 --- a/src/libstore/build/derivation-building-goal.cc +++ b/src/libstore/build/derivation-building-goal.cc @@ -417,7 +417,9 @@ Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution() Goal::Co DerivationBuildingGoal::tryToBuild() { - /* Recheck at goal start. In particular, whereas before we were + std::map initialOutputs; + + /* Recheck at this point. In particular, whereas before we were given this information by the downstream goal, that cannot happen anymore if the downstream goal only cares about one output, but we care about all outputs. */ @@ -440,7 +442,7 @@ Goal::Co DerivationBuildingGoal::tryToBuild() std::move(v), }); } - checkPathValidity(); + checkPathValidity(initialOutputs); auto started = [&]() { auto msg = @@ -528,7 +530,7 @@ Goal::Co DerivationBuildingGoal::tryToBuild() omitted, but that would be less efficient.) Note that since we now hold the locks on the output paths, no other process can build this derivation, so no further checks are necessary. */ - auto [allValid, validOutputs] = checkPathValidity(); + auto [allValid, validOutputs] = checkPathValidity(initialOutputs); if (buildMode != bmCheck && allValid) { debug("skipping build of derivation '%s', someone beat us to it", worker.store.printStorePath(drvPath)); @@ -556,7 +558,7 @@ Goal::Co DerivationBuildingGoal::tryToBuild() if (buildLocally) { useHook = false; } else { - switch (tryBuildHook()) { + switch (tryBuildHook(initialOutputs)) { case rpAccept: /* Yes, it has started doing so. Wait until we get EOF from the hook. */ @@ -644,8 +646,16 @@ Goal::Co DerivationBuildingGoal::tryToBuild() We can only early return when the outputs are known a priori. For floating content-addressing derivations this isn't the case. + + Aborts if any output is not valid or corrupt, and otherwise + returns a 'SingleDrvOutputs' structure containing all outputs. */ - assertPathValidity(); + [&] { + auto [allValid, validOutputs] = checkPathValidity(initialOutputs); + if (!allValid) + throw Error("some outputs are unexpectedly invalid"); + return validOutputs; + }(); StorePathSet outputPaths; for (auto & [_, output] : builtOutputs) @@ -960,7 +970,7 @@ BuildError DerivationBuildingGoal::fixupBuilderFailureErrorMessage(BuilderFailur return BuildError{e.status, msg}; } -HookReply DerivationBuildingGoal::tryBuildHook() +HookReply DerivationBuildingGoal::tryBuildHook(const std::map & initialOutputs) { #ifdef _WIN32 // TODO enable build hook on Windows return rpDecline; @@ -1239,7 +1249,8 @@ std::map> DerivationBuildingGoal::queryPar return res; } -std::pair DerivationBuildingGoal::checkPathValidity() +std::pair +DerivationBuildingGoal::checkPathValidity(std::map & initialOutputs) { if (drv->type().isImpure()) return {false, {}}; @@ -1296,14 +1307,6 @@ std::pair DerivationBuildingGoal::checkPathValidity() return {allValid, validOutputs}; } -SingleDrvOutputs DerivationBuildingGoal::assertPathValidity() -{ - auto [allValid, validOutputs] = checkPathValidity(); - if (!allValid) - throw Error("some outputs are unexpectedly invalid"); - return validOutputs; -} - Goal::Done DerivationBuildingGoal::doneSuccess(BuildResult::Status status, SingleDrvOutputs builtOutputs) { buildResult.status = status; 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 07f9b21ae..d394eb3c9 100644 --- a/src/libstore/include/nix/store/build/derivation-building-goal.hh +++ b/src/libstore/include/nix/store/build/derivation-building-goal.hh @@ -55,8 +55,6 @@ private: */ StorePathSet inputPaths; - std::map initialOutputs; - /** * File descriptor for the log file. */ @@ -108,7 +106,7 @@ private: /** * Is the build hook willing to perform the build? */ - HookReply tryBuildHook(); + HookReply tryBuildHook(const std::map & initialOutputs); /** * Open a log file and a pipe to it. @@ -142,13 +140,7 @@ private: * whether all outputs are valid and non-corrupt, and a * 'SingleDrvOutputs' structure containing the valid outputs. */ - std::pair checkPathValidity(); - - /** - * Aborts if any output is not valid or corrupt, and otherwise - * returns a 'SingleDrvOutputs' structure containing all outputs. - */ - SingleDrvOutputs assertPathValidity(); + std::pair checkPathValidity(std::map & initialOutputs); /** * Forcibly kill the child process, if any.