From c37df9c87c5170b1736337fd43a80ce54550d09c Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 9 Jul 2025 17:10:05 -0400 Subject: [PATCH 1/2] Inline `DerivationGoal::query{,Partial}DerivationOutputMap` The functions are used just once. --- src/libstore/build/derivation-goal.cc | 58 ++++++++----------- .../nix/store/build/derivation-goal.hh | 8 --- 2 files changed, 25 insertions(+), 41 deletions(-) diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index b2faa7a6e..7eb5bd09d 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -241,7 +241,17 @@ Goal::Co DerivationGoal::repairClosure() that produced those outputs. */ /* Get the output closure. */ - auto outputs = queryDerivationOutputMap(); + auto outputs = [&] { + for (auto * drvStore : {&worker.evalStore, &worker.store}) + if (drvStore->isValidPath(drvPath)) + return worker.store.queryDerivationOutputMap(drvPath, drvStore); + + OutputPathMap res; + for (auto & [name, output] : drv->outputsAndOptPaths(worker.store)) + res.insert_or_assign(name, *output.second); + return res; + }(); + StorePathSet outputClosure; if (auto mPath = get(outputs, wantedOutput)) { worker.store.computeFSClosure(*mPath, outputClosure); @@ -304,37 +314,6 @@ Goal::Co DerivationGoal::repairClosure() co_return done(BuildResult::AlreadyValid, assertPathValidity()); } -std::map> DerivationGoal::queryPartialDerivationOutputMap() -{ - assert(!drv->type().isImpure()); - - for (auto * drvStore : {&worker.evalStore, &worker.store}) - if (drvStore->isValidPath(drvPath)) - return worker.store.queryPartialDerivationOutputMap(drvPath, drvStore); - - /* In-memory derivation will naturally fall back on this case, where - we do best-effort with static information. */ - std::map> res; - for (auto & [name, output] : drv->outputs) - res.insert_or_assign(name, output.path(worker.store, drv->name, name)); - return res; -} - -OutputPathMap DerivationGoal::queryDerivationOutputMap() -{ - assert(!drv->type().isImpure()); - - for (auto * drvStore : {&worker.evalStore, &worker.store}) - if (drvStore->isValidPath(drvPath)) - return worker.store.queryDerivationOutputMap(drvPath, drvStore); - - // See comment in `DerivationGoal::queryPartialDerivationOutputMap`. - OutputPathMap res; - for (auto & [name, output] : drv->outputsAndOptPaths(worker.store)) - res.insert_or_assign(name, *output.second); - return res; -} - std::pair DerivationGoal::checkPathValidity() { if (drv->type().isImpure()) @@ -344,7 +323,20 @@ std::pair DerivationGoal::checkPathValidity() StringSet wantedOutputsLeft{wantedOutput}; SingleDrvOutputs validOutputs; - for (auto & i : queryPartialDerivationOutputMap()) { + auto partialDerivationOutputMap = [&] { + for (auto * drvStore : {&worker.evalStore, &worker.store}) + if (drvStore->isValidPath(drvPath)) + return worker.store.queryPartialDerivationOutputMap(drvPath, drvStore); + + /* In-memory derivation will naturally fall back on this case, where + we do best-effort with static information. */ + std::map> res; + for (auto & [name, output] : drv->outputs) + res.insert_or_assign(name, output.path(worker.store, drv->name, name)); + return res; + }(); + + for (auto & i : partialDerivationOutputMap) { auto initialOutput = get(initialOutputs, i.first); if (!initialOutput) // this is an invalid output, gets caught with (!wantedOutputsLeft.empty()) diff --git a/src/libstore/include/nix/store/build/derivation-goal.hh b/src/libstore/include/nix/store/build/derivation-goal.hh index d78073a91..5d5c8d131 100644 --- a/src/libstore/include/nix/store/build/derivation-goal.hh +++ b/src/libstore/include/nix/store/build/derivation-goal.hh @@ -78,14 +78,6 @@ struct DerivationGoal : public Goal */ Co haveDerivation(); - /** - * Wrappers around the corresponding Store methods that first consult the - * derivation. This is currently needed because when there is no drv file - * there also is no DB entry. - */ - std::map> queryPartialDerivationOutputMap(); - OutputPathMap queryDerivationOutputMap(); - /** * Update 'initialOutputs' to determine the current status of the * outputs of the derivation. Also returns a Boolean denoting From ed5593700260edcd6882e5c713670252e920cbe2 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 9 Jul 2025 17:15:32 -0400 Subject: [PATCH 2/2] Make many members of `DerivationGoal` private --- .../nix/store/build/derivation-goal.hh | 42 ++++++++++--------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/src/libstore/include/nix/store/build/derivation-goal.hh b/src/libstore/include/nix/store/build/derivation-goal.hh index 5d5c8d131..4b004e73b 100644 --- a/src/libstore/include/nix/store/build/derivation-goal.hh +++ b/src/libstore/include/nix/store/build/derivation-goal.hh @@ -43,21 +43,6 @@ struct DerivationGoal : public Goal */ OutputName wantedOutput; - /** - * The derivation stored at drvPath. - */ - std::unique_ptr drv; - - /** - * The remainder is state held during the build. - */ - - std::map initialOutputs; - - BuildMode buildMode; - - std::unique_ptr> mcExpectedBuilds; - DerivationGoal( const StorePath & drvPath, const Derivation & drv, @@ -73,6 +58,28 @@ struct DerivationGoal : public Goal std::string key() override; + JobCategory jobCategory() const override + { + return JobCategory::Administration; + }; + +private: + + /** + * The derivation stored at drvPath. + */ + std::unique_ptr drv; + + /** + * The remainder is state held during the build. + */ + + std::map initialOutputs; + + BuildMode buildMode; + + std::unique_ptr> mcExpectedBuilds; + /** * The states. */ @@ -95,11 +102,6 @@ struct DerivationGoal : public Goal Co repairClosure(); Done done(BuildResult::Status status, SingleDrvOutputs builtOutputs = {}, std::optional ex = {}); - - JobCategory jobCategory() const override - { - return JobCategory::Administration; - }; }; } // namespace nix