From 97a389b0bee7baf2d445121afa6ec84bef3a4bd7 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Fri, 26 Jul 2024 13:28:29 +0200 Subject: [PATCH] libstore: move Goal::getBuildResult to BuildResult there are no other uses for this yet, but asking for just a subset of outputs does seem at least somewhat useful to have as a generic thing Change-Id: I30ff5055a666c351b1b086b8d05b9d7c9fb1c77a --- src/libstore/build-result.cc | 23 +++++++++++++++++++++++ src/libstore/build-result.hh | 14 ++++++++++++++ src/libstore/build/derivation-goal.cc | 2 +- src/libstore/build/entry-points.cc | 7 ++----- src/libstore/build/goal.cc | 23 ----------------------- src/libstore/build/goal.hh | 13 ------------- 6 files changed, 40 insertions(+), 42 deletions(-) diff --git a/src/libstore/build-result.cc b/src/libstore/build-result.cc index 18f519c5c..de280c13f 100644 --- a/src/libstore/build-result.cc +++ b/src/libstore/build-result.cc @@ -15,4 +15,27 @@ GENERATE_CMP_EXT( me->cpuUser, me->cpuSystem); +KeyedBuildResult BuildResult::restrictTo(DerivedPath path) const +{ + KeyedBuildResult res{*this, std::move(path)}; + + if (auto pbp = std::get_if(&res.path)) { + auto & bp = *pbp; + + /* Because goals are in general shared between derived paths + that share the same derivation, we need to filter their + results to get back just the results we care about. + */ + + for (auto it = res.builtOutputs.begin(); it != res.builtOutputs.end();) { + if (bp.outputs.contains(it->first)) + ++it; + else + it = res.builtOutputs.erase(it); + } + } + + return res; +} + } diff --git a/src/libstore/build-result.hh b/src/libstore/build-result.hh index 8840fa7e3..9634fb944 100644 --- a/src/libstore/build-result.hh +++ b/src/libstore/build-result.hh @@ -11,6 +11,8 @@ namespace nix { +struct KeyedBuildResult; + struct BuildResult { /** @@ -112,6 +114,18 @@ struct BuildResult { throw Error("%s", errorMsg); } + + /** + * Project a BuildResult with just the information that pertains to + * the given path. + * + * A `BuildResult` may hold information for multiple derived paths; + * this function discards information about outputs not relevant in + * `path`. Build `Goal`s in particular may contain more outputs for + * a single build result than asked for directly, it's necessary to + * remove any such additional result to not leak other build infos. + */ + KeyedBuildResult restrictTo(DerivedPath path) const; }; /** diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index c0ca18310..38c54e854 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -1560,7 +1560,7 @@ void DerivationGoal::waiteeDone(GoalPtr waitee, ExitCode result) auto & outputs = nodeP->value; for (auto & outputName : outputs) { - auto buildResult = dg->getBuildResult(DerivedPath::Built { + auto buildResult = dg->buildResult.restrictTo(DerivedPath::Built { .drvPath = makeConstantStorePathRef(dg->drvPath), .outputs = OutputsSpec::Names { outputName }, }); diff --git a/src/libstore/build/entry-points.cc b/src/libstore/build/entry-points.cc index 67236bb39..c6955600e 100644 --- a/src/libstore/build/entry-points.cc +++ b/src/libstore/build/entry-points.cc @@ -62,10 +62,7 @@ std::vector Store::buildPathsWithResults( std::vector results; for (auto & [req, goalPtr] : state) - results.emplace_back(KeyedBuildResult { - goalPtr->getBuildResult(req), - /* .path = */ req, - }); + results.emplace_back(goalPtr->buildResult.restrictTo(req)); return results; } @@ -78,7 +75,7 @@ BuildResult Store::buildDerivation(const StorePath & drvPath, const BasicDerivat try { worker.run(Goals{goal}); - return goal->getBuildResult(DerivedPath::Built { + return goal->buildResult.restrictTo(DerivedPath::Built { .drvPath = makeConstantStorePathRef(drvPath), .outputs = OutputsSpec::All {}, }); diff --git a/src/libstore/build/goal.cc b/src/libstore/build/goal.cc index f4973efc9..4db6af6e6 100644 --- a/src/libstore/build/goal.cc +++ b/src/libstore/build/goal.cc @@ -11,29 +11,6 @@ bool CompareGoalPtrs::operator() (const GoalPtr & a, const GoalPtr & b) const { } -BuildResult Goal::getBuildResult(const DerivedPath & req) const { - BuildResult res { buildResult }; - - if (auto pbp = std::get_if(&req)) { - auto & bp = *pbp; - - /* Because goals are in general shared between derived paths - that share the same derivation, we need to filter their - results to get back just the results we care about. - */ - - for (auto it = res.builtOutputs.begin(); it != res.builtOutputs.end();) { - if (bp.outputs.contains(it->first)) - ++it; - else - it = res.builtOutputs.erase(it); - } - } - - return res; -} - - void Goal::addWaitee(GoalPtr waitee) { waitees.insert(waitee); diff --git a/src/libstore/build/goal.hh b/src/libstore/build/goal.hh index 94346531e..575621037 100644 --- a/src/libstore/build/goal.hh +++ b/src/libstore/build/goal.hh @@ -98,7 +98,6 @@ struct Goal : public std::enable_shared_from_this */ std::optional exitCode; -protected: /** * Build result. */ @@ -106,18 +105,6 @@ protected: public: - /** - * Project a `BuildResult` with just the information that pertains - * to the given request. - * - * In general, goals may be aliased between multiple requests, and - * the stored `BuildResult` has information for the union of all - * requests. We don't want to leak what the other request are for - * sake of both privacy and determinism, and this "safe accessor" - * ensures we don't. - */ - BuildResult getBuildResult(const DerivedPath &) const; - /** * Exception containing an error message, if any. */