diff options
Diffstat (limited to 'src/libstore')
-rw-r--r-- | src/libstore/build-result.hh | 18 | ||||
-rw-r--r-- | src/libstore/build/derivation-goal.cc | 93 | ||||
-rw-r--r-- | src/libstore/build/derivation-goal.hh | 62 | ||||
-rw-r--r-- | src/libstore/build/entry-points.cc | 47 | ||||
-rw-r--r-- | src/libstore/build/goal.cc | 23 | ||||
-rw-r--r-- | src/libstore/build/goal.hh | 16 | ||||
-rw-r--r-- | src/libstore/build/local-derivation-goal.cc | 8 | ||||
-rw-r--r-- | src/libstore/build/local-derivation-goal.hh | 2 | ||||
-rw-r--r-- | src/libstore/build/worker.cc | 15 | ||||
-rw-r--r-- | src/libstore/build/worker.hh | 12 | ||||
-rw-r--r-- | src/libstore/daemon.cc | 7 | ||||
-rw-r--r-- | src/libstore/globals.cc | 27 | ||||
-rw-r--r-- | src/libstore/globals.hh | 4 | ||||
-rw-r--r-- | src/libstore/legacy-ssh-store.cc | 15 | ||||
-rw-r--r-- | src/libstore/realisation.hh | 33 | ||||
-rw-r--r-- | src/libstore/remote-store.cc | 70 | ||||
-rw-r--r-- | src/libstore/remote-store.hh | 2 | ||||
-rw-r--r-- | src/libstore/store-api.hh | 3 | ||||
-rw-r--r-- | src/libstore/worker-protocol.hh | 1 |
19 files changed, 331 insertions, 127 deletions
diff --git a/src/libstore/build-result.hh b/src/libstore/build-result.hh index 27d1a1b6c..b7a56e791 100644 --- a/src/libstore/build-result.hh +++ b/src/libstore/build-result.hh @@ -84,15 +84,10 @@ struct BuildResult bool isNonDeterministic = false; /** - * The derivation we built or the store path we substituted. - */ - DerivedPath path; - - /** * For derivations, a mapping from the names of the wanted outputs * to actual paths. */ - DrvOutputs builtOutputs; + SingleDrvOutputs builtOutputs; /** * The start/stop times of the build (or one of the rounds, if it @@ -116,4 +111,15 @@ struct BuildResult } }; +/** + * A `BuildResult` together with its "primary key". + */ +struct KeyedBuildResult : BuildResult +{ + /** + * The derivation we built or the store path we substituted. + */ + DerivedPath path; +}; + } diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 26faf8c8e..a4bb94b0e 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -145,8 +145,20 @@ void DerivationGoal::work() void DerivationGoal::addWantedOutputs(const OutputsSpec & outputs) { auto newWanted = wantedOutputs.union_(outputs); - if (!newWanted.isSubsetOf(wantedOutputs)) - needRestart = true; + switch (needRestart) { + case NeedRestartForMoreOutputs::OutputsUnmodifedDontNeed: + if (!newWanted.isSubsetOf(wantedOutputs)) + needRestart = NeedRestartForMoreOutputs::OutputsAddedDoNeed; + break; + case NeedRestartForMoreOutputs::OutputsAddedDoNeed: + /* No need to check whether we added more outputs, because a + restart is already queued up. */ + break; + case NeedRestartForMoreOutputs::BuildInProgressWillNotNeed: + /* We are already building all outputs, so it doesn't matter if + we now want more. */ + break; + }; wantedOutputs = newWanted; } @@ -297,12 +309,29 @@ void DerivationGoal::outputsSubstitutionTried() In particular, it may be the case that the hole in the closure is an output of the current derivation, which causes a loop if retried. */ - if (nrIncompleteClosure > 0 && nrIncompleteClosure == nrFailed) retrySubstitution = true; + { + bool substitutionFailed = + nrIncompleteClosure > 0 && + nrIncompleteClosure == nrFailed; + switch (retrySubstitution) { + case RetrySubstitution::NoNeed: + if (substitutionFailed) + retrySubstitution = RetrySubstitution::YesNeed; + break; + case RetrySubstitution::YesNeed: + // Should not be able to reach this state from here. + assert(false); + break; + case RetrySubstitution::AlreadyRetried: + debug("substitution failed again, but we already retried once. Not retrying again."); + break; + } + } nrFailed = nrNoSubstituters = nrIncompleteClosure = 0; - if (needRestart) { - needRestart = false; + if (needRestart == NeedRestartForMoreOutputs::OutputsAddedDoNeed) { + needRestart = NeedRestartForMoreOutputs::OutputsUnmodifedDontNeed; haveDerivation(); return; } @@ -330,6 +359,10 @@ void DerivationGoal::outputsSubstitutionTried() produced using a substitute. So we have to build instead. */ void DerivationGoal::gaveUpOnSubstitution() { + /* At this point we are building all outputs, so if more are wanted there + is no need to restart. */ + needRestart = NeedRestartForMoreOutputs::BuildInProgressWillNotNeed; + /* The inputs must be built before we can build this goal. */ inputDrvOutputs.clear(); if (useDerivation) @@ -451,8 +484,8 @@ void DerivationGoal::inputsRealised() return; } - if (retrySubstitution && !retriedSubstitution) { - retriedSubstitution = true; + if (retrySubstitution == RetrySubstitution::YesNeed) { + retrySubstitution = RetrySubstitution::AlreadyRetried; haveDerivation(); return; } @@ -570,8 +603,6 @@ void DerivationGoal::inputsRealised() build hook. */ state = &DerivationGoal::tryToBuild; worker.wakeUp(shared_from_this()); - - buildResult = BuildResult { .path = buildResult.path }; } void DerivationGoal::started() @@ -982,7 +1013,7 @@ void DerivationGoal::resolvedFinished() auto resolvedDrv = *resolvedDrvGoal->drv; auto & resolvedResult = resolvedDrvGoal->buildResult; - DrvOutputs builtOutputs; + SingleDrvOutputs builtOutputs; if (resolvedResult.success()) { auto resolvedHashes = staticOutputHashes(worker.store, resolvedDrv); @@ -1008,7 +1039,7 @@ void DerivationGoal::resolvedFinished() worker.store.printStorePath(drvPath), wantedOutput); auto realisation = [&]{ - auto take1 = get(resolvedResult.builtOutputs, DrvOutput { *resolvedHash, wantedOutput }); + auto take1 = get(resolvedResult.builtOutputs, wantedOutput); if (take1) return *take1; /* The above `get` should work. But sateful tracking of @@ -1033,7 +1064,7 @@ void DerivationGoal::resolvedFinished() worker.store.registerDrvOutput(newRealisation); } outputPaths.insert(realisation.outPath); - builtOutputs.emplace(realisation.id, realisation); + builtOutputs.emplace(wantedOutput, realisation); } runPostBuildHook( @@ -1158,7 +1189,7 @@ HookReply DerivationGoal::tryBuildHook() } -DrvOutputs DerivationGoal::registerOutputs() +SingleDrvOutputs DerivationGoal::registerOutputs() { /* 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 @@ -1320,7 +1351,7 @@ OutputPathMap DerivationGoal::queryDerivationOutputMap() } -std::pair<bool, DrvOutputs> DerivationGoal::checkPathValidity() +std::pair<bool, SingleDrvOutputs> DerivationGoal::checkPathValidity() { if (!drv->type().isPure()) return { false, {} }; @@ -1333,7 +1364,7 @@ std::pair<bool, DrvOutputs> DerivationGoal::checkPathValidity() return static_cast<StringSet>(names); }, }, wantedOutputs.raw()); - DrvOutputs validOutputs; + SingleDrvOutputs validOutputs; for (auto & i : queryPartialDerivationOutputMap()) { auto initialOutput = get(initialOutputs, i.first); @@ -1376,7 +1407,7 @@ std::pair<bool, DrvOutputs> DerivationGoal::checkPathValidity() } } if (info.wanted && info.known && info.known->isValid()) - validOutputs.emplace(drvOutput, Realisation { drvOutput, info.known->path }); + validOutputs.emplace(i.first, Realisation { drvOutput, info.known->path }); } // If we requested all the outputs, we are always fine. @@ -1400,7 +1431,7 @@ std::pair<bool, DrvOutputs> DerivationGoal::checkPathValidity() } -DrvOutputs DerivationGoal::assertPathValidity() +SingleDrvOutputs DerivationGoal::assertPathValidity() { auto [allValid, validOutputs] = checkPathValidity(); if (!allValid) @@ -1411,7 +1442,7 @@ DrvOutputs DerivationGoal::assertPathValidity() void DerivationGoal::done( BuildResult::Status status, - DrvOutputs builtOutputs, + SingleDrvOutputs builtOutputs, std::optional<Error> ex) { buildResult.status = status; @@ -1452,12 +1483,28 @@ void DerivationGoal::waiteeDone(GoalPtr waitee, ExitCode result) { Goal::waiteeDone(waitee, result); - if (waitee->buildResult.success()) - if (auto bfd = std::get_if<DerivedPath::Built>(&waitee->buildResult.path)) - for (auto & [output, realisation] : waitee->buildResult.builtOutputs) + if (!useDerivation) return; + auto & fullDrv = *dynamic_cast<Derivation *>(drv.get()); + + auto * dg = dynamic_cast<DerivationGoal *>(&*waitee); + if (!dg) return; + + auto outputs = fullDrv.inputDrvs.find(dg->drvPath); + if (outputs == fullDrv.inputDrvs.end()) return; + + for (auto & outputName : outputs->second) { + auto buildResult = dg->getBuildResult(DerivedPath::Built { + .drvPath = dg->drvPath, + .outputs = OutputsSpec::Names { outputName }, + }); + if (buildResult.success()) { + auto i = buildResult.builtOutputs.find(outputName); + if (i != buildResult.builtOutputs.end()) inputDrvOutputs.insert_or_assign( - { bfd->drvPath, output.outputName }, - realisation.outPath); + { dg->drvPath, outputName }, + i->second.outPath); + } + } } } diff --git a/src/libstore/build/derivation-goal.hh b/src/libstore/build/derivation-goal.hh index 3a6f0c2d9..7033b7a58 100644 --- a/src/libstore/build/derivation-goal.hh +++ b/src/libstore/build/derivation-goal.hh @@ -79,21 +79,57 @@ struct DerivationGoal : public Goal std::map<std::pair<StorePath, std::string>, StorePath> inputDrvOutputs; /** + * See `needRestart`; just for that field. + */ + enum struct NeedRestartForMoreOutputs { + /** + * The goal state machine is progressing based on the current value of + * `wantedOutputs. No actions are needed. + */ + OutputsUnmodifedDontNeed, + /** + * `wantedOutputs` has been extended, but the state machine is + * proceeding according to its old value, so we need to restart. + */ + OutputsAddedDoNeed, + /** + * The goal state machine has progressed to the point of doing a build, + * in which case all outputs will be produced, so extensions to + * `wantedOutputs` no longer require a restart. + */ + BuildInProgressWillNotNeed, + }; + + /** * Whether additional wanted outputs have been added. */ - bool needRestart = false; + NeedRestartForMoreOutputs needRestart = NeedRestartForMoreOutputs::OutputsUnmodifedDontNeed; /** - * Whether to retry substituting the outputs after building the - * inputs. This is done in case of an incomplete closure. + * See `retrySubstitution`; just for that field. */ - bool retrySubstitution = false; + enum RetrySubstitution { + /** + * No issues have yet arose, no need to restart. + */ + NoNeed, + /** + * Something failed and there is an incomplete closure. Let's retry + * substituting. + */ + YesNeed, + /** + * We are current or have already retried substitution, and whether or + * not something goes wrong we will not retry again. + */ + AlreadyRetried, + }; /** - * Whether we've retried substitution, in which case we won't try - * again. + * Whether to retry substituting the outputs after building the + * inputs. This is done in case of an incomplete closure. */ - bool retriedSubstitution = false; + RetrySubstitution retrySubstitution = RetrySubstitution::NoNeed; /** * The derivation stored at drvPath. @@ -217,7 +253,7 @@ struct DerivationGoal : public Goal * Check that the derivation outputs all exist and register them * as valid. */ - virtual DrvOutputs registerOutputs(); + virtual SingleDrvOutputs registerOutputs(); /** * Open a log file and a pipe to it. @@ -270,17 +306,17 @@ struct DerivationGoal : public Goal * Update 'initialOutputs' to determine the current status of the * outputs of the derivation. Also returns a Boolean denoting * whether all outputs are valid and non-corrupt, and a - * 'DrvOutputs' structure containing the valid and wanted + * 'SingleDrvOutputs' structure containing the valid and wanted * outputs. */ - std::pair<bool, DrvOutputs> checkPathValidity(); + std::pair<bool, SingleDrvOutputs> checkPathValidity(); /** * Aborts if any output is not valid or corrupt, and otherwise - * returns a 'DrvOutputs' structure containing the wanted + * returns a 'SingleDrvOutputs' structure containing the wanted * outputs. */ - DrvOutputs assertPathValidity(); + SingleDrvOutputs assertPathValidity(); /** * Forcibly kill the child process, if any. @@ -293,7 +329,7 @@ struct DerivationGoal : public Goal void done( BuildResult::Status status, - DrvOutputs builtOutputs = {}, + SingleDrvOutputs builtOutputs = {}, std::optional<Error> ex = {}); void waiteeDone(GoalPtr waitee, ExitCode result) override; diff --git a/src/libstore/build/entry-points.cc b/src/libstore/build/entry-points.cc index 2925fe3ca..74eae0692 100644 --- a/src/libstore/build/entry-points.cc +++ b/src/libstore/build/entry-points.cc @@ -10,16 +10,8 @@ void Store::buildPaths(const std::vector<DerivedPath> & reqs, BuildMode buildMod Worker worker(*this, evalStore ? *evalStore : *this); Goals goals; - for (const auto & br : reqs) { - std::visit(overloaded { - [&](const DerivedPath::Built & bfd) { - goals.insert(worker.makeDerivationGoal(bfd.drvPath, bfd.outputs, buildMode)); - }, - [&](const DerivedPath::Opaque & bo) { - goals.insert(worker.makePathSubstitutionGoal(bo.path, buildMode == bmRepair ? Repair : NoRepair)); - }, - }, br.raw()); - } + for (auto & br : reqs) + goals.insert(worker.makeGoal(br, buildMode)); worker.run(goals); @@ -47,7 +39,7 @@ void Store::buildPaths(const std::vector<DerivedPath> & reqs, BuildMode buildMod } } -std::vector<BuildResult> Store::buildPathsWithResults( +std::vector<KeyedBuildResult> Store::buildPathsWithResults( const std::vector<DerivedPath> & reqs, BuildMode buildMode, std::shared_ptr<Store> evalStore) @@ -55,23 +47,23 @@ std::vector<BuildResult> Store::buildPathsWithResults( Worker worker(*this, evalStore ? *evalStore : *this); Goals goals; - for (const auto & br : reqs) { - std::visit(overloaded { - [&](const DerivedPath::Built & bfd) { - goals.insert(worker.makeDerivationGoal(bfd.drvPath, bfd.outputs, buildMode)); - }, - [&](const DerivedPath::Opaque & bo) { - goals.insert(worker.makePathSubstitutionGoal(bo.path, buildMode == bmRepair ? Repair : NoRepair)); - }, - }, br.raw()); + std::vector<std::pair<const DerivedPath &, GoalPtr>> state; + + for (const auto & req : reqs) { + auto goal = worker.makeGoal(req, buildMode); + goals.insert(goal); + state.push_back({req, goal}); } worker.run(goals); - std::vector<BuildResult> results; + std::vector<KeyedBuildResult> results; - for (auto & i : goals) - results.push_back(i->buildResult); + for (auto & [req, goalPtr] : state) + results.emplace_back(KeyedBuildResult { + goalPtr->getBuildResult(req), + /* .path = */ req, + }); return results; } @@ -84,15 +76,14 @@ BuildResult Store::buildDerivation(const StorePath & drvPath, const BasicDerivat try { worker.run(Goals{goal}); - return goal->buildResult; + return goal->getBuildResult(DerivedPath::Built { + .drvPath = drvPath, + .outputs = OutputsSpec::All {}, + }); } catch (Error & e) { return BuildResult { .status = BuildResult::MiscFailure, .errorMsg = e.msg(), - .path = DerivedPath::Built { - .drvPath = drvPath, - .outputs = OutputsSpec::All { }, - }, }; }; } diff --git a/src/libstore/build/goal.cc b/src/libstore/build/goal.cc index d59b94797..ca7097a68 100644 --- a/src/libstore/build/goal.cc +++ b/src/libstore/build/goal.cc @@ -11,6 +11,29 @@ bool CompareGoalPtrs::operator() (const GoalPtr & a, const GoalPtr & b) const { } +BuildResult Goal::getBuildResult(const DerivedPath & req) { + BuildResult res { buildResult }; + + if (auto pbp = std::get_if<DerivedPath::Built>(&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 addToWeakGoals(WeakGoals & goals, GoalPtr p) { if (goals.find(p) != goals.end()) diff --git a/src/libstore/build/goal.hh b/src/libstore/build/goal.hh index f4bf6f38b..c0e12a2ed 100644 --- a/src/libstore/build/goal.hh +++ b/src/libstore/build/goal.hh @@ -81,11 +81,26 @@ struct Goal : public std::enable_shared_from_this<Goal> */ ExitCode exitCode = ecBusy; +protected: /** * Build result. */ BuildResult buildResult; +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 &); + /** * Exception containing an error message, if any. */ @@ -93,7 +108,6 @@ struct Goal : public std::enable_shared_from_this<Goal> Goal(Worker & worker, DerivedPath path) : worker(worker) - , buildResult { .path = std::move(path) } { } virtual ~Goal() diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index bbec4aea0..0a208750e 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -1335,7 +1335,7 @@ struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual Lo result.rethrow(); } - std::vector<BuildResult> buildPathsWithResults( + std::vector<KeyedBuildResult> buildPathsWithResults( const std::vector<DerivedPath> & paths, BuildMode buildMode = bmNormal, std::shared_ptr<Store> evalStore = nullptr) override @@ -2174,7 +2174,7 @@ void LocalDerivationGoal::runChild() } -DrvOutputs LocalDerivationGoal::registerOutputs() +SingleDrvOutputs LocalDerivationGoal::registerOutputs() { /* 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 @@ -2695,7 +2695,7 @@ DrvOutputs LocalDerivationGoal::registerOutputs() means it's safe to link the derivation to the output hash. We must do that for floating CA derivations, which otherwise couldn't be cached, but it's fine to do in all cases. */ - DrvOutputs builtOutputs; + SingleDrvOutputs builtOutputs; for (auto & [outputName, newInfo] : infos) { auto oldinfo = get(initialOutputs, outputName); @@ -2714,7 +2714,7 @@ DrvOutputs LocalDerivationGoal::registerOutputs() worker.store.registerDrvOutput(thisRealisation); } if (wantedOutputs.contains(outputName)) - builtOutputs.emplace(thisRealisation.id, thisRealisation); + builtOutputs.emplace(outputName, thisRealisation); } return builtOutputs; diff --git a/src/libstore/build/local-derivation-goal.hh b/src/libstore/build/local-derivation-goal.hh index 42d32a31a..9acd7593d 100644 --- a/src/libstore/build/local-derivation-goal.hh +++ b/src/libstore/build/local-derivation-goal.hh @@ -237,7 +237,7 @@ struct LocalDerivationGoal : public DerivationGoal * Check that the derivation outputs all exist and register them * as valid. */ - DrvOutputs registerOutputs() override; + SingleDrvOutputs registerOutputs() override; void signRealisation(Realisation &) override; diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index f775f8486..6ad4a0e2b 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -92,6 +92,7 @@ std::shared_ptr<PathSubstitutionGoal> Worker::makePathSubstitutionGoal(const Sto return goal; } + std::shared_ptr<DrvOutputSubstitutionGoal> Worker::makeDrvOutputSubstitutionGoal(const DrvOutput& id, RepairFlag repair, std::optional<ContentAddress> ca) { std::weak_ptr<DrvOutputSubstitutionGoal> & goal_weak = drvOutputSubstitutionGoals[id]; @@ -104,6 +105,20 @@ std::shared_ptr<DrvOutputSubstitutionGoal> Worker::makeDrvOutputSubstitutionGoal return goal; } + +GoalPtr Worker::makeGoal(const DerivedPath & req, BuildMode buildMode) +{ + return std::visit(overloaded { + [&](const DerivedPath::Built & bfd) -> GoalPtr { + return makeDerivationGoal(bfd.drvPath, bfd.outputs, buildMode); + }, + [&](const DerivedPath::Opaque & bo) -> GoalPtr { + return makePathSubstitutionGoal(bo.path, buildMode == bmRepair ? Repair : NoRepair); + }, + }, req.raw()); +} + + template<typename K, typename G> static void removeGoal(std::shared_ptr<G> goal, std::map<K, std::weak_ptr<G>> & goalMap) { diff --git a/src/libstore/build/worker.hh b/src/libstore/build/worker.hh index 48a1a27fa..bb51d641d 100644 --- a/src/libstore/build/worker.hh +++ b/src/libstore/build/worker.hh @@ -181,7 +181,7 @@ public: */ /** - * derivation goal + * @ref DerivationGoal "derivation goal" */ private: std::shared_ptr<DerivationGoal> makeDerivationGoalCommon( @@ -196,12 +196,20 @@ public: const OutputsSpec & wantedOutputs, BuildMode buildMode = bmNormal); /** - * substitution goal + * @ref SubstitutionGoal "substitution goal" */ std::shared_ptr<PathSubstitutionGoal> makePathSubstitutionGoal(const StorePath & storePath, RepairFlag repair = NoRepair, std::optional<ContentAddress> ca = std::nullopt); std::shared_ptr<DrvOutputSubstitutionGoal> makeDrvOutputSubstitutionGoal(const DrvOutput & id, RepairFlag repair = NoRepair, std::optional<ContentAddress> ca = std::nullopt); /** + * Make a goal corresponding to the `DerivedPath`. + * + * It will be a `DerivationGoal` for a `DerivedPath::Built` or + * a `SubstitutionGoal` for a `DerivedPath::Opaque`. + */ + GoalPtr makeGoal(const DerivedPath & req, BuildMode buildMode = bmNormal); + + /** * Remove a dead goal. */ void removeGoal(GoalPtr goal); diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 139a05587..d3b9988c9 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -641,7 +641,10 @@ static void performOp(TunnelLogger * logger, ref<Store> store, to << res.timesBuilt << res.isNonDeterministic << res.startTime << res.stopTime; } if (GET_PROTOCOL_MINOR(clientVersion) >= 28) { - worker_proto::write(*store, to, res.builtOutputs); + DrvOutputs builtOutputs; + for (auto & [output, realisation] : res.builtOutputs) + builtOutputs.insert_or_assign(realisation.id, realisation); + worker_proto::write(*store, to, builtOutputs); } break; } @@ -1068,6 +1071,8 @@ void processConnection( opCount++; + debug("performing daemon worker op: %d", op); + try { performOp(tunnelLogger, store, trusted, recursive, clientVersion, from, to, op); } catch (Error & e) { diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc index 1b38e32fb..4c66d08ee 100644 --- a/src/libstore/globals.cc +++ b/src/libstore/globals.cc @@ -22,6 +22,9 @@ #include <dlfcn.h> #endif +#include "config-impl.hh" + + namespace nix { @@ -192,18 +195,18 @@ NLOHMANN_JSON_SERIALIZE_ENUM(SandboxMode, { {SandboxMode::smDisabled, false}, }); -template<> void BaseSetting<SandboxMode>::set(const std::string & str, bool append) +template<> SandboxMode BaseSetting<SandboxMode>::parse(const std::string & str) const { - if (str == "true") value = smEnabled; - else if (str == "relaxed") value = smRelaxed; - else if (str == "false") value = smDisabled; + if (str == "true") return smEnabled; + else if (str == "relaxed") return smRelaxed; + else if (str == "false") return smDisabled; else throw UsageError("option '%s' has invalid value '%s'", name, str); } -template<> bool BaseSetting<SandboxMode>::isAppendable() +template<> struct BaseSetting<SandboxMode>::trait { - return false; -} + static constexpr bool appendable = false; +}; template<> std::string BaseSetting<SandboxMode>::to_string() const { @@ -235,23 +238,23 @@ template<> void BaseSetting<SandboxMode>::convertToArg(Args & args, const std::s }); } -void MaxBuildJobsSetting::set(const std::string & str, bool append) +unsigned int MaxBuildJobsSetting::parse(const std::string & str) const { - if (str == "auto") value = std::max(1U, std::thread::hardware_concurrency()); + if (str == "auto") return std::max(1U, std::thread::hardware_concurrency()); else { if (auto n = string2Int<decltype(value)>(str)) - value = *n; + return *n; else throw UsageError("configuration setting '%s' should be 'auto' or an integer", name); } } -void PluginFilesSetting::set(const std::string & str, bool append) +Paths PluginFilesSetting::parse(const std::string & str) const { if (pluginsLoaded) throw UsageError("plugin-files set after plugins were loaded, you may need to move the flag before the subcommand"); - BaseSetting<Paths>::set(str, append); + return BaseSetting<Paths>::parse(str); } diff --git a/src/libstore/globals.hh b/src/libstore/globals.hh index d6c5d437a..609cf53b8 100644 --- a/src/libstore/globals.hh +++ b/src/libstore/globals.hh @@ -26,7 +26,7 @@ struct MaxBuildJobsSetting : public BaseSetting<unsigned int> options->addSetting(this); } - void set(const std::string & str, bool append = false) override; + unsigned int parse(const std::string & str) const override; }; struct PluginFilesSetting : public BaseSetting<Paths> @@ -43,7 +43,7 @@ struct PluginFilesSetting : public BaseSetting<Paths> options->addSetting(this); } - void set(const std::string & str, bool append = false) override; + Paths parse(const std::string & str) const override; }; const uint32_t maxIdsPerBuild = diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index c3cb3032a..2012584e0 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -287,19 +287,18 @@ public: conn->to.flush(); - BuildResult status { - .path = DerivedPath::Built { - .drvPath = drvPath, - .outputs = OutputsSpec::All { }, - }, - }; + BuildResult status; status.status = (BuildResult::Status) readInt(conn->from); conn->from >> status.errorMsg; if (GET_PROTOCOL_MINOR(conn->remoteVersion) >= 3) conn->from >> status.timesBuilt >> status.isNonDeterministic >> status.startTime >> status.stopTime; if (GET_PROTOCOL_MINOR(conn->remoteVersion) >= 6) { - status.builtOutputs = worker_proto::read(*this, conn->from, Phantom<DrvOutputs> {}); + auto builtOutputs = worker_proto::read(*this, conn->from, Phantom<DrvOutputs> {}); + for (auto && [output, realisation] : builtOutputs) + status.builtOutputs.insert_or_assign( + std::move(output.outputName), + std::move(realisation)); } return status; } @@ -330,7 +329,7 @@ public: conn->to.flush(); - BuildResult result { .path = DerivedPath::Opaque { StorePath::dummy } }; + BuildResult result; result.status = (BuildResult::Status) readInt(conn->from); if (!result.success()) { diff --git a/src/libstore/realisation.hh b/src/libstore/realisation.hh index a18cf2aa8..3922d1267 100644 --- a/src/libstore/realisation.hh +++ b/src/libstore/realisation.hh @@ -13,9 +13,25 @@ namespace nix { class Store; +/** + * A general `Realisation` key. + * + * This is similar to a `DerivedPath::Opaque`, but the derivation is + * identified by its "hash modulo" instead of by its store path. + */ struct DrvOutput { - // The hash modulo of the derivation + /** + * The hash modulo of the derivation. + * + * Computed from the derivation itself for most types of + * derivations, but computed from the (fixed) content address of the + * output for fixed-output derivations. + */ Hash drvHash; + + /** + * The name of the output. + */ std::string outputName; std::string to_string() const; @@ -60,6 +76,21 @@ struct Realisation { GENERATE_CMP(Realisation, me->id, me->outPath); }; +/** + * Collection type for a single derivation's outputs' `Realisation`s. + * + * Since these are the outputs of a single derivation, we know the + * output names are unique so we can use them as the map key. + */ +typedef std::map<std::string, Realisation> SingleDrvOutputs; + +/** + * Collection type for multiple derivations' outputs' `Realisation`s. + * + * `DrvOutput` is used because in general the derivations are not all + * the same, so we need to identify firstly which derivation, and + * secondly which output of that derivation. + */ typedef std::map<DrvOutput, Realisation> DrvOutputs; struct OpaquePath { diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 2abd3aa51..ff5348830 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -125,10 +125,26 @@ void write(const Store & store, Sink & out, const DrvOutput & drvOutput) } -BuildResult read(const Store & store, Source & from, Phantom<BuildResult> _) +KeyedBuildResult read(const Store & store, Source & from, Phantom<KeyedBuildResult> _) { auto path = worker_proto::read(store, from, Phantom<DerivedPath> {}); - BuildResult res { .path = path }; + auto br = worker_proto::read(store, from, Phantom<BuildResult> {}); + return KeyedBuildResult { + std::move(br), + /* .path = */ std::move(path), + }; +} + +void write(const Store & store, Sink & to, const KeyedBuildResult & res) +{ + worker_proto::write(store, to, res.path); + worker_proto::write(store, to, static_cast<const BuildResult &>(res)); +} + + +BuildResult read(const Store & store, Source & from, Phantom<BuildResult> _) +{ + BuildResult res; res.status = (BuildResult::Status) readInt(from); from >> res.errorMsg @@ -136,13 +152,16 @@ BuildResult read(const Store & store, Source & from, Phantom<BuildResult> _) >> res.isNonDeterministic >> res.startTime >> res.stopTime; - res.builtOutputs = worker_proto::read(store, from, Phantom<DrvOutputs> {}); + auto builtOutputs = worker_proto::read(store, from, Phantom<DrvOutputs> {}); + for (auto && [output, realisation] : builtOutputs) + res.builtOutputs.insert_or_assign( + std::move(output.outputName), + std::move(realisation)); return res; } void write(const Store & store, Sink & to, const BuildResult & res) { - worker_proto::write(store, to, res.path); to << res.status << res.errorMsg @@ -150,7 +169,10 @@ void write(const Store & store, Sink & to, const BuildResult & res) << res.isNonDeterministic << res.startTime << res.stopTime; - worker_proto::write(store, to, res.builtOutputs); + DrvOutputs builtOutputs; + for (auto & [output, realisation] : res.builtOutputs) + builtOutputs.insert_or_assign(realisation.id, realisation); + worker_proto::write(store, to, builtOutputs); } @@ -869,7 +891,7 @@ void RemoteStore::buildPaths(const std::vector<DerivedPath> & drvPaths, BuildMod readInt(conn->from); } -std::vector<BuildResult> RemoteStore::buildPathsWithResults( +std::vector<KeyedBuildResult> RemoteStore::buildPathsWithResults( const std::vector<DerivedPath> & paths, BuildMode buildMode, std::shared_ptr<Store> evalStore) @@ -884,7 +906,7 @@ std::vector<BuildResult> RemoteStore::buildPathsWithResults( writeDerivedPaths(*this, conn, paths); conn->to << buildMode; conn.processStderr(); - return worker_proto::read(*this, conn->from, Phantom<std::vector<BuildResult>> {}); + return worker_proto::read(*this, conn->from, Phantom<std::vector<KeyedBuildResult>> {}); } else { // Avoid deadlock. conn_.reset(); @@ -893,21 +915,25 @@ std::vector<BuildResult> RemoteStore::buildPathsWithResults( // fails, but meh. buildPaths(paths, buildMode, evalStore); - std::vector<BuildResult> results; + std::vector<KeyedBuildResult> results; for (auto & path : paths) { std::visit( overloaded { [&](const DerivedPath::Opaque & bo) { - results.push_back(BuildResult { - .status = BuildResult::Substituted, - .path = bo, + results.push_back(KeyedBuildResult { + { + .status = BuildResult::Substituted, + }, + /* .path = */ bo, }); }, [&](const DerivedPath::Built & bfd) { - BuildResult res { - .status = BuildResult::Built, - .path = bfd, + KeyedBuildResult res { + { + .status = BuildResult::Built + }, + /* .path = */ bfd, }; OutputPathMap outputs; @@ -926,10 +952,10 @@ std::vector<BuildResult> RemoteStore::buildPathsWithResults( queryRealisation(outputId); if (!realisation) throw MissingRealisation(outputId); - res.builtOutputs.emplace(realisation->id, *realisation); + res.builtOutputs.emplace(output, *realisation); } else { res.builtOutputs.emplace( - outputId, + output, Realisation { .id = outputId, .outPath = outputPath, @@ -956,12 +982,7 @@ BuildResult RemoteStore::buildDerivation(const StorePath & drvPath, const BasicD writeDerivation(conn->to, *this, drv); conn->to << buildMode; conn.processStderr(); - BuildResult res { - .path = DerivedPath::Built { - .drvPath = drvPath, - .outputs = OutputsSpec::All { }, - }, - }; + BuildResult res; res.status = (BuildResult::Status) readInt(conn->from); conn->from >> res.errorMsg; if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 29) { @@ -969,7 +990,10 @@ BuildResult RemoteStore::buildDerivation(const StorePath & drvPath, const BasicD } if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 28) { auto builtOutputs = worker_proto::read(*this, conn->from, Phantom<DrvOutputs> {}); - res.builtOutputs = builtOutputs; + for (auto && [output, realisation] : builtOutputs) + res.builtOutputs.insert_or_assign( + std::move(output.outputName), + std::move(realisation)); } return res; } diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh index 3babf8e21..82e4656ab 100644 --- a/src/libstore/remote-store.hh +++ b/src/libstore/remote-store.hh @@ -115,7 +115,7 @@ public: void buildPaths(const std::vector<DerivedPath> & paths, BuildMode buildMode, std::shared_ptr<Store> evalStore) override; - std::vector<BuildResult> buildPathsWithResults( + std::vector<KeyedBuildResult> buildPathsWithResults( const std::vector<DerivedPath> & paths, BuildMode buildMode, std::shared_ptr<Store> evalStore) override; diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 74f50a00d..c910d1c96 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -92,6 +92,7 @@ enum BuildMode { bmNormal, bmRepair, bmCheck }; enum TrustedFlag : bool { NotTrusted = false, Trusted = true }; struct BuildResult; +struct KeyedBuildResult; typedef std::map<StorePath, std::optional<ContentAddress>> StorePathCAMap; @@ -569,7 +570,7 @@ public: * case of a build/substitution error, this function won't throw an * exception, but return a BuildResult containing an error message. */ - virtual std::vector<BuildResult> buildPathsWithResults( + virtual std::vector<KeyedBuildResult> buildPathsWithResults( const std::vector<DerivedPath> & paths, BuildMode buildMode = bmNormal, std::shared_ptr<Store> evalStore = nullptr); diff --git a/src/libstore/worker-protocol.hh b/src/libstore/worker-protocol.hh index c7a6f8688..34b2fc17b 100644 --- a/src/libstore/worker-protocol.hh +++ b/src/libstore/worker-protocol.hh @@ -103,6 +103,7 @@ MAKE_WORKER_PROTO(, DerivedPath); MAKE_WORKER_PROTO(, Realisation); MAKE_WORKER_PROTO(, DrvOutput); MAKE_WORKER_PROTO(, BuildResult); +MAKE_WORKER_PROTO(, KeyedBuildResult); MAKE_WORKER_PROTO(, std::optional<TrustedFlag>); MAKE_WORKER_PROTO(template<typename T>, std::vector<T>); |