From c98525235f5b8f1ed02fd1b3849b42e5f669d364 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 20 Nov 2023 16:38:41 -0500 Subject: [PATCH 1/5] Revert "Revert "Adapt scheduler to work with dynamic derivations"" This fixes dynamic derivations, reverting #9081. I believe that this time around, #9052 is fixed. When I first rebased this, tests were failing (which wasn't the case before). The cause of those test failures were due to the crude job in which the outer goal tried to exit with the inner goal's status. Now, that error handling has been reworked to be more faithful. The exit exit status and exception of the inner goal is returned by the outer goal. The exception was what was causing the test failures, but I believe it was not having the right error code (there is more than one for failure) that caused #9081. The only cost of doing things the "right way" was that I had to introduce a hacky `preserveException` boolean. I don't like this, but, then again, none of us like anything about how the scheduler works. Issue #11927 is still there to clean everything up, subsuming the need for any `preserveException` because I doubt we will be fishing information out of state machines like this at all. This reverts commit 8440afbed756254784d9fea3eaab06649dffd390. Co-Authored-By: Eelco Dolstra --- ...erivation-creation-and-realisation-goal.cc | 126 ++++++++++++++++++ ...erivation-creation-and-realisation-goal.hh | 89 +++++++++++++ src/libstore/build/derivation-goal.cc | 26 +--- src/libstore/build/derivation-goal.hh | 4 + src/libstore/build/entry-points.cc | 5 +- src/libstore/build/goal.cc | 2 +- src/libstore/build/goal.hh | 21 +++ src/libstore/build/worker.cc | 92 ++++++++++--- src/libstore/build/worker.hh | 24 ++++ src/libstore/derived-path-map.cc | 4 + src/libstore/derived-path-map.hh | 7 +- src/libstore/meson.build | 2 + tests/functional/dyn-drv/build-built-drv.sh | 7 +- tests/functional/dyn-drv/dep-built-drv.sh | 7 +- tests/functional/dyn-drv/failing-outer.sh | 12 ++ tests/functional/dyn-drv/meson.build | 1 + .../functional/dyn-drv/text-hashed-output.nix | 10 ++ 17 files changed, 397 insertions(+), 42 deletions(-) create mode 100644 src/libstore/build/derivation-creation-and-realisation-goal.cc create mode 100644 src/libstore/build/derivation-creation-and-realisation-goal.hh create mode 100644 tests/functional/dyn-drv/failing-outer.sh diff --git a/src/libstore/build/derivation-creation-and-realisation-goal.cc b/src/libstore/build/derivation-creation-and-realisation-goal.cc new file mode 100644 index 00000000000..c33b7571f04 --- /dev/null +++ b/src/libstore/build/derivation-creation-and-realisation-goal.cc @@ -0,0 +1,126 @@ +#include "derivation-creation-and-realisation-goal.hh" +#include "worker.hh" + +namespace nix { + +DerivationCreationAndRealisationGoal::DerivationCreationAndRealisationGoal( + ref drvReq, const OutputsSpec & wantedOutputs, Worker & worker, BuildMode buildMode) + : Goal(worker, DerivedPath::Built{.drvPath = drvReq, .outputs = wantedOutputs}) + , drvReq(drvReq) + , wantedOutputs(wantedOutputs) + , buildMode(buildMode) +{ + name = + fmt("outer obtaining drv from '%s' and then building outputs %s", + drvReq->to_string(worker.store), + std::visit( + overloaded{ + [&](const OutputsSpec::All) -> std::string { return "* (all of them)"; }, + [&](const OutputsSpec::Names os) { return concatStringsSep(", ", quoteStrings(os)); }, + }, + wantedOutputs.raw)); + trace("created outer"); + + worker.updateProgress(); +} + +DerivationCreationAndRealisationGoal::~DerivationCreationAndRealisationGoal() {} + +static StorePath pathPartOfReq(const SingleDerivedPath & req) +{ + return std::visit( + overloaded{ + [&](const SingleDerivedPath::Opaque & bo) { return bo.path; }, + [&](const SingleDerivedPath::Built & bfd) { return pathPartOfReq(*bfd.drvPath); }, + }, + req.raw()); +} + +std::string DerivationCreationAndRealisationGoal::key() +{ + /* Ensure that derivations get built in order of their name, + i.e. a derivation named "aardvark" always comes before "baboon". And + substitution goals and inner derivation goals always happen before + derivation goals (due to "b$"). */ + return "c$" + std::string(pathPartOfReq(*drvReq).name()) + "$" + drvReq->to_string(worker.store); +} + +void DerivationCreationAndRealisationGoal::timedOut(Error && ex) {} + +void DerivationCreationAndRealisationGoal::addWantedOutputs(const OutputsSpec & outputs) +{ + /* If we already want all outputs, there is nothing to do. */ + auto newWanted = wantedOutputs.union_(outputs); + bool needRestart = !newWanted.isSubsetOf(wantedOutputs); + wantedOutputs = newWanted; + + if (!needRestart) + return; + + if (!optDrvPath) + // haven't started steps where the outputs matter yet + return; + worker.makeDerivationGoal(*optDrvPath, outputs, buildMode); +} + +Goal::Co DerivationCreationAndRealisationGoal::init() +{ + trace("outer init"); + + /* The first thing to do is to make sure that the derivation + exists. If it doesn't, it may be created through a + substitute. */ + if (auto optDrvPath = [this]() -> std::optional { + if (buildMode != bmNormal) + return std::nullopt; + + auto drvPath = StorePath::dummy; + try { + drvPath = resolveDerivedPath(worker.store, *drvReq); + } catch (MissingRealisation &) { + return std::nullopt; + } + auto cond = worker.evalStore.isValidPath(drvPath) || worker.store.isValidPath(drvPath); + return cond ? std::optional{drvPath} : std::nullopt; + }()) { + trace( + fmt("already have drv '%s' for '%s', can go straight to building", + worker.store.printStorePath(*optDrvPath), + drvReq->to_string(worker.store))); + } else { + trace("need to obtain drv we want to build"); + addWaitee(worker.makeGoal(DerivedPath::fromSingle(*drvReq))); + co_await Suspend{}; + } + + trace("outer load and build derivation"); + + if (nrFailed != 0) { + co_return amDone(ecFailed, Error("cannot build missing derivation '%s'", drvReq->to_string(worker.store))); + } + + StorePath drvPath = resolveDerivedPath(worker.store, *drvReq); + /* Build this step! */ + concreteDrvGoal = worker.makeDerivationGoal(drvPath, wantedOutputs, buildMode); + { + auto g = upcast_goal(concreteDrvGoal); + /* We will finish with it ourselves, as if we were the derivational goal. */ + g->preserveException = true; + } + optDrvPath = std::move(drvPath); + addWaitee(upcast_goal(concreteDrvGoal)); + co_await Suspend{}; + + trace("outer build done"); + + buildResult = upcast_goal(concreteDrvGoal) + ->getBuildResult(DerivedPath::Built{ + .drvPath = drvReq, + .outputs = wantedOutputs, + }); + + auto g = upcast_goal(concreteDrvGoal); + co_return amDone(g->exitCode, g->ex); +} + +} diff --git a/src/libstore/build/derivation-creation-and-realisation-goal.hh b/src/libstore/build/derivation-creation-and-realisation-goal.hh new file mode 100644 index 00000000000..9960484ac95 --- /dev/null +++ b/src/libstore/build/derivation-creation-and-realisation-goal.hh @@ -0,0 +1,89 @@ +#pragma once + +#include "parsed-derivations.hh" +#include "user-lock.hh" +#include "store-api.hh" +#include "pathlocks.hh" +#include "goal.hh" + +namespace nix { + +struct DerivationGoal; + +/** + * This goal type is essentially the serial composition (like function + * composition) of a goal for getting a derivation, and then a + * `DerivationGoal` using the newly-obtained derivation. + * + * In the (currently experimental) general inductive case of derivations + * that are themselves build outputs, that first goal will be *another* + * `DerivationCreationAndRealisationGoal`. In the (much more common) base-case + * where the derivation has no provence and is just referred to by + * (content-addressed) store path, that first goal is a + * `SubstitutionGoal`. + * + * If we already have the derivation (e.g. if the evaluator has created + * the derivation locally and then instructured the store to build it), + * we can skip the first goal entirely as a small optimization. + */ +struct DerivationCreationAndRealisationGoal : public Goal +{ + /** + * How to obtain a store path of the derivation to build. + */ + ref drvReq; + + /** + * The path of the derivation, once obtained. + **/ + std::optional optDrvPath; + + /** + * The goal for the corresponding concrete derivation. + **/ + std::shared_ptr concreteDrvGoal; + + /** + * The specific outputs that we need to build. + */ + OutputsSpec wantedOutputs; + + /** + * The final output paths of the build. + * + * - For input-addressed derivations, always the precomputed paths + * + * - For content-addressed derivations, calcuated from whatever the + * hash ends up being. (Note that fixed outputs derivations that + * produce the "wrong" output still install that data under its + * true content-address.) + */ + OutputPathMap finalOutputs; + + BuildMode buildMode; + + DerivationCreationAndRealisationGoal( + ref drvReq, + const OutputsSpec & wantedOutputs, + Worker & worker, + BuildMode buildMode = bmNormal); + virtual ~DerivationCreationAndRealisationGoal(); + + void timedOut(Error && ex) override; + + std::string key() override; + + /** + * Add wanted outputs to an already existing derivation goal. + */ + void addWantedOutputs(const OutputsSpec & outputs); + + Co init() override; + + JobCategory jobCategory() const override + { + return JobCategory::Administration; + }; +}; + +} diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 0d16f09750b..fee9c65be80 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -137,21 +137,8 @@ Goal::Co DerivationGoal::init() { trace("init"); if (useDerivation) { - /* The first thing to do is to make sure that the derivation - exists. If it doesn't, it may be created through a - substitute. */ - - if (buildMode != bmNormal || !worker.evalStore.isValidPath(drvPath)) { - addWaitee(upcast_goal(worker.makePathSubstitutionGoal(drvPath))); - co_await Suspend{}; - } - trace("loading derivation"); - if (nrFailed != 0) { - co_return done(BuildResult::MiscFailure, {}, Error("cannot build missing derivation '%s'", worker.store.printStorePath(drvPath))); - } - /* `drvPath' should already be a root, but let's be on the safe side: if the user forgot to make it a root, we wouldn't want things being garbage collected while we're busy. */ @@ -1549,23 +1536,24 @@ void DerivationGoal::waiteeDone(GoalPtr waitee, ExitCode result) if (!useDerivation || !drv) return; auto & fullDrv = *dynamic_cast(drv.get()); - auto * dg = dynamic_cast(&*waitee); - if (!dg) return; + std::optional info = tryGetConcreteDrvGoal(waitee); + if (!info) return; + const auto & [dg, drvReq] = *info; - auto * nodeP = fullDrv.inputDrvs.findSlot(DerivedPath::Opaque { .path = dg->drvPath }); + auto * nodeP = fullDrv.inputDrvs.findSlot(drvReq.get()); if (!nodeP) return; auto & outputs = nodeP->value; for (auto & outputName : outputs) { - auto buildResult = dg->getBuildResult(DerivedPath::Built { - .drvPath = makeConstantStorePathRef(dg->drvPath), + auto buildResult = dg.get().getBuildResult(DerivedPath::Built { + .drvPath = makeConstantStorePathRef(dg.get().drvPath), .outputs = OutputsSpec::Names { outputName }, }); if (buildResult.success()) { auto i = buildResult.builtOutputs.find(outputName); if (i != buildResult.builtOutputs.end()) inputDrvOutputs.insert_or_assign( - { dg->drvPath, outputName }, + { dg.get().drvPath, outputName }, i->second.outPath); } } diff --git a/src/libstore/build/derivation-goal.hh b/src/libstore/build/derivation-goal.hh index 4e9c1451901..53884081301 100644 --- a/src/libstore/build/derivation-goal.hh +++ b/src/libstore/build/derivation-goal.hh @@ -56,6 +56,10 @@ struct InitialOutput { /** * A goal for building some or all of the outputs of a derivation. + * + * The derivation must already be present, either in the store in a drv + * or in memory. If the derivation itself needs to be gotten first, a + * `DerivationCreationAndRealisationGoal` goal must be used instead. */ struct DerivationGoal : public Goal { diff --git a/src/libstore/build/entry-points.cc b/src/libstore/build/entry-points.cc index 3bf22320e3a..a473daff914 100644 --- a/src/libstore/build/entry-points.cc +++ b/src/libstore/build/entry-points.cc @@ -1,6 +1,7 @@ #include "worker.hh" #include "substitution-goal.hh" #ifndef _WIN32 // TODO Enable building on Windows +# include "derivation-creation-and-realisation-goal.hh" # include "derivation-goal.hh" #endif #include "local-store.hh" @@ -29,8 +30,8 @@ void Store::buildPaths(const std::vector & reqs, BuildMode buildMod } if (i->exitCode != Goal::ecSuccess) { #ifndef _WIN32 // TODO Enable building on Windows - if (auto i2 = dynamic_cast(i.get())) - failed.insert(printStorePath(i2->drvPath)); + if (auto i2 = dynamic_cast(i.get())) + failed.insert(i2->drvReq->to_string(*this)); else #endif if (auto i2 = dynamic_cast(i.get())) diff --git a/src/libstore/build/goal.cc b/src/libstore/build/goal.cc index 9a16da14555..c381e5b581f 100644 --- a/src/libstore/build/goal.cc +++ b/src/libstore/build/goal.cc @@ -175,7 +175,7 @@ Goal::Done Goal::amDone(ExitCode result, std::optional ex) exitCode = result; if (ex) { - if (!waiters.empty()) + if (!preserveException && !waiters.empty()) logError(ex->info()); else this->ex = std::move(*ex); diff --git a/src/libstore/build/goal.hh b/src/libstore/build/goal.hh index 1dd7ed52537..2db1098b736 100644 --- a/src/libstore/build/goal.hh +++ b/src/libstore/build/goal.hh @@ -50,6 +50,16 @@ enum struct JobCategory { * A substitution an arbitrary store object; it will use network resources. */ Substitution, + /** + * A goal that does no "real" work by itself, and just exists to depend on + * other goals which *do* do real work. These goals therefore are not + * limited. + * + * These goals cannot infinitely create themselves, so there is no risk of + * a "fork bomb" type situation (which would be a problem even though the + * goal do no real work) either. + */ + Administration, }; struct Goal : public std::enable_shared_from_this @@ -373,6 +383,17 @@ public: */ BuildResult getBuildResult(const DerivedPath &) const; + /** + * Hack to say that this goal should not log `ex`, but instead keep + * it around. Set by a waitee which sees itself as the designated + * continuation of this goal, responsible for reporting its + * successes or failures. + * + * @todo this is yet another not-nice hack in the goal system that + * we ought to get rid of. See #11927 + */ + bool preserveException = false; + /** * Exception containing an error message, if any. */ diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index dbe86f43f6a..6510bbe9edd 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -5,6 +5,7 @@ #include "drv-output-substitution-goal.hh" #include "derivation-goal.hh" #ifndef _WIN32 // TODO Enable building on Windows +# include "derivation-creation-and-realisation-goal.hh" # include "local-derivation-goal.hh" # include "hook-instance.hh" #endif @@ -43,6 +44,24 @@ Worker::~Worker() } +std::shared_ptr Worker::makeDerivationCreationAndRealisationGoal( + ref drvReq, + const OutputsSpec & wantedOutputs, + BuildMode buildMode) +{ + std::weak_ptr & goal_weak = outerDerivationGoals.ensureSlot(*drvReq).value; + std::shared_ptr goal = goal_weak.lock(); + if (!goal) { + goal = std::make_shared(drvReq, wantedOutputs, *this, buildMode); + goal_weak = goal; + wakeUp(goal); + } else { + goal->addWantedOutputs(wantedOutputs); + } + return goal; +} + + std::shared_ptr Worker::makeDerivationGoalCommon( const StorePath & drvPath, const OutputsSpec & wantedOutputs, @@ -120,10 +139,7 @@ GoalPtr Worker::makeGoal(const DerivedPath & req, BuildMode buildMode) { return std::visit(overloaded { [&](const DerivedPath::Built & bfd) -> GoalPtr { - if (auto bop = std::get_if(&*bfd.drvPath)) - return makeDerivationGoal(bop->path, bfd.outputs, buildMode); - else - throw UnimplementedError("Building dynamic derivations in one shot is not yet implemented."); + return makeDerivationCreationAndRealisationGoal(bfd.drvPath, bfd.outputs, buildMode); }, [&](const DerivedPath::Opaque & bo) -> GoalPtr { return makePathSubstitutionGoal(bo.path, buildMode == bmRepair ? Repair : NoRepair); @@ -132,24 +148,46 @@ GoalPtr Worker::makeGoal(const DerivedPath & req, BuildMode buildMode) } +template +static void cullMap(std::map & goalMap, F f) +{ + for (auto i = goalMap.begin(); i != goalMap.end();) + if (!f(i->second)) + i = goalMap.erase(i); + else ++i; +} + + template static void removeGoal(std::shared_ptr goal, std::map> & goalMap) { /* !!! inefficient */ - for (auto i = goalMap.begin(); - i != goalMap.end(); ) - if (i->second.lock() == goal) { - auto j = i; ++j; - goalMap.erase(i); - i = j; - } - else ++i; + cullMap(goalMap, [&](const std::weak_ptr & gp) -> bool { + return gp.lock() != goal; + }); +} + +template +static void removeGoal(std::shared_ptr goal, std::map>::ChildNode> & goalMap); + +template +static void removeGoal(std::shared_ptr goal, std::map>::ChildNode> & goalMap) +{ + /* !!! inefficient */ + cullMap(goalMap, [&](DerivedPathMap>::ChildNode & node) -> bool { + if (node.value.lock() == goal) + node.value.reset(); + removeGoal(goal, node.childMap); + return !node.value.expired() || !node.childMap.empty(); + }); } void Worker::removeGoal(GoalPtr goal) { - if (auto drvGoal = std::dynamic_pointer_cast(goal)) + if (auto drvGoal = std::dynamic_pointer_cast(goal)) + nix::removeGoal(drvGoal, outerDerivationGoals.map); + else if (auto drvGoal = std::dynamic_pointer_cast(goal)) nix::removeGoal(drvGoal, derivationGoals); else if (auto subGoal = std::dynamic_pointer_cast(goal)) @@ -215,6 +253,9 @@ void Worker::childStarted(GoalPtr goal, const std::set 0); nrLocalBuilds--; break; + case JobCategory::Administration: + /* Intentionally not limited, see docs */ + break; default: unreachable(); } @@ -290,9 +334,9 @@ void Worker::run(const Goals & _topGoals) for (auto & i : _topGoals) { topGoals.insert(i); - if (auto goal = dynamic_cast(i.get())) { + if (auto goal = dynamic_cast(i.get())) { topPaths.push_back(DerivedPath::Built { - .drvPath = makeConstantStorePathRef(goal->drvPath), + .drvPath = goal->drvReq, .outputs = goal->wantedOutputs, }); } else @@ -552,4 +596,22 @@ GoalPtr upcast_goal(std::shared_ptr subGoal) return subGoal; } +GoalPtr upcast_goal(std::shared_ptr subGoal) +{ + return subGoal; +} + +std::optional, std::reference_wrapper>> tryGetConcreteDrvGoal(GoalPtr waitee) +{ + auto * odg = dynamic_cast(&*waitee); + if (!odg) return std::nullopt; + /* If we failed to obtain the concrete drv, we won't have created + the concrete derivation goal. */ + if (!odg->concreteDrvGoal) return std::nullopt; + return {{ + std::cref(*odg->concreteDrvGoal), + std::cref(*odg->drvReq), + }}; +} + } diff --git a/src/libstore/build/worker.hh b/src/libstore/build/worker.hh index f5e61720807..efd518f9995 100644 --- a/src/libstore/build/worker.hh +++ b/src/libstore/build/worker.hh @@ -3,6 +3,7 @@ #include "types.hh" #include "store-api.hh" +#include "derived-path-map.hh" #include "goal.hh" #include "realisation.hh" #include "muxable-pipe.hh" @@ -13,6 +14,7 @@ namespace nix { /* Forward definition. */ +struct DerivationCreationAndRealisationGoal; struct DerivationGoal; struct PathSubstitutionGoal; class DrvOutputSubstitutionGoal; @@ -31,9 +33,25 @@ class DrvOutputSubstitutionGoal; */ GoalPtr upcast_goal(std::shared_ptr subGoal); GoalPtr upcast_goal(std::shared_ptr subGoal); +GoalPtr upcast_goal(std::shared_ptr subGoal); typedef std::chrono::time_point steady_time_point; +/** + * The current implementation of impure derivations has + * `DerivationGoal`s accumulate realisations from their waitees. + * Unfortunately, `DerivationGoal`s don't directly depend on other + * goals, but instead depend on `DerivationCreationAndRealisationGoal`s. + * + * We try not to share any of the details of any goal type with any + * other, for sake of modularity and quicker rebuilds. This means we + * cannot "just" downcast and fish out the field. So as an escape hatch, + * we have made the function, written in `worker.cc` where all the goal + * types are visible, and use it instead. + */ + +std::optional, std::reference_wrapper>> tryGetConcreteDrvGoal(GoalPtr waitee); + /** * A mapping used to remember for each child process to what goal it * belongs, and comm channels for receiving log data and output @@ -103,6 +121,9 @@ private: * Maps used to prevent multiple instantiations of a goal for the * same derivation / path. */ + + DerivedPathMap> outerDerivationGoals; + std::map> derivationGoals; std::map> substitutionGoals; std::map> drvOutputSubstitutionGoals; @@ -196,6 +217,9 @@ public: * @ref DerivationGoal "derivation goal" */ private: + std::shared_ptr makeDerivationCreationAndRealisationGoal( + ref drvPath, + const OutputsSpec & wantedOutputs, BuildMode buildMode = bmNormal); std::shared_ptr makeDerivationGoalCommon( const StorePath & drvPath, const OutputsSpec & wantedOutputs, std::function()> mkDrvGoal); diff --git a/src/libstore/derived-path-map.cc b/src/libstore/derived-path-map.cc index c97d52773eb..0095a9d7814 100644 --- a/src/libstore/derived-path-map.cc +++ b/src/libstore/derived-path-map.cc @@ -52,6 +52,7 @@ typename DerivedPathMap::ChildNode * DerivedPathMap::findSlot(const Single // instantiations +#include "derivation-creation-and-realisation-goal.hh" namespace nix { template<> @@ -68,4 +69,7 @@ std::strong_ordering DerivedPathMap>::ChildNode::operator template struct DerivedPathMap>::ChildNode; template struct DerivedPathMap>; +template struct DerivedPathMap>; + + }; diff --git a/src/libstore/derived-path-map.hh b/src/libstore/derived-path-map.hh index bd60fe88710..61e0b5463e1 100644 --- a/src/libstore/derived-path-map.hh +++ b/src/libstore/derived-path-map.hh @@ -21,8 +21,11 @@ namespace nix { * * @param V A type to instantiate for each output. It should probably * should be an "optional" type so not every interior node has to have a - * value. `* const Something` or `std::optional` would be - * good choices for "optional" types. + * value. For example, the scheduler uses + * `DerivedPathMap>` to + * remember which goals correspond to which outputs. `* const Something` + * or `std::optional` would also be good choices for + * "optional" types. */ template struct DerivedPathMap { diff --git a/src/libstore/meson.build b/src/libstore/meson.build index 79d91249722..d42b010614a 100644 --- a/src/libstore/meson.build +++ b/src/libstore/meson.build @@ -183,6 +183,7 @@ sources = files( 'binary-cache-store.cc', 'build-result.cc', 'build/derivation-goal.cc', + 'build/derivation-creation-and-realisation-goal.cc', 'build/drv-output-substitution-goal.cc', 'build/entry-points.cc', 'build/goal.cc', @@ -255,6 +256,7 @@ headers = [config_h] + files( 'binary-cache-store.hh', 'build-result.hh', 'build/derivation-goal.hh', + 'build/derivation-creation-and-realisation-goal.hh', 'build/drv-output-substitution-goal.hh', 'build/goal.hh', 'build/substitution-goal.hh', diff --git a/tests/functional/dyn-drv/build-built-drv.sh b/tests/functional/dyn-drv/build-built-drv.sh index 647be945716..fcb25a34b45 100644 --- a/tests/functional/dyn-drv/build-built-drv.sh +++ b/tests/functional/dyn-drv/build-built-drv.sh @@ -18,4 +18,9 @@ clearStore drvDep=$(nix-instantiate ./text-hashed-output.nix -A producingDrv) -expectStderr 1 nix build "${drvDep}^out^out" --no-link | grepQuiet "Building dynamic derivations in one shot is not yet implemented" +# Store layer needs bugfix +requireDaemonNewerThan "2.27pre20250205" + +out2=$(nix build "${drvDep}^out^out" --no-link) + +test $out1 == $out2 diff --git a/tests/functional/dyn-drv/dep-built-drv.sh b/tests/functional/dyn-drv/dep-built-drv.sh index 4f6e9b080fa..9d470099a0f 100644 --- a/tests/functional/dyn-drv/dep-built-drv.sh +++ b/tests/functional/dyn-drv/dep-built-drv.sh @@ -4,8 +4,11 @@ source common.sh out1=$(nix-build ./text-hashed-output.nix -A hello --no-out-link) +# Store layer needs bugfix +requireDaemonNewerThan "2.27pre20250205" + clearStore -expectStderr 1 nix-build ./text-hashed-output.nix -A wrapper --no-out-link | grepQuiet "Building dynamic derivations in one shot is not yet implemented" +out2=$(nix-build ./text-hashed-output.nix -A wrapper --no-out-link) -# diff -r $out1 $out2 +diff -r $out1 $out2 diff --git a/tests/functional/dyn-drv/failing-outer.sh b/tests/functional/dyn-drv/failing-outer.sh new file mode 100644 index 00000000000..d888ea876e6 --- /dev/null +++ b/tests/functional/dyn-drv/failing-outer.sh @@ -0,0 +1,12 @@ +#!/usr/bin/env bash + +source common.sh + +# Store layer needs bugfix +requireDaemonNewerThan "2.27pre20250205" + +expected=100 +if [[ -v NIX_DAEMON_PACKAGE ]]; then expected=1; fi # work around the daemon not returning a 100 status correctly + +expectStderr "$expected" nix-build ./text-hashed-output.nix -A failingWrapper --no-out-link \ + | grepQuiet "build of '.*use-dynamic-drv-in-non-dynamic-drv-wrong.drv' failed" diff --git a/tests/functional/dyn-drv/meson.build b/tests/functional/dyn-drv/meson.build index 5b60a46980b..4ad343aee8e 100644 --- a/tests/functional/dyn-drv/meson.build +++ b/tests/functional/dyn-drv/meson.build @@ -12,6 +12,7 @@ suites += { 'recursive-mod-json.sh', 'build-built-drv.sh', 'eval-outputOf.sh', + 'failing-outer.sh', 'dep-built-drv.sh', 'old-daemon-error-hack.sh', ], diff --git a/tests/functional/dyn-drv/text-hashed-output.nix b/tests/functional/dyn-drv/text-hashed-output.nix index 99203b51849..a604a7cbf2c 100644 --- a/tests/functional/dyn-drv/text-hashed-output.nix +++ b/tests/functional/dyn-drv/text-hashed-output.nix @@ -13,6 +13,7 @@ rec { echo "Hello World" > $out/hello ''; }; + producingDrv = mkDerivation { name = "hello.drv"; buildCommand = '' @@ -23,6 +24,7 @@ rec { outputHashMode = "text"; outputHashAlgo = "sha256"; }; + wrapper = mkDerivation { name = "use-dynamic-drv-in-non-dynamic-drv"; buildCommand = '' @@ -30,4 +32,12 @@ rec { cp -r ${builtins.outputOf producingDrv.outPath "out"} $out ''; }; + + failingWrapper = mkDerivation { + name = "use-dynamic-drv-in-non-dynamic-drv-wrong"; + buildCommand = '' + echo "Fail at copying the output of the dynamic derivation" + fail ${builtins.outputOf producingDrv.outPath "out"} $out + ''; + }; } From 1c6e766edbe33a138c746d6c2a91084415c4d952 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 13 Feb 2025 00:52:05 -0500 Subject: [PATCH 2/5] Don't do store operations in `*DerivedPath::toJSON` It is inappropriate to do "real work" when converting plain old data to JSON. --- src/libstore/derived-path.cc | 47 +++++++++++------------------------ src/libstore/derived-path.hh | 8 +++--- tests/functional/build-dry.sh | 15 +++-------- 3 files changed, 22 insertions(+), 48 deletions(-) diff --git a/src/libstore/derived-path.cc b/src/libstore/derived-path.cc index 1eef881de0c..16cd0e389fe 100644 --- a/src/libstore/derived-path.cc +++ b/src/libstore/derived-path.cc @@ -41,49 +41,30 @@ nlohmann::json DerivedPath::Opaque::toJSON(const StoreDirConfig & store) const return store.printStorePath(path); } -nlohmann::json SingleDerivedPath::Built::toJSON(Store & store) const { - nlohmann::json res; - res["drvPath"] = drvPath->toJSON(store); - // Fallback for the input-addressed derivation case: We expect to always be - // able to print the output paths, so let’s do it - // FIXME try-resolve on drvPath - const auto outputMap = store.queryPartialDerivationOutputMap(resolveDerivedPath(store, *drvPath)); - res["output"] = output; - auto outputPathIter = outputMap.find(output); - if (outputPathIter == outputMap.end()) - res["outputPath"] = nullptr; - else if (std::optional p = outputPathIter->second) - res["outputPath"] = store.printStorePath(*p); - else - res["outputPath"] = nullptr; - return res; +nlohmann::json SingleDerivedPath::Built::toJSON(const StoreDirConfig & store) const +{ + return nlohmann::json{ + {"drvPath", drvPath->toJSON(store)}, + {"output", output}, + }; } -nlohmann::json DerivedPath::Built::toJSON(Store & store) const { - nlohmann::json res; - res["drvPath"] = drvPath->toJSON(store); - // Fallback for the input-addressed derivation case: We expect to always be - // able to print the output paths, so let’s do it - // FIXME try-resolve on drvPath - const auto outputMap = store.queryPartialDerivationOutputMap(resolveDerivedPath(store, *drvPath)); - for (const auto & [output, outputPathOpt] : outputMap) { - if (!outputs.contains(output)) continue; - if (outputPathOpt) - res["outputs"][output] = store.printStorePath(*outputPathOpt); - else - res["outputs"][output] = nullptr; - } - return res; +nlohmann::json DerivedPath::Built::toJSON(const StoreDirConfig & store) const +{ + return nlohmann::json{ + {"drvPath", drvPath->toJSON(store)}, + {"outputs", outputs}, + }; } -nlohmann::json SingleDerivedPath::toJSON(Store & store) const +nlohmann::json SingleDerivedPath::toJSON(const StoreDirConfig & store) const { return std::visit([&](const auto & buildable) { return buildable.toJSON(store); }, raw()); } -nlohmann::json DerivedPath::toJSON(Store & store) const +nlohmann::json DerivedPath::toJSON(const StoreDirConfig & store) const { return std::visit([&](const auto & buildable) { return buildable.toJSON(store); diff --git a/src/libstore/derived-path.hh b/src/libstore/derived-path.hh index 4ba3fb37d4c..6568410e3bd 100644 --- a/src/libstore/derived-path.hh +++ b/src/libstore/derived-path.hh @@ -77,7 +77,7 @@ struct SingleDerivedPathBuilt { const StoreDirConfig & store, ref drvPath, OutputNameView outputs, const ExperimentalFeatureSettings & xpSettings = experimentalFeatureSettings); - nlohmann::json toJSON(Store & store) const; + nlohmann::json toJSON(const StoreDirConfig & store) const; bool operator == (const SingleDerivedPathBuilt &) const noexcept; std::strong_ordering operator <=> (const SingleDerivedPathBuilt &) const noexcept; @@ -151,7 +151,7 @@ struct SingleDerivedPath : _SingleDerivedPathRaw { const StoreDirConfig & store, std::string_view, const ExperimentalFeatureSettings & xpSettings = experimentalFeatureSettings); - nlohmann::json toJSON(Store & store) const; + nlohmann::json toJSON(const StoreDirConfig & store) const; }; static inline ref makeConstantStorePathRef(StorePath drvPath) @@ -204,7 +204,7 @@ struct DerivedPathBuilt { const StoreDirConfig & store, ref, std::string_view, const ExperimentalFeatureSettings & xpSettings = experimentalFeatureSettings); - nlohmann::json toJSON(Store & store) const; + nlohmann::json toJSON(const StoreDirConfig & store) const; bool operator == (const DerivedPathBuilt &) const noexcept; // TODO libc++ 16 (used by darwin) missing `std::set::operator <=>`, can't do yet. @@ -285,7 +285,7 @@ struct DerivedPath : _DerivedPathRaw { */ static DerivedPath fromSingle(const SingleDerivedPath &); - nlohmann::json toJSON(Store & store) const; + nlohmann::json toJSON(const StoreDirConfig & store) const; }; typedef std::vector DerivedPaths; diff --git a/tests/functional/build-dry.sh b/tests/functional/build-dry.sh index cff0f9a49fe..7d09c7f4532 100755 --- a/tests/functional/build-dry.sh +++ b/tests/functional/build-dry.sh @@ -58,14 +58,7 @@ clearCache RES=$(nix build -f dependencies.nix --dry-run --json) -if [[ -z "${NIX_TESTS_CA_BY_DEFAULT-}" ]]; then - echo "$RES" | jq '.[0] | [ - (.drvPath | test("'"$NIX_STORE_DIR"'.*\\.drv")), - (.outputs.out | test("'"$NIX_STORE_DIR"'")) - ] | all' -else - echo "$RES" | jq '.[0] | [ - (.drvPath | test("'"$NIX_STORE_DIR"'.*\\.drv")), - .outputs.out == null - ] | all' -fi +echo "$RES" | jq '.[0] | [ + (.drvPath | test("'"$NIX_STORE_DIR"'.*\\.drv")), + .outputs == [ "out" ] +] | all' From 40fc7b7a37d1d95d18d8c56828a681d36ca751e1 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 12 Feb 2025 23:54:24 -0500 Subject: [PATCH 3/5] Revert "Use the hash modulo in the derivation outputs" This reverts commit bab1cda0e6c30e25460b5a9c809589d3948f35df. --- maintainers/flake-module.nix | 2 - src/build-remote/build-remote.cc | 6 +- src/libcmd/built-path.cc | 23 +- src/libexpr/primops.cc | 26 +- src/libstore-tests/common-protocol.cc | 45 ---- src/libstore-tests/serve-protocol.cc | 61 ++--- src/libstore-tests/worker-protocol.cc | 73 ++---- src/libstore/binary-cache-store.cc | 35 ++- src/libstore/binary-cache-store.hh | 22 +- ...erivation-creation-and-realisation-goal.hh | 2 +- src/libstore/build/derivation-goal.cc | 74 +++--- src/libstore/build/derivation-goal.hh | 1 - .../build/drv-output-substitution-goal.cc | 67 ++---- .../build/drv-output-substitution-goal.hh | 10 +- src/libstore/ca-specific-schema.sql | 27 --- src/libstore/common-protocol.cc | 26 -- src/libstore/common-protocol.hh | 3 - src/libstore/daemon.cc | 26 +- src/libstore/derivations.cc | 225 +++++++++++------- src/libstore/derivations.hh | 65 +++-- src/libstore/dummy-store.cc | 2 +- src/libstore/legacy-ssh-store.hh | 2 +- src/libstore/local-overlay-store.cc | 8 +- src/libstore/local-overlay-store.hh | 2 +- src/libstore/local-store.cc | 73 +----- src/libstore/local-store.hh | 6 +- src/libstore/misc.cc | 75 +----- src/libstore/nar-info-disk-cache.cc | 63 +++-- src/libstore/realisation.cc | 214 +++++++---------- src/libstore/realisation.hh | 162 ++++++++----- src/libstore/remote-store.cc | 42 +--- src/libstore/remote-store.hh | 2 +- src/libstore/serve-protocol.cc | 97 +++++++- src/libstore/serve-protocol.hh | 11 +- src/libstore/store-api.cc | 61 +++-- src/libstore/store-api.hh | 13 +- .../unix/build/local-derivation-goal.cc | 30 ++- src/libstore/worker-protocol.cc | 117 ++++++++- src/libstore/worker-protocol.hh | 13 +- src/nix/develop.cc | 10 +- src/nix/realisation.cc | 2 +- src/perl/lib/Nix/Store.xs | 11 +- 42 files changed, 897 insertions(+), 938 deletions(-) diff --git a/maintainers/flake-module.nix b/maintainers/flake-module.nix index 208296194a1..5ec6fe4530b 100644 --- a/maintainers/flake-module.nix +++ b/maintainers/flake-module.nix @@ -257,8 +257,6 @@ ''^src/libstore/pathlocks\.hh$'' ''^src/libstore/profiles\.cc$'' ''^src/libstore/profiles\.hh$'' - ''^src/libstore/realisation\.cc$'' - ''^src/libstore/realisation\.hh$'' ''^src/libstore/remote-fs-accessor\.cc$'' ''^src/libstore/remote-fs-accessor\.hh$'' ''^src/libstore/remote-store-connection\.hh$'' diff --git a/src/build-remote/build-remote.cc b/src/build-remote/build-remote.cc index 82ad7d86212..77d51e261ab 100644 --- a/src/build-remote/build-remote.cc +++ b/src/build-remote/build-remote.cc @@ -345,13 +345,11 @@ static int main_build_remote(int argc, char * * argv) } - auto outputHashes = staticOutputHashes(*store, drv); std::set missingRealisations; StorePathSet missingPaths; if (experimentalFeatureSettings.isEnabled(Xp::CaDerivations) && !drv.type().hasKnownOutputPaths()) { for (auto & outputName : wantedOutputs) { - auto thisOutputHash = outputHashes.at(outputName); - auto thisOutputId = DrvOutput{ thisOutputHash, outputName }; + auto thisOutputId = DrvOutput{ *drvPath, outputName }; if (!store->queryRealisation(thisOutputId)) { debug("missing output %s", outputName); assert(optResult); @@ -359,7 +357,7 @@ static int main_build_remote(int argc, char * * argv) auto i = result.builtOutputs.find(outputName); assert(i != result.builtOutputs.end()); auto & newRealisation = i->second; - missingRealisations.insert(newRealisation); + missingRealisations.insert({newRealisation, thisOutputId}); missingPaths.insert(newRealisation.outPath); } } diff --git a/src/libcmd/built-path.cc b/src/libcmd/built-path.cc index 905e70f32c9..d3677713dcb 100644 --- a/src/libcmd/built-path.cc +++ b/src/libcmd/built-path.cc @@ -116,21 +116,16 @@ RealisedPath::Set BuiltPath::toRealisedPaths(Store & store) const overloaded{ [&](const BuiltPath::Opaque & p) { res.insert(p.path); }, [&](const BuiltPath::Built & p) { - auto drvHashes = - staticOutputHashes(store, store.readDerivation(p.drvPath->outPath())); for (auto& [outputName, outputPath] : p.outputs) { - if (experimentalFeatureSettings.isEnabled( - Xp::CaDerivations)) { - auto drvOutput = get(drvHashes, outputName); - if (!drvOutput) - throw Error( - "the derivation '%s' has unrealised output '%s' (derived-path.cc/toRealisedPaths)", - store.printStorePath(p.drvPath->outPath()), outputName); - auto thisRealisation = store.queryRealisation( - DrvOutput{*drvOutput, outputName}); - assert(thisRealisation); // We’ve built it, so we must - // have the realisation - res.insert(*thisRealisation); + if (experimentalFeatureSettings.isEnabled(Xp::CaDerivations)) { + DrvOutput key{ + .drvPath = p.drvPath->outPath(), + .outputName = outputName, + }; + auto thisRealisation = store.queryRealisation(key); + // We’ve built it, so we must have the realisation. + assert(thisRealisation); + res.insert(Realisation{*thisRealisation, key}); } else { res.insert(outputPath); } diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 51d2991e799..e3dedc9549c 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -1522,31 +1522,7 @@ static void derivationStrictInternal( DerivationOutput::Deferred { }); } - auto hashModulo = hashDerivationModulo(*state.store, Derivation(drv), true); - switch (hashModulo.kind) { - case DrvHash::Kind::Regular: - for (auto & i : outputs) { - auto h = get(hashModulo.hashes, i); - if (!h) - state.error( - "derivation produced no hash for output '%s'", - i - ).atPos(v).debugThrow(); - auto outPath = state.store->makeOutputPath(i, *h, drvName); - drv.env[i] = state.store->printStorePath(outPath); - drv.outputs.insert_or_assign( - i, - DerivationOutput::InputAddressed { - .path = std::move(outPath), - }); - } - break; - ; - case DrvHash::Kind::Deferred: - for (auto & i : outputs) { - drv.outputs.insert_or_assign(i, DerivationOutput::Deferred {}); - } - } + resolveInputAddressed(*state.store, drv); } /* Write the resulting term into the Nix store directory. */ diff --git a/src/libstore-tests/common-protocol.cc b/src/libstore-tests/common-protocol.cc index c8f6dd002d5..167fc7f81eb 100644 --- a/src/libstore-tests/common-protocol.cc +++ b/src/libstore-tests/common-protocol.cc @@ -96,51 +96,6 @@ CHARACTERIZATION_TEST( }, })) -CHARACTERIZATION_TEST( - drvOutput, - "drv-output", - (std::tuple { - { - .drvHash = Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), - .outputName = "baz", - }, - DrvOutput { - .drvHash = Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), - .outputName = "quux", - }, - })) - -CHARACTERIZATION_TEST( - realisation, - "realisation", - (std::tuple { - Realisation { - .id = DrvOutput { - .drvHash = Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), - .outputName = "baz", - }, - .outPath = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo" }, - .signatures = { "asdf", "qwer" }, - }, - Realisation { - .id = { - .drvHash = Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), - .outputName = "baz", - }, - .outPath = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo" }, - .signatures = { "asdf", "qwer" }, - .dependentRealisations = { - { - DrvOutput { - .drvHash = Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), - .outputName = "quux", - }, - StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo" }, - }, - }, - }, - })) - CHARACTERIZATION_TEST( vector, "vector", diff --git a/src/libstore-tests/serve-protocol.cc b/src/libstore-tests/serve-protocol.cc index 3dbbf38799a..52635ba3b81 100644 --- a/src/libstore-tests/serve-protocol.cc +++ b/src/libstore-tests/serve-protocol.cc @@ -73,51 +73,44 @@ VERSIONED_CHARACTERIZATION_TEST( VERSIONED_CHARACTERIZATION_TEST( ServeProtoTest, drvOutput, - "drv-output", - defaultVersion, + "drv-output-2.8", + 2 << 8 | 8, (std::tuple { { - .drvHash = Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), + .drvPath = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo.drv" }, .outputName = "baz", }, DrvOutput { - .drvHash = Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), + .drvPath = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo.drv" }, .outputName = "quux", }, })) #endif +VERSIONED_CHARACTERIZATION_TEST( + ServeProtoTest, + unkeyedRealisation, + "unkeyed-realisation-2.8", + 2 << 8 | 8, + (UnkeyedRealisation { + .outPath = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo" }, + .signatures = { "asdf", "qwer" }, + })) + VERSIONED_CHARACTERIZATION_TEST( ServeProtoTest, realisation, - "realisation", - defaultVersion, - (std::tuple { - Realisation { - .id = DrvOutput { - .drvHash = Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), - .outputName = "baz", - }, + "realisation-2.8", + 2 << 8 | 8, + (Realisation { + UnkeyedRealisation { .outPath = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo" }, .signatures = { "asdf", "qwer" }, }, - Realisation { - .id = { - .drvHash = Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), - .outputName = "baz", - }, - .outPath = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo" }, - .signatures = { "asdf", "qwer" }, - .dependentRealisations = { - { - DrvOutput { - .drvHash = Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), - .outputName = "quux", - }, - StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo" }, - }, - }, + DrvOutput { + .drvPath = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo.drv" }, + .outputName = "baz", }, })) @@ -176,8 +169,8 @@ VERSIONED_CHARACTERIZATION_TEST( VERSIONED_CHARACTERIZATION_TEST( ServeProtoTest, buildResult_2_6, - "build-result-2.6", - 2 << 8 | 6, + "build-result-2.8", + 2 << 8 | 8, ({ using namespace std::literals::chrono_literals; std::tuple t { @@ -200,20 +193,12 @@ VERSIONED_CHARACTERIZATION_TEST( { "foo", { - .id = DrvOutput { - .drvHash = Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), - .outputName = "foo", - }, .outPath = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo" }, }, }, { "bar", { - .id = DrvOutput { - .drvHash = Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), - .outputName = "bar", - }, .outPath = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar" }, }, }, diff --git a/src/libstore-tests/worker-protocol.cc b/src/libstore-tests/worker-protocol.cc index 99b042d5ba4..a34399b579f 100644 --- a/src/libstore-tests/worker-protocol.cc +++ b/src/libstore-tests/worker-protocol.cc @@ -125,49 +125,42 @@ VERSIONED_CHARACTERIZATION_TEST( VERSIONED_CHARACTERIZATION_TEST( WorkerProtoTest, drvOutput, - "drv-output", - defaultVersion, + "drv-output-1.39", + 1 << 8 | 39, (std::tuple { { - .drvHash = Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), + .drvPath = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo.drv" }, .outputName = "baz", }, DrvOutput { - .drvHash = Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), + .drvPath = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo.drv" }, .outputName = "quux", }, })) +VERSIONED_CHARACTERIZATION_TEST( + WorkerProtoTest, + unkeyedRealisation, + "unkeyed-realisation-1.39", + 1 << 8 | 39, + (UnkeyedRealisation { + .outPath = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo" }, + .signatures = { "asdf", "qwer" }, + })) + VERSIONED_CHARACTERIZATION_TEST( WorkerProtoTest, realisation, - "realisation", - defaultVersion, - (std::tuple { - Realisation { - .id = DrvOutput { - .drvHash = Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), - .outputName = "baz", - }, + "realisation-1.39", + 1 << 8 | 39, + (Realisation { + UnkeyedRealisation { .outPath = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo" }, .signatures = { "asdf", "qwer" }, }, - Realisation { - .id = { - .drvHash = Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), - .outputName = "baz", - }, - .outPath = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo" }, - .signatures = { "asdf", "qwer" }, - .dependentRealisations = { - { - DrvOutput { - .drvHash = Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), - .outputName = "quux", - }, - StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo" }, - }, - }, + DrvOutput { + .drvPath = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo.drv" }, + .outputName = "baz", }, })) @@ -216,20 +209,12 @@ VERSIONED_CHARACTERIZATION_TEST( { "foo", { - .id = DrvOutput { - .drvHash = Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), - .outputName = "foo", - }, .outPath = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo" }, }, }, { "bar", { - .id = DrvOutput { - .drvHash = Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), - .outputName = "bar", - }, .outPath = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar" }, }, }, @@ -266,20 +251,12 @@ VERSIONED_CHARACTERIZATION_TEST( { "foo", { - .id = DrvOutput { - .drvHash = Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), - .outputName = "foo", - }, .outPath = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo" }, }, }, { "bar", { - .id = DrvOutput { - .drvHash = Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), - .outputName = "bar", - }, .outPath = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar" }, }, }, @@ -318,20 +295,12 @@ VERSIONED_CHARACTERIZATION_TEST( { "foo", { - .id = DrvOutput { - .drvHash = Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), - .outputName = "foo", - }, .outPath = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo" }, }, }, { "bar", { - .id = DrvOutput { - .drvHash = Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), - .outputName = "bar", - }, .outPath = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar" }, }, }, diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index 896779f85fc..32e0bd64ac9 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -481,22 +481,36 @@ StorePath BinaryCacheStore::addToStore( })->path; } +std::string BinaryCacheStore::makeRealisationPath(const DrvOutput & id) +{ + return realisationsPrefix + + "/" + id.drvPath.to_string() + + "/" + id.outputName + + ".doi"; +} + void BinaryCacheStore::queryRealisationUncached(const DrvOutput & id, - Callback> callback) noexcept + Callback> callback) noexcept { - auto outputInfoFilePath = realisationsPrefix + "/" + id.to_string() + ".doi"; + auto outputInfoFilePath = makeRealisationPath(id); auto callbackPtr = std::make_shared(std::move(callback)); Callback> newCallback = { - [=](std::future> fut) { + [=,this](std::future> fut) { try { auto data = fut.get(); if (!data) return (*callbackPtr)({}); - auto realisation = Realisation::fromJSON( - nlohmann::json::parse(*data), outputInfoFilePath); - return (*callbackPtr)(std::make_shared(realisation)); + UnkeyedRealisation realisation { .outPath = StorePath::dummy, }; + auto json = nlohmann::json::parse(*data); + try { + realisation = UnkeyedRealisation::fromJSON(*this, json); + } catch (Error & e) { + e.addTrace({}, "reading build trace key-value file '%s'", outputInfoFilePath); + throw; + } + return (*callbackPtr)(std::make_shared(realisation)); } catch (...) { callbackPtr->rethrow(); } @@ -506,11 +520,14 @@ void BinaryCacheStore::queryRealisationUncached(const DrvOutput & id, getFile(outputInfoFilePath, std::move(newCallback)); } -void BinaryCacheStore::registerDrvOutput(const Realisation& info) { +void BinaryCacheStore::registerDrvOutput(const Realisation & info) +{ if (diskCache) diskCache->upsertRealisation(getUri(), info); - auto filePath = realisationsPrefix + "/" + info.id.to_string() + ".doi"; - upsertFile(filePath, info.toJSON().dump(), "application/json"); + upsertFile( + makeRealisationPath(info.id), + info.toJSON(*this).dump(), + "application/json"); } ref BinaryCacheStore::getFSAccessor(bool requireValidPath) diff --git a/src/libstore/binary-cache-store.hh b/src/libstore/binary-cache-store.hh index 6bd7fd14ac9..4f122d463b0 100644 --- a/src/libstore/binary-cache-store.hh +++ b/src/libstore/binary-cache-store.hh @@ -61,13 +61,27 @@ private: protected: - // The prefix under which realisation infos will be stored - const std::string realisationsPrefix = "realisations"; + /** + * The prefix under which realisation infos will be stored + * + * @note The previous (still experimental, though) hash-keyed + * realisations were under "realisations". "build trace" is a better + * name anyways (issue #11895), and this serves as some light + * versioning. + */ + constexpr const static std::string realisationsPrefix = "build-trace"; - const std::string cacheInfoFile = "nix-cache-info"; + constexpr const static std::string cacheInfoFile = "nix-cache-info"; BinaryCacheStore(const Params & params); + /** + * Compute the path to the given realisation + * + * It's `${realisationsPrefix}/${drvPath}/${outputName}`. + */ + std::string makeRealisationPath(const DrvOutput & id); + public: virtual bool fileExists(const std::string & path) = 0; @@ -151,7 +165,7 @@ public: void registerDrvOutput(const Realisation & info) override; void queryRealisationUncached(const DrvOutput &, - Callback> callback) noexcept override; + Callback> callback) noexcept override; void narFromPath(const StorePath & path, Sink & sink) override; diff --git a/src/libstore/build/derivation-creation-and-realisation-goal.hh b/src/libstore/build/derivation-creation-and-realisation-goal.hh index 9960484ac95..9b3b1a1afe3 100644 --- a/src/libstore/build/derivation-creation-and-realisation-goal.hh +++ b/src/libstore/build/derivation-creation-and-realisation-goal.hh @@ -53,7 +53,7 @@ struct DerivationCreationAndRealisationGoal : public Goal * * - For input-addressed derivations, always the precomputed paths * - * - For content-addressed derivations, calcuated from whatever the + * - For content-addressing derivations, calcuated from whatever the * hash ends up being. (Note that fixed outputs derivations that * produce the "wrong" output still install that data under its * true content-address.) diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 3f652c0ca5b..71abd6d30d2 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -181,11 +181,9 @@ Goal::Co DerivationGoal::haveDerivation() if (impure) experimentalFeatureSettings.require(Xp::ImpureDerivations); - auto outputHashes = staticOutputHashes(worker.evalStore, *drv); - for (auto & [outputName, outputHash] : outputHashes) { + for (auto & [outputName, _] : drv->outputs) { InitialOutput v{ .wanted = true, // Will be refined later - .outputHash = outputHash }; /* TODO we might want to also allow randomizing the paths @@ -231,7 +229,7 @@ Goal::Co DerivationGoal::haveDerivation() addWaitee( upcast_goal( worker.makeDrvOutputSubstitutionGoal( - DrvOutput{status.outputHash, outputName}, + DrvOutput{drvPath, outputName}, buildMode == bmRepair ? Repair : NoRepair ) ) @@ -1032,47 +1030,34 @@ Goal::Co DerivationGoal::resolvedFinished() SingleDrvOutputs builtOutputs; if (resolvedResult.success()) { - auto resolvedHashes = staticOutputHashes(worker.store, resolvedDrv); - StorePathSet outputPaths; for (auto & outputName : resolvedDrv.outputNames()) { auto initialOutput = get(initialOutputs, outputName); - auto resolvedHash = get(resolvedHashes, outputName); - if ((!initialOutput) || (!resolvedHash)) + if ((!initialOutput)) throw Error( "derivation '%s' doesn't have expected output '%s' (derivation-goal.cc/resolvedFinished,resolve)", worker.store.printStorePath(drvPath), outputName); auto realisation = [&]{ - auto take1 = get(resolvedResult.builtOutputs, outputName); - if (take1) return *take1; - - /* The above `get` should work. But sateful tracking of - outputs in resolvedResult, this can get out of sync with the - store, which is our actual source of truth. For now we just - check the store directly if it fails. */ - auto take2 = worker.evalStore.queryRealisation(DrvOutput { *resolvedHash, outputName }); - if (take2) return *take2; - - throw Error( - "derivation '%s' doesn't have expected output '%s' (derivation-goal.cc/resolvedFinished,realisation)", - worker.store.printStorePath(resolvedDrvGoal->drvPath), outputName); + auto take1 = get(resolvedResult.builtOutputs, outputName); + if (take1) return *take1; + + /* The above `get` should work. But stateful tracking of + outputs in resolvedResult, this can get out of sync with the + store, which is our actual source of truth. For now we just + check the store directly if it fails. */ + auto take2 = worker.evalStore.queryRealisation(DrvOutput { + .drvPath = resolvedDrvGoal->drvPath, + .outputName = outputName, + }); + if (take2) return *take2; + + throw Error( + "derivation '%s' doesn't have expected output '%s' (derivation-goal.cc/resolvedFinished,realisation)", + worker.store.printStorePath(resolvedDrvGoal->drvPath), outputName); }(); - if (!drv->type().isImpure()) { - auto newRealisation = realisation; - newRealisation.id = DrvOutput { initialOutput->outputHash, outputName }; - newRealisation.signatures.clear(); - if (!drv->type().isFixed()) { - auto & drvStore = worker.evalStore.isValidPath(drvPath) - ? worker.evalStore - : worker.store; - newRealisation.dependentRealisations = drvOutputReferences(worker.store, *drv, realisation.outPath, &drvStore); - } - signRealisation(newRealisation); - worker.store.registerDrvOutput(newRealisation); - } outputPaths.insert(realisation.outPath); builtOutputs.emplace(outputName, realisation); } @@ -1437,7 +1422,7 @@ std::pair DerivationGoal::checkPathValidity() : PathStatus::Corrupt, }; } - auto drvOutput = DrvOutput{info.outputHash, i.first}; + auto drvOutput = DrvOutput{drvPath, i.first}; if (experimentalFeatureSettings.isEnabled(Xp::CaDerivations)) { if (auto real = worker.store.queryRealisation(drvOutput)) { info.known = { @@ -1449,16 +1434,21 @@ std::pair DerivationGoal::checkPathValidity() // derivation, and the output path is valid, but we don't have // its realisation stored (probably because it has been built // without the `ca-derivations` experimental flag). - worker.store.registerDrvOutput( - Realisation { - drvOutput, - info.known->path, - } - ); + worker.store.registerDrvOutput(Realisation { + { + .outPath = info.known->path, + }, + drvOutput, + }); } } if (info.known && info.known->isValid()) - validOutputs.emplace(i.first, Realisation { drvOutput, info.known->path }); + validOutputs.emplace(i.first, Realisation { + { + .outPath = info.known->path, + }, + drvOutput, + }); } // If we requested all the outputs, we are always fine. diff --git a/src/libstore/build/derivation-goal.hh b/src/libstore/build/derivation-goal.hh index 53884081301..665c83463c5 100644 --- a/src/libstore/build/derivation-goal.hh +++ b/src/libstore/build/derivation-goal.hh @@ -50,7 +50,6 @@ struct InitialOutputStatus { struct InitialOutput { bool wanted; - Hash outputHash; std::optional known; }; diff --git a/src/libstore/build/drv-output-substitution-goal.cc b/src/libstore/build/drv-output-substitution-goal.cc index f069c0d9404..0972555740f 100644 --- a/src/libstore/build/drv-output-substitution-goal.cc +++ b/src/libstore/build/drv-output-substitution-goal.cc @@ -14,7 +14,7 @@ DrvOutputSubstitutionGoal::DrvOutputSubstitutionGoal( : Goal(worker, DerivedPath::Opaque { StorePath::dummy }) , id(id) { - name = fmt("substitution of '%s'", id.to_string()); + name = fmt("substitution of '%s'", id.render(worker.store)); trace("created"); } @@ -45,11 +45,11 @@ Goal::Co DrvOutputSubstitutionGoal::init() outPipe->createAsyncPipe(worker.ioport.get()); #endif - auto promise = std::make_shared>>(); + auto promise = std::make_shared>>(); sub->queryRealisation( id, - { [outPipe(outPipe), promise(promise)](std::future> res) { + { [outPipe(outPipe), promise(promise)](std::future> res) { try { Finally updateStats([&]() { outPipe->writeSide.close(); }); promise->set_value(res.get()); @@ -70,12 +70,6 @@ Goal::Co DrvOutputSubstitutionGoal::init() worker.childTerminated(this); - /* - * The realisation corresponding to the given output id. - * Will be filled once we can get it. - */ - std::shared_ptr outputInfo; - try { outputInfo = promise->get_future().get(); } catch (std::exception & e) { @@ -85,36 +79,25 @@ Goal::Co DrvOutputSubstitutionGoal::init() if (!outputInfo) continue; - bool failed = false; - - for (const auto & [depId, depPath] : outputInfo->dependentRealisations) { - if (depId != id) { - if (auto localOutputInfo = worker.store.queryRealisation(depId); - localOutputInfo && localOutputInfo->outPath != depPath) { - warn( - "substituter '%s' has an incompatible realisation for '%s', ignoring.\n" - "Local: %s\n" - "Remote: %s", - sub->getUri(), - depId.to_string(), - worker.store.printStorePath(localOutputInfo->outPath), - worker.store.printStorePath(depPath) - ); - failed = true; - break; - } - addWaitee(worker.makeDrvOutputSubstitutionGoal(depId)); - } + addWaitee(worker.makePathSubstitutionGoal(outputInfo->outPath)); + co_await Suspend{}; + + trace("output path substituted"); + + if (nrFailed > 0) { + debug("The output path of the derivation output '%s' could not be substituted", id.render(worker.store)); + co_return amDone(nrNoSubstituters > 0 || nrIncompleteClosure > 0 ? ecIncompleteClosure : ecFailed); } - if (failed) continue; + worker.store.registerDrvOutput({*outputInfo, id}); - co_return realisationFetched(outputInfo, sub); + trace("finished"); + co_return amDone(ecSuccess); } /* None left. Terminate this goal and let someone else deal with it. */ - debug("derivation output '%s' is required, but there is no substituter that can provide it", id.to_string()); + debug("derivation output '%s' is required, but there is no substituter that can provide it", id.render(worker.store)); if (substituterFailed) { worker.failedSubstitutions++; @@ -127,29 +110,11 @@ Goal::Co DrvOutputSubstitutionGoal::init() co_return amDone(substituterFailed ? ecFailed : ecNoSubstituters); } -Goal::Co DrvOutputSubstitutionGoal::realisationFetched(std::shared_ptr outputInfo, nix::ref sub) { - addWaitee(worker.makePathSubstitutionGoal(outputInfo->outPath)); - - if (!waitees.empty()) co_await Suspend{}; - - trace("output path substituted"); - - if (nrFailed > 0) { - debug("The output path of the derivation output '%s' could not be substituted", id.to_string()); - co_return amDone(nrNoSubstituters > 0 || nrIncompleteClosure > 0 ? ecIncompleteClosure : ecFailed); - } - - worker.store.registerDrvOutput(*outputInfo); - - trace("finished"); - co_return amDone(ecSuccess); -} - std::string DrvOutputSubstitutionGoal::key() { /* "a$" ensures substitution goals happen before derivation goals. */ - return "a$" + std::string(id.to_string()); + return "a$" + std::string(id.render(worker.store)); } void DrvOutputSubstitutionGoal::handleEOF(Descriptor fd) diff --git a/src/libstore/build/drv-output-substitution-goal.hh b/src/libstore/build/drv-output-substitution-goal.hh index 8c60d01987a..cfec79a225b 100644 --- a/src/libstore/build/drv-output-substitution-goal.hh +++ b/src/libstore/build/drv-output-substitution-goal.hh @@ -28,13 +28,15 @@ class DrvOutputSubstitutionGoal : public Goal { DrvOutput id; public: - DrvOutputSubstitutionGoal(const DrvOutput& id, Worker & worker, RepairFlag repair = NoRepair, std::optional ca = std::nullopt); + DrvOutputSubstitutionGoal(const DrvOutput & id, Worker & worker, RepairFlag repair = NoRepair, std::optional ca = std::nullopt); - typedef void (DrvOutputSubstitutionGoal::*GoalState)(); - GoalState state; + /** + * The realisation corresponding to the given output id. + * Will be filled once we can get it. + */ + std::shared_ptr outputInfo; Co init() override; - Co realisationFetched(std::shared_ptr outputInfo, nix::ref sub); void timedOut(Error && ex) override { unreachable(); }; diff --git a/src/libstore/ca-specific-schema.sql b/src/libstore/ca-specific-schema.sql index c5e4e389799..d563b33d879 100644 --- a/src/libstore/ca-specific-schema.sql +++ b/src/libstore/ca-specific-schema.sql @@ -12,30 +12,3 @@ create table if not exists Realisations ( ); create index if not exists IndexRealisations on Realisations(drvPath, outputName); - --- We can end-up in a weird edge-case where a path depends on itself because --- it’s an output of a CA derivation, that happens to be the same as one of its --- dependencies. --- In that case we have a dependency loop (path -> realisation1 -> realisation2 --- -> path) that we need to break by removing the dependencies between the --- realisations -create trigger if not exists DeleteSelfRefsViaRealisations before delete on ValidPaths - begin - delete from RealisationsRefs where realisationReference in ( - select id from Realisations where outputPath = old.id - ); - end; - -create table if not exists RealisationsRefs ( - referrer integer not null, - realisationReference integer, - foreign key (referrer) references Realisations(id) on delete cascade, - foreign key (realisationReference) references Realisations(id) on delete restrict -); --- used by deletion trigger -create index if not exists IndexRealisationsRefsRealisationReference on RealisationsRefs(realisationReference); - --- used by QueryRealisationReferences -create index if not exists IndexRealisationsRefs on RealisationsRefs(referrer); --- used by cascade deletion when ValidPaths is deleted -create index if not exists IndexRealisationsRefsOnOutputPath on Realisations(outputPath); diff --git a/src/libstore/common-protocol.cc b/src/libstore/common-protocol.cc index fc2b5ac6f3f..82b701eaaab 100644 --- a/src/libstore/common-protocol.cc +++ b/src/libstore/common-protocol.cc @@ -46,32 +46,6 @@ void CommonProto::Serialise::write(const StoreDirConfig & store, } -Realisation CommonProto::Serialise::read(const StoreDirConfig & store, CommonProto::ReadConn conn) -{ - std::string rawInput = readString(conn.from); - return Realisation::fromJSON( - nlohmann::json::parse(rawInput), - "remote-protocol" - ); -} - -void CommonProto::Serialise::write(const StoreDirConfig & store, CommonProto::WriteConn conn, const Realisation & realisation) -{ - conn.to << realisation.toJSON().dump(); -} - - -DrvOutput CommonProto::Serialise::read(const StoreDirConfig & store, CommonProto::ReadConn conn) -{ - return DrvOutput::parse(readString(conn.from)); -} - -void CommonProto::Serialise::write(const StoreDirConfig & store, CommonProto::WriteConn conn, const DrvOutput & drvOutput) -{ - conn.to << drvOutput.to_string(); -} - - std::optional CommonProto::Serialise>::read(const StoreDirConfig & store, CommonProto::ReadConn conn) { auto s = readString(conn.from); diff --git a/src/libstore/common-protocol.hh b/src/libstore/common-protocol.hh index a878e84c9d8..d2e1f1b8ff0 100644 --- a/src/libstore/common-protocol.hh +++ b/src/libstore/common-protocol.hh @@ -12,7 +12,6 @@ struct Source; class StorePath; struct ContentAddress; struct DrvOutput; -struct Realisation; /** @@ -69,8 +68,6 @@ template<> DECLARE_COMMON_SERIALISER(ContentAddress); template<> DECLARE_COMMON_SERIALISER(DrvOutput); -template<> -DECLARE_COMMON_SERIALISER(Realisation); template DECLARE_COMMON_SERIALISER(std::vector); diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index d6745f51612..f9efe14ddaf 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -960,32 +960,30 @@ static void performOp(TunnelLogger * logger, ref store, case WorkerProto::Op::RegisterDrvOutput: { logger->startWork(); - if (GET_PROTOCOL_MINOR(conn.protoVersion) < 31) { - auto outputId = DrvOutput::parse(readString(conn.from)); - auto outputPath = StorePath(readString(conn.from)); - store->registerDrvOutput(Realisation{ - .id = outputId, .outPath = outputPath}); - } else { - auto realisation = WorkerProto::Serialise::read(*store, rconn); - store->registerDrvOutput(realisation); - } + // TODO move to WorkerProto::Serialise and friends + //if (GET_PROTOCOL_MINOR(conn.protoVersion) < 39) { + // throw Error("old-style build traces no longer supported"); + //} + auto realisation = WorkerProto::Serialise::read(*store, rconn); + store->registerDrvOutput(realisation); logger->stopWork(); break; } case WorkerProto::Op::QueryRealisation: { logger->startWork(); - auto outputId = DrvOutput::parse(readString(conn.from)); - auto info = store->queryRealisation(outputId); + auto outputId = WorkerProto::Serialise::read(*store, rconn); + std::optional info = *store->queryRealisation(outputId); logger->stopWork(); if (GET_PROTOCOL_MINOR(conn.protoVersion) < 31) { std::set outPaths; if (info) outPaths.insert(info->outPath); WorkerProto::write(*store, wconn, outPaths); + } else if (GET_PROTOCOL_MINOR(conn.protoVersion) < 39) { + // No longer support this format + WorkerProto::write(*store, wconn, StringSet{}); } else { - std::set realisations; - if (info) realisations.insert(*info); - WorkerProto::write(*store, wconn, realisations); + WorkerProto::write(*store, wconn, info); } break; } diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index b54838a0aa9..3f2022c5bd9 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -787,7 +787,7 @@ Sync drvHashes; /* Look up the derivation by value and memoize the `hashDerivationModulo` call. */ -static const DrvHash pathDerivationModulo(Store & store, const StorePath & drvPath) +static const DrvHashModulo & pathDerivationModulo(Store & store, const StorePath & drvPath) { { auto hashes = drvHashes.lock(); @@ -796,13 +796,16 @@ static const DrvHash pathDerivationModulo(Store & store, const StorePath & drvPa return h->second; } } - auto h = hashDerivationModulo( - store, - store.readInvalidDerivation(drvPath), - false); + // Cache it - drvHashes.lock()->insert_or_assign(drvPath, h); - return h; + auto [iter, _] = drvHashes.lock()->insert_or_assign( + drvPath, + hashDerivationModulo( + store, + store.readInvalidDerivation(drvPath), + false)); + + return iter->second; } /* See the header for interface details. These are the implementation details. @@ -822,12 +825,10 @@ static const DrvHash pathDerivationModulo(Store & store, const StorePath & drvPa don't leak the provenance of fixed outputs, reducing pointless cache misses as the build itself won't know this. */ -DrvHash hashDerivationModulo(Store & store, const Derivation & drv, bool maskOutputs) +DrvHashModulo hashDerivationModulo(Store & store, const Derivation & drv, bool maskOutputs) { - auto type = drv.type(); - /* Return a fixed hash for fixed-output derivations. */ - if (type.isFixed()) { + if (drv.type().isFixed()) { std::map outputHashes; for (const auto & i : drv.outputs) { auto & dof = std::get(i.second.raw); @@ -837,58 +838,69 @@ DrvHash hashDerivationModulo(Store & store, const Derivation & drv, bool maskOut + store.printStorePath(dof.path(store, drv.name, i.first))); outputHashes.insert_or_assign(i.first, std::move(hash)); } - return DrvHash { - .hashes = outputHashes, - .kind = DrvHash::Kind::Regular, - }; + return outputHashes; } - auto kind = std::visit(overloaded { + if (std::visit(overloaded { [](const DerivationType::InputAddressed & ia) { /* This might be a "pesimistically" deferred output, so we don't "taint" the kind yet. */ - return DrvHash::Kind::Regular; + return false; }, [](const DerivationType::ContentAddressed & ca) { - return ca.fixed - ? DrvHash::Kind::Regular - : DrvHash::Kind::Deferred; + // Already covered + assert(!ca.fixed); + return true; }, - [](const DerivationType::Impure &) -> DrvHash::Kind { - return DrvHash::Kind::Deferred; + [](const DerivationType::Impure &) { + return true; } - }, drv.type().raw); + }, drv.type().raw)) { + return DrvHashModulo::DeferredDrv{}; + } + /* For other derivations, replace the inputs paths with recursive + calls to this function. */ DerivedPathMap::ChildNode::Map inputs2; for (auto & [drvPath, node] : drv.inputDrvs.map) { - const auto & res = pathDerivationModulo(store, drvPath); - if (res.kind == DrvHash::Kind::Deferred) - kind = DrvHash::Kind::Deferred; - for (auto & outputName : node.value) { - const auto h = get(res.hashes, outputName); - if (!h) - throw Error("no hash for output '%s' of derivation '%s'", outputName, drv.name); - inputs2[h->to_string(HashFormat::Base16, false)].value.insert(outputName); + /* Need to build and resolve dynamic derivations first */ + if (!node.childMap.empty()) { + return DrvHashModulo::DeferredDrv{}; } - } - - auto hash = hashString(HashAlgorithm::SHA256, drv.unparse(store, maskOutputs, &inputs2)); - std::map outputHashes; - for (const auto & [outputName, _] : drv.outputs) { - outputHashes.insert_or_assign(outputName, hash); + const auto & res = pathDerivationModulo(store, drvPath); + if (std::visit(overloaded { + [&](const DrvHashModulo::DeferredDrv &) { + return true; + }, + // Regular non-CA derivation, replace derivation + [&](const DrvHashModulo::DrvHash & drvHash) { + inputs2.insert_or_assign( + drvHash.to_string(HashFormat::Base16, false), + node); + return false; + }, + // CA derivation's output hashes + [&](const DrvHashModulo::CaOutputHashes & outputHashes) { + for (auto & outputName : node.value) { + /* Put each one in with a single "out" output.. */ + const auto h = get(outputHashes, outputName); + if (!h) + throw Error("no hash for output '%s' of derivation '%s'", outputName, drv.name); + inputs2.insert_or_assign( + h->to_string(HashFormat::Base16, false), + DerivedPathMap::ChildNode{ + .value = {"out"}, + }); + } + return false; + }, + }, res.raw)) { + return DrvHashModulo::DeferredDrv{}; + } } - return DrvHash { - .hashes = outputHashes, - .kind = kind, - }; -} - - -std::map staticOutputHashes(Store & store, const Derivation & drv) -{ - return hashDerivationModulo(store, drv, true).hashes; + return hashString(HashAlgorithm::SHA256, drv.unparse(store, maskOutputs, &inputs2)); } @@ -1029,25 +1041,39 @@ void BasicDerivation::applyRewrites(const StringMap & rewrites) env = std::move(newEnv); } -static void rewriteDerivation(Store & store, BasicDerivation & drv, const StringMap & rewrites) +void resolveInputAddressed(Store & store, Derivation & drv) { - drv.applyRewrites(rewrites); + std::optional hashModulo_; + + auto hashModulo = [&]() -> const auto & { + if (!hashModulo_) { + // somewhat expensive so we do lazily + hashModulo_ = hashDerivationModulo(store, drv, true); + } + return *hashModulo_; + }; - auto hashModulo = hashDerivationModulo(store, Derivation(drv), true); for (auto & [outputName, output] : drv.outputs) { if (std::holds_alternative(output.raw)) { - auto h = get(hashModulo.hashes, outputName); - if (!h) - throw Error("derivation '%s' output '%s' has no hash (derivations.cc/rewriteDerivation)", - drv.name, outputName); - auto outPath = store.makeOutputPath(outputName, *h, drv.name); - drv.env[outputName] = store.printStorePath(outPath); - output = DerivationOutput::InputAddressed { - .path = std::move(outPath), - }; + std::visit(overloaded { + [&](const DrvHashModulo::DrvHash & drvHash) { + auto outPath = store.makeOutputPath(outputName, drvHash, drv.name); + drv.env.insert_or_assign(outputName, store.printStorePath(outPath)); + output = DerivationOutput::InputAddressed { + .path = std::move(outPath), + }; + }, + [&](const DrvHashModulo::CaOutputHashes &) { + /* Shouldn't happen as the original output is + deferred (waiting to be input-addressed). */ + assert(false); + }, + [&](const DrvHashModulo::DeferredDrv &) { + // Nothing to do, already deferred + }, + }, hashModulo().raw); } } - } std::optional Derivation::tryResolve(Store & store, Store * evalStore) const @@ -1132,9 +1158,13 @@ std::optional Derivation::tryResolve( nullptr, inputDrv, inputNode, inputDrvOutputs)) return std::nullopt; - rewriteDerivation(store, resolved, inputRewrites); + resolved.applyRewrites(inputRewrites); - return resolved; + Derivation resolved2{std::move(resolved)}; + + resolveInputAddressed(store, resolved2); + + return resolved2; } @@ -1162,38 +1192,73 @@ void Derivation::checkInvariants(Store & store, const StorePath & drvPath) const // combinations that are currently prohibited. type(); - std::optional hashesModulo; - for (auto & i : outputs) { + std::optional hashModulo_; + + auto hashModulo = [&]() -> const auto & { + if (!hashModulo_) { + // somewhat expensive so we do lazily + hashModulo_ = hashDerivationModulo(store, *this, true); + } + return *hashModulo_; + }; + + for (auto & [outputName, output] : outputs) { std::visit(overloaded { [&](const DerivationOutput::InputAddressed & doia) { - if (!hashesModulo) { - // somewhat expensive so we do lazily - hashesModulo = hashDerivationModulo(store, *this, true); - } - auto currentOutputHash = get(hashesModulo->hashes, i.first); - if (!currentOutputHash) - throw Error("derivation '%s' has unexpected output '%s' (local-store / hashesModulo) named '%s'", - store.printStorePath(drvPath), store.printStorePath(doia.path), i.first); - StorePath recomputed = store.makeOutputPath(i.first, *currentOutputHash, drvName); - if (doia.path != recomputed) - throw Error("derivation '%s' has incorrect output '%s', should be '%s'", - store.printStorePath(drvPath), store.printStorePath(doia.path), store.printStorePath(recomputed)); - envHasRightPath(doia.path, i.first); + std::visit(overloaded { + [&](const DrvHashModulo::DrvHash & drvHash) { + StorePath recomputed = store.makeOutputPath(outputName, drvHash, drvName); + if (doia.path != recomputed) + throw Error( + "derivation '%s' has incorrect output '%s', should be '%s'", + store.printStorePath(drvPath), + store.printStorePath(doia.path), + store.printStorePath(recomputed)); + }, + [&](const DrvHashModulo::CaOutputHashes &) { + /* Shouldn't happen as the original output is + input-addressed. */ + assert(false); + }, + [&](const DrvHashModulo::DeferredDrv &) { + throw Error( + "derivation '%s' has output '%s', but derivation is not yet ready to be input-addressed", + store.printStorePath(drvPath), + store.printStorePath(doia.path)); + }, + }, hashModulo().raw); + envHasRightPath(doia.path, outputName); }, [&](const DerivationOutput::CAFixed & dof) { - auto path = dof.path(store, drvName, i.first); - envHasRightPath(path, i.first); + auto path = dof.path(store, drvName, outputName); + envHasRightPath(path, outputName); }, [&](const DerivationOutput::CAFloating &) { /* Nothing to check */ }, [&](const DerivationOutput::Deferred &) { /* Nothing to check */ + std::visit(overloaded { + [&](const DrvHashModulo::DrvHash & drvHash) { + throw Error( + "derivation '%s' has deferred output '%s', yet is ready to be input-addressed", + store.printStorePath(drvPath), + outputName); + }, + [&](const DrvHashModulo::CaOutputHashes &) { + /* Shouldn't happen as the original output is + input-addressed. */ + assert(false); + }, + [&](const DrvHashModulo::DeferredDrv &) { + /* Nothing to check */ + }, + }, hashModulo().raw); }, [&](const DerivationOutput::Impure &) { /* Nothing to check */ }, - }, i.second.raw); + }, output.raw); } } diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index 5b2101ed53c..19ea7e715d6 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -440,33 +440,43 @@ std::string outputPathName(std::string_view drvName, OutputNameView outputName); * derivations (fixed-output or not) will have a different hash for each * output. */ -struct DrvHash { +struct DrvHashModulo +{ /** - * Map from output names to hashes + * Single hash for the derivation + * + * This is for an input-addressed derivation that doesn't + * transitively depend on any floating-CA derivations. */ - std::map hashes; - - enum struct Kind : bool { - /** - * Statically determined derivations. - * This hash will be directly used to compute the output paths - */ - Regular, + using DrvHash = Hash; - /** - * Floating-output derivations (and their reverse dependencies). - */ - Deferred, - }; + /** + * Known CA drv's output hashes, for fixed-output derivations whose + * output hashes are always known since they are fixed up-front. + */ + using CaOutputHashes = std::map; /** - * The kind of derivation this is, simplified for just "derivation hash - * modulo" purposes. + * This derivation doesn't yet have known output hashes. + * + * Either because itself is floating CA, or it (transtively) depends + * on a floating CA derivation. */ - Kind kind; -}; + using DeferredDrv = std::monostate; + + using Raw = std::variant< + DrvHash, + CaOutputHashes, + DeferredDrv + >; -void operator |= (DrvHash::Kind & self, const DrvHash::Kind & other) noexcept; + Raw raw; + + bool operator == (const DrvHashModulo &) const = default; + //auto operator <=> (const DrvHashModulo &) const = default; + + MAKE_WRAPPER_CONSTRUCTOR(DrvHashModulo); +}; /** * Returns hashes with the details of fixed-output subderivations @@ -492,20 +502,23 @@ void operator |= (DrvHash::Kind & self, const DrvHash::Kind & other) noexcept; * ATerm, after subderivations have been likewise expunged from that * derivation. */ -DrvHash hashDerivationModulo(Store & store, const Derivation & drv, bool maskOutputs); +DrvHashModulo hashDerivationModulo(Store & store, const Derivation & drv, bool maskOutputs); + /** - * Return a map associating each output to a hash that uniquely identifies its - * derivation (modulo the self-references). + * If a derivation is input addressed and doesn't yet have its input + * addressed (is deferred) try using `hashDerivationModulo`. * - * \todo What is the Hash in this map? + * Does nothing if not deferred input-addressed, or + * `hashDerivationModulo` indicates it is missing inputs' output paths + * and is not yet ready (and must stay deferred). */ -std::map staticOutputHashes(Store & store, const Derivation & drv); +void resolveInputAddressed(Store & store, Derivation & drv); /** * Memoisation of hashDerivationModulo(). */ -typedef std::map DrvHashes; +typedef std::map DrvHashes; // FIXME: global, though at least thread-safe. extern Sync drvHashes; diff --git a/src/libstore/dummy-store.cc b/src/libstore/dummy-store.cc index c1e871e9384..40a5035b451 100644 --- a/src/libstore/dummy-store.cc +++ b/src/libstore/dummy-store.cc @@ -79,7 +79,7 @@ struct DummyStore : public virtual DummyStoreConfig, public virtual Store { unsupported("narFromPath"); } void queryRealisationUncached(const DrvOutput &, - Callback> callback) noexcept override + Callback> callback) noexcept override { callback(nullptr); } virtual ref getFSAccessor(bool requireValidPath) override diff --git a/src/libstore/legacy-ssh-store.hh b/src/libstore/legacy-ssh-store.hh index b541455b4e5..6e95dba6c94 100644 --- a/src/libstore/legacy-ssh-store.hh +++ b/src/libstore/legacy-ssh-store.hh @@ -133,7 +133,7 @@ public: } void queryRealisationUncached(const DrvOutput &, - Callback> callback) noexcept override + Callback> callback) noexcept override // TODO: Implement { unsupported("queryRealisation"); } }; diff --git a/src/libstore/local-overlay-store.cc b/src/libstore/local-overlay-store.cc index 56ff6bef3e5..7a7f08c63a2 100644 --- a/src/libstore/local-overlay-store.cc +++ b/src/libstore/local-overlay-store.cc @@ -62,7 +62,7 @@ void LocalOverlayStore::registerDrvOutput(const Realisation & info) // First do queryRealisation on lower layer to populate DB auto res = lowerStore->queryRealisation(info.id); if (res) - LocalStore::registerDrvOutput(*res); + LocalStore::registerDrvOutput({*res, info.id}); LocalStore::registerDrvOutput(info); } @@ -96,12 +96,12 @@ void LocalOverlayStore::queryPathInfoUncached(const StorePath & path, void LocalOverlayStore::queryRealisationUncached(const DrvOutput & drvOutput, - Callback> callback) noexcept + Callback> callback) noexcept { auto callbackPtr = std::make_shared(std::move(callback)); LocalStore::queryRealisationUncached(drvOutput, - {[this, drvOutput, callbackPtr](std::future> fut) { + {[this, drvOutput, callbackPtr](std::future> fut) { try { auto info = fut.get(); if (info) @@ -111,7 +111,7 @@ void LocalOverlayStore::queryRealisationUncached(const DrvOutput & drvOutput, } // If we don't have it, check lower store lowerStore->queryRealisation(drvOutput, - {[callbackPtr](std::future> fut) { + {[callbackPtr](std::future> fut) { try { (*callbackPtr)(fut.get()); } catch (...) { diff --git a/src/libstore/local-overlay-store.hh b/src/libstore/local-overlay-store.hh index 63628abed50..137d7ebc59e 100644 --- a/src/libstore/local-overlay-store.hh +++ b/src/libstore/local-overlay-store.hh @@ -158,7 +158,7 @@ private: * Check lower store if upper DB does not have. */ void queryRealisationUncached(const DrvOutput&, - Callback> callback) noexcept override; + Callback> callback) noexcept override; /** * Call `remountIfNecessary` after collecting garbage normally. diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 9fa68303f2c..8742594c3f2 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -91,8 +91,6 @@ struct LocalStore::State::Stmts { SQLiteStmt QueryAllRealisedOutputs; SQLiteStmt QueryPathFromHashPart; SQLiteStmt QueryValidPaths; - SQLiteStmt QueryRealisationReferences; - SQLiteStmt AddRealisationReference; }; LocalStore::LocalStore( @@ -358,19 +356,6 @@ LocalStore::LocalStore( where drvPath = ? ; )"); - state->stmts->QueryRealisationReferences.create(state->db, - R"( - select drvPath, outputName from Realisations - join RealisationsRefs on realisationReference = Realisations.id - where referrer = ?; - )"); - state->stmts->AddRealisationReference.create(state->db, - R"( - insert or replace into RealisationsRefs (referrer, realisationReference) - values ( - (select id from Realisations where drvPath = ? and outputName = ?), - (select id from Realisations where drvPath = ? and outputName = ?)); - )"); } } @@ -604,7 +589,7 @@ void LocalStore::registerDrvOutput(const Realisation & info) info.signatures.end()); state->stmts->UpdateRealisedOutput.use() (concatStringsSep(" ", combinedSignatures)) - (info.id.strHash()) + (info.id.drvPath.to_string()) (info.id.outputName) .exec(); } else { @@ -619,30 +604,12 @@ void LocalStore::registerDrvOutput(const Realisation & info) } } else { state->stmts->RegisterRealisedOutput.use() - (info.id.strHash()) + (info.id.drvPath.to_string()) (info.id.outputName) (printStorePath(info.outPath)) (concatStringsSep(" ", info.signatures)) .exec(); } - for (auto & [outputId, depPath] : info.dependentRealisations) { - auto localRealisation = queryRealisationCore_(*state, outputId); - if (!localRealisation) - throw Error("unable to register the derivation '%s' as it " - "depends on the non existent '%s'", - info.id.to_string(), outputId.to_string()); - if (localRealisation->second.outPath != depPath) - throw Error("unable to register the derivation '%s' as it " - "depends on a realisation of '%s' that doesn’t" - "match what we have locally", - info.id.to_string(), outputId.to_string()); - state->stmts->AddRealisationReference.use() - (info.id.strHash()) - (info.id.outputName) - (outputId.strHash()) - (outputId.outputName) - .exec(); - } }); } @@ -1034,7 +1001,7 @@ bool LocalStore::pathInfoIsUntrusted(const ValidPathInfo & info) bool LocalStore::realisationIsUntrusted(const Realisation & realisation) { - return requireSigs && !realisation.checkSignatures(getPublicKeys()); + return requireSigs && !realisation.checkSignatures(*this, realisation.id, getPublicKeys()); } void LocalStore::addToStore(const ValidPathInfo & info, Source & source, @@ -1590,7 +1557,7 @@ void LocalStore::signRealisation(Realisation & realisation) for (auto & secretKeyFile : secretKeyFiles.get()) { SecretKey secretKey(readFile(secretKeyFile)); LocalSigner signer(std::move(secretKey)); - realisation.sign(signer); + realisation.sign(*this, realisation.id, signer); } } @@ -1608,13 +1575,13 @@ void LocalStore::signPathInfo(ValidPathInfo & info) } -std::optional> LocalStore::queryRealisationCore_( +std::optional> LocalStore::queryRealisationCore_( LocalStore::State & state, const DrvOutput & id) { auto useQueryRealisedOutput( state.stmts->QueryRealisedOutput.use() - (id.strHash()) + (id.drvPath.to_string()) (id.outputName)); if (!useQueryRealisedOutput.next()) return std::nullopt; @@ -1625,15 +1592,14 @@ std::optional> LocalStore::queryRealisationCore_ return {{ realisationDbId, - Realisation{ - .id = id, + UnkeyedRealisation{ .outPath = outputPath, .signatures = signatures, } }}; } -std::optional LocalStore::queryRealisation_( +std::optional LocalStore::queryRealisation_( LocalStore::State & state, const DrvOutput & id) { @@ -1642,38 +1608,21 @@ std::optional LocalStore::queryRealisation_( return std::nullopt; auto [realisationDbId, res] = *maybeCore; - std::map dependentRealisations; - auto useRealisationRefs( - state.stmts->QueryRealisationReferences.use() - (realisationDbId)); - while (useRealisationRefs.next()) { - auto depId = DrvOutput { - Hash::parseAnyPrefixed(useRealisationRefs.getStr(0)), - useRealisationRefs.getStr(1), - }; - auto dependentRealisation = queryRealisationCore_(state, depId); - assert(dependentRealisation); // Enforced by the db schema - auto outputPath = dependentRealisation->second.outPath; - dependentRealisations.insert({depId, outputPath}); - } - - res.dependentRealisations = dependentRealisations; - return { res }; } void LocalStore::queryRealisationUncached(const DrvOutput & id, - Callback> callback) noexcept + Callback> callback) noexcept { try { auto maybeRealisation - = retrySQLite>([&]() { + = retrySQLite>([&]() { auto state(_state.lock()); return queryRealisation_(*state, id); }); if (maybeRealisation) callback( - std::make_shared(maybeRealisation.value())); + std::make_shared(maybeRealisation.value())); else callback(nullptr); diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index 83154d65193..222b6014e59 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -334,10 +334,10 @@ public: const std::string & outputName, const StorePath & output); - std::optional queryRealisation_(State & state, const DrvOutput & id); - std::optional> queryRealisationCore_(State & state, const DrvOutput & id); + std::optional queryRealisation_(State & state, const DrvOutput & id); + std::optional> queryRealisationCore_(State & state, const DrvOutput & id); void queryRealisationUncached(const DrvOutput&, - Callback> callback) noexcept override; + Callback> callback) noexcept override; std::optional getVersion() override; diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index bcc02206bc9..f7d39e1ec70 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -228,16 +228,15 @@ void Store::queryMissing(const std::vector & targets, // If there are unknown output paths, attempt to find if the // paths are known to substituters through a realisation. - auto outputHashes = staticOutputHashes(*this, *drv); knownOutputPaths = true; - for (auto [outputName, hash] : outputHashes) { + for (auto & [outputName, _] : drv->outputs) { if (!bfd.outputs.contains(outputName)) continue; bool found = false; for (auto &sub : getDefaultSubstituters()) { - auto realisation = sub->queryRealisation({hash, outputName}); + auto realisation = sub->queryRealisation({drvPath, outputName}); if (!realisation) continue; found = true; @@ -315,72 +314,6 @@ StorePaths Store::topoSortPaths(const StorePathSet & paths) }}); } -std::map drvOutputReferences( - const std::set & inputRealisations, - const StorePathSet & pathReferences) -{ - std::map res; - - for (const auto & input : inputRealisations) { - if (pathReferences.count(input.outPath)) { - res.insert({input.id, input.outPath}); - } - } - - return res; -} - -std::map drvOutputReferences( - Store & store, - const Derivation & drv, - const StorePath & outputPath, - Store * evalStore_) -{ - auto & evalStore = evalStore_ ? *evalStore_ : store; - - std::set inputRealisations; - - std::function::ChildNode &)> accumRealisations; - - accumRealisations = [&](const StorePath & inputDrv, const DerivedPathMap::ChildNode & inputNode) { - if (!inputNode.value.empty()) { - auto outputHashes = - staticOutputHashes(evalStore, evalStore.readDerivation(inputDrv)); - for (const auto & outputName : inputNode.value) { - auto outputHash = get(outputHashes, outputName); - if (!outputHash) - throw Error( - "output '%s' of derivation '%s' isn't realised", outputName, - store.printStorePath(inputDrv)); - auto thisRealisation = store.queryRealisation( - DrvOutput{*outputHash, outputName}); - if (!thisRealisation) - throw Error( - "output '%s' of derivation '%s' isn’t built", outputName, - store.printStorePath(inputDrv)); - inputRealisations.insert(*thisRealisation); - } - } - if (!inputNode.value.empty()) { - auto d = makeConstantStorePathRef(inputDrv); - for (const auto & [outputName, childNode] : inputNode.childMap) { - SingleDerivedPath next = SingleDerivedPath::Built { d, outputName }; - accumRealisations( - // TODO deep resolutions for dynamic derivations, issue #8947, would go here. - resolveDerivedPath(store, next, evalStore_), - childNode); - } - } - }; - - for (const auto & [inputDrv, inputNode] : drv.inputDrvs.map) - accumRealisations(inputDrv, inputNode); - - auto info = store.queryPathInfo(outputPath); - - return drvOutputReferences(Realisation::closure(store, inputRealisations), info->references); -} - OutputPathMap resolveDerivedPath(Store & store, const DerivedPath::Built & bfd, Store * evalStore_) { auto drvPath = resolveDerivedPath(store, *bfd.drvPath, evalStore_); @@ -410,7 +343,7 @@ OutputPathMap resolveDerivedPath(Store & store, const DerivedPath::Built & bfd, OutputPathMap outputs; for (auto & [outputName, outputPathOpt] : outputsOpt) { if (!outputPathOpt) - throw MissingRealisation(bfd.drvPath->to_string(store), outputName); + throw MissingRealisation(store, *bfd.drvPath, drvPath, outputName); auto & outputPath = *outputPathOpt; outputs.insert_or_assign(outputName, outputPath); } @@ -434,7 +367,7 @@ StorePath resolveDerivedPath(Store & store, const SingleDerivedPath & req, Store store.printStorePath(drvPath), bfd.output); auto & optPath = outputPaths.at(bfd.output); if (!optPath) - throw MissingRealisation(bfd.drvPath->to_string(store), bfd.output); + throw MissingRealisation(store, *bfd.drvPath, drvPath, bfd.output); return *optPath; }, }, req.raw()); diff --git a/src/libstore/nar-info-disk-cache.cc b/src/libstore/nar-info-disk-cache.cc index 80e8d34149d..b334996f00a 100644 --- a/src/libstore/nar-info-disk-cache.cc +++ b/src/libstore/nar-info-disk-cache.cc @@ -44,10 +44,16 @@ create table if not exists NARs ( create table if not exists Realisations ( cache integer not null, - outputId text not null, - content blob, -- Json serialisation of the realisation, or null if the realisation is absent + + drvPath text not null, + outputName text not null, + + -- The following are null if the realisation is absent + outputPath text, + sigs text, + timestamp integer not null, - primary key (cache, outputId), + primary key (cache, drvPath, outputName), foreign key (cache) references BinaryCaches(id) on delete cascade ); @@ -117,22 +123,22 @@ class NarInfoDiskCacheImpl : public NarInfoDiskCache state->insertRealisation.create(state->db, R"( - insert or replace into Realisations(cache, outputId, content, timestamp) - values (?, ?, ?, ?) + insert or replace into Realisations(cache, drvPath, outputName, outputPath, sigs, timestamp) + values (?, ?, ?, ?, ?, ?) )"); state->insertMissingRealisation.create(state->db, R"( - insert or replace into Realisations(cache, outputId, timestamp) - values (?, ?, ?) + insert or replace into Realisations(cache, drvPath, outputName, timestamp) + values (?, ?, ?, ?) )"); state->queryRealisation.create(state->db, R"( - select content from Realisations - where cache = ? and outputId = ? and - ((content is null and timestamp > ?) or - (content is not null and timestamp > ?)) + select outputPath, sigs from Realisations + where cache = ? and drvPath = ? and outputName = ? and + ((outputPath is null and timestamp > ?) or + (outputPath is not null and timestamp > ?)) )"); /* Periodically purge expired entries from the database. */ @@ -295,22 +301,31 @@ class NarInfoDiskCacheImpl : public NarInfoDiskCache auto queryRealisation(state->queryRealisation.use() (cache.id) - (id.to_string()) + (id.drvPath.to_string()) + (id.outputName) (now - settings.ttlNegativeNarInfoCache) (now - settings.ttlPositiveNarInfoCache)); if (!queryRealisation.next()) - return {oUnknown, 0}; + return {oUnknown, nullptr}; if (queryRealisation.isNull(0)) - return {oInvalid, 0}; - - auto realisation = - std::make_shared(Realisation::fromJSON( - nlohmann::json::parse(queryRealisation.getStr(0)), - "Local disk cache")); - - return {oValid, realisation}; + return {oInvalid, nullptr}; + + try { + return { + oValid, + std::make_shared( + UnkeyedRealisation{ + .outPath = StorePath{queryRealisation.getStr(0)}, + .signatures = nlohmann::json::parse(queryRealisation.getStr(1)), + }, + id), + }; + } catch (Error & e) { + e.addTrace({}, "reading build trace key-value from the local disk cache"); + throw; + } }); } @@ -365,8 +380,10 @@ class NarInfoDiskCacheImpl : public NarInfoDiskCache state->insertRealisation.use() (cache.id) - (realisation.id.to_string()) - (realisation.toJSON().dump()) + (realisation.id.drvPath.to_string()) + (realisation.id.outputName) + (realisation.outPath.to_string()) + (nlohmann::json(realisation.signatures)) (time(0)).exec(); }); diff --git a/src/libstore/realisation.cc b/src/libstore/realisation.cc index 86bfdd1a8bf..026ef262337 100644 --- a/src/libstore/realisation.cc +++ b/src/libstore/realisation.cc @@ -1,130 +1,113 @@ #include "realisation.hh" #include "store-api.hh" -#include "closure.hh" #include "signature/local-keys.hh" -#include +#include "json-utils.hh" namespace nix { MakeError(InvalidDerivationOutputId, Error); -DrvOutput DrvOutput::parse(const std::string &strRep) { - size_t n = strRep.find("!"); - if (n == strRep.npos) - throw InvalidDerivationOutputId("Invalid derivation output id %s", strRep); - +DrvOutput DrvOutput::parse(const StoreDirConfig & store, std::string_view s) +{ + size_t n = s.rfind('^'); + if (n == s.npos) + throw InvalidDerivationOutputId("Invalid derivation output id '%s': missing '^'", s); return DrvOutput{ - .drvHash = Hash::parseAnyPrefixed(strRep.substr(0, n)), - .outputName = strRep.substr(n+1), + .drvPath = store.parseStorePath(s.substr(0, n)), + .outputName = OutputName{s.substr(n + 1)}, }; } -std::string DrvOutput::to_string() const { - return strHash() + "!" + outputName; +std::string DrvOutput::render(const StoreDirConfig & store) const +{ + return std::string(store.printStorePath(drvPath)) + "^" + outputName; } -std::set Realisation::closure(Store & store, const std::set & startOutputs) +std::string DrvOutput::to_string() const { - std::set res; - Realisation::closure(store, startOutputs, res); - return res; + return std::string(drvPath.to_string()) + "^" + outputName; } -void Realisation::closure(Store & store, const std::set & startOutputs, std::set & res) +nlohmann::json DrvOutput::toJSON(const StoreDirConfig & store) const { - auto getDeps = [&](const Realisation& current) -> std::set { - std::set res; - for (auto& [currentDep, _] : current.dependentRealisations) { - if (auto currentRealisation = store.queryRealisation(currentDep)) - res.insert(*currentRealisation); - else - throw Error( - "Unrealised derivation '%s'", currentDep.to_string()); - } - return res; + return nlohmann::json{ + {"drvPath", store.printStorePath(drvPath)}, + {"outputName", outputName}, }; +} - computeClosure( - startOutputs, res, - [&](const Realisation& current, - std::function>&)> - processEdges) { - std::promise> promise; - try { - auto res = getDeps(current); - promise.set_value(res); - } catch (...) { - promise.set_exception(std::current_exception()); - } - return processEdges(promise); - }); -} - -nlohmann::json Realisation::toJSON() const { - auto jsonDependentRealisations = nlohmann::json::object(); - for (auto & [depId, depOutPath] : dependentRealisations) - jsonDependentRealisations.emplace(depId.to_string(), depOutPath.to_string()); +DrvOutput DrvOutput::fromJSON(const StoreDirConfig & store, const nlohmann::json & json) +{ + auto obj = getObject(json); + + return { + .drvPath = store.parseStorePath(getString(valueAt(obj, "drvPath"))), + .outputName = getString(valueAt(obj, "outputName")), + }; +} + +nlohmann::json UnkeyedRealisation::toJSON(const StoreDirConfig & store) const +{ return nlohmann::json{ - {"id", id.to_string()}, - {"outPath", outPath.to_string()}, + {"outPath", store.printStorePath(outPath)}, {"signatures", signatures}, - {"dependentRealisations", jsonDependentRealisations}, }; } -Realisation Realisation::fromJSON( - const nlohmann::json& json, - const std::string& whence) { - auto getOptionalField = [&](std::string fieldName) -> std::optional { - auto fieldIterator = json.find(fieldName); - if (fieldIterator == json.end()) - return std::nullopt; - return {*fieldIterator}; - }; - auto getField = [&](std::string fieldName) -> std::string { - if (auto field = getOptionalField(fieldName)) - return *field; - else - throw Error( - "Drv output info file '%1%' is corrupt, missing field %2%", - whence, fieldName); - }; +UnkeyedRealisation UnkeyedRealisation::fromJSON(const StoreDirConfig & store, const nlohmann::json & json) +{ + auto obj = getObject(json); StringSet signatures; - if (auto signaturesIterator = json.find("signatures"); signaturesIterator != json.end()) - signatures.insert(signaturesIterator->begin(), signaturesIterator->end()); + if (auto * signaturesJson = get(obj, "signatures")) + signatures = getStringSet(*signaturesJson); - std::map dependentRealisations; - if (auto jsonDependencies = json.find("dependentRealisations"); jsonDependencies != json.end()) - for (auto & [jsonDepId, jsonDepOutPath] : jsonDependencies->get>()) - dependentRealisations.insert({DrvOutput::parse(jsonDepId), StorePath(jsonDepOutPath)}); - - return Realisation{ - .id = DrvOutput::parse(getField("id")), - .outPath = StorePath(getField("outPath")), + return { + .outPath = store.parseStorePath(getString(valueAt(obj, "outPath"))), .signatures = signatures, - .dependentRealisations = dependentRealisations, }; } -std::string Realisation::fingerprint() const +nlohmann::json Realisation::toJSON(const StoreDirConfig & store) const +{ + return nlohmann::json{ + {"key", id.toJSON(store)}, + {"value", static_cast(*this).toJSON(store)}, + }; +} + +Realisation Realisation::fromJSON(const StoreDirConfig & store, const nlohmann::json & json) +{ + auto obj = getObject(json); + + return { + UnkeyedRealisation::fromJSON(store, valueAt(obj, "key")), + DrvOutput::fromJSON(store, valueAt(obj, "value")), + }; +} + +std::string UnkeyedRealisation::fingerprint(const StoreDirConfig & store, const DrvOutput & key) const { - auto serialized = toJSON(); - serialized.erase("signatures"); - return serialized.dump(); + auto serialised = Realisation{*this, key}.toJSON(store); + auto value = serialised.find("value"); + assert(value != serialised.end()); + value->erase("signatures"); + return serialised.dump(); } -void Realisation::sign(const Signer &signer) +void UnkeyedRealisation::sign(const StoreDirConfig & store, const DrvOutput & key, const Signer & signer) { - signatures.insert(signer.signDetached(fingerprint())); + signatures.insert(signer.signDetached(fingerprint(store, key))); } -bool Realisation::checkSignature(const PublicKeys & publicKeys, const std::string & sig) const +bool UnkeyedRealisation::checkSignature( + const StoreDirConfig & store, const DrvOutput & key, const PublicKeys & publicKeys, const std::string & sig) const { - return verifyDetached(fingerprint(), sig, publicKeys); + return verifyDetached(fingerprint(store, key), sig, publicKeys); } -size_t Realisation::checkSignatures(const PublicKeys & publicKeys) const +size_t UnkeyedRealisation::checkSignatures( + const StoreDirConfig & store, const DrvOutput & key, const PublicKeys & publicKeys) const { // FIXME: Maybe we should return `maxSigs` if the realisation corresponds to // an input-addressed one − because in that case the drv is enough to check @@ -132,16 +115,15 @@ size_t Realisation::checkSignatures(const PublicKeys & publicKeys) const size_t good = 0; for (auto & sig : signatures) - if (checkSignature(publicKeys, sig)) + if (checkSignature(store, key, publicKeys, sig)) good++; return good; } - -SingleDrvOutputs filterDrvOutputs(const OutputsSpec& wanted, SingleDrvOutputs&& outputs) +SingleDrvOutputs filterDrvOutputs(const OutputsSpec & wanted, SingleDrvOutputs && outputs) { SingleDrvOutputs ret = std::move(outputs); - for (auto it = ret.begin(); it != ret.end(); ) { + for (auto it = ret.begin(); it != ret.end();) { if (!wanted.contains(it->first)) it = ret.erase(it); else @@ -150,53 +132,25 @@ SingleDrvOutputs filterDrvOutputs(const OutputsSpec& wanted, SingleDrvOutputs&& return ret; } -StorePath RealisedPath::path() const { - return std::visit([](auto && arg) { return arg.getPath(); }, raw); -} - -bool Realisation::isCompatibleWith(const Realisation & other) const -{ - assert (id == other.id); - if (outPath == other.outPath) { - if (dependentRealisations.empty() != other.dependentRealisations.empty()) { - warn( - "Encountered a realisation for '%s' with an empty set of " - "dependencies. This is likely an artifact from an older Nix. " - "I’ll try to fix the realisation if I can", - id.to_string()); - return true; - } else if (dependentRealisations == other.dependentRealisations) { - return true; - } - } - return false; -} - -void RealisedPath::closure( - Store& store, - const RealisedPath::Set& startPaths, - RealisedPath::Set& ret) +const StorePath & RealisedPath::path() const { - // FIXME: This only builds the store-path closure, not the real realisation - // closure - StorePathSet initialStorePaths, pathsClosure; - for (auto& path : startPaths) - initialStorePaths.insert(path.path()); - store.computeFSClosure(initialStorePaths, pathsClosure); - ret.insert(startPaths.begin(), startPaths.end()); - ret.insert(pathsClosure.begin(), pathsClosure.end()); + return std::visit([](auto && arg) -> auto & { return arg.getPath(); }, raw); } -void RealisedPath::closure(Store& store, RealisedPath::Set & ret) const +MissingRealisation::MissingRealisation( + const StoreDirConfig & store, const StorePath & drvPath, const OutputName & outputName) + : Error("cannot operate on output '%s' of the unbuilt derivation '%s'", outputName, store.printStorePath(drvPath)) { - RealisedPath::closure(store, {*this}, ret); } -RealisedPath::Set RealisedPath::closure(Store& store) const +MissingRealisation::MissingRealisation( + const StoreDirConfig & store, + const SingleDerivedPath & drvPath, + const StorePath & drvPathResolved, + const OutputName & outputName) + : MissingRealisation{store, drvPathResolved, outputName} { - RealisedPath::Set ret; - closure(store, ret); - return ret; + addTrace({}, "looking up realisation for derivation '%s'", drvPath.to_string(store)); } } // namespace nix diff --git a/src/libstore/realisation.hh b/src/libstore/realisation.hh index ddb4af770a2..84fe96515a4 100644 --- a/src/libstore/realisation.hh +++ b/src/libstore/realisation.hh @@ -18,64 +18,97 @@ struct OutputsSpec; /** * 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. + * This is similar to a `DerivedPath::Built`, except it is only a single + * step: `drvPath` is a `StorePath` rather than a `DerivedPath`. */ -struct DrvOutput { +struct DrvOutput +{ /** - * 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. + * The store path to the derivation */ - Hash drvHash; + StorePath drvPath; /** * The name of the output. */ OutputName outputName; + /** + * Skips the store dir on the `drvPath` + */ std::string to_string() const; - std::string strHash() const - { return drvHash.to_string(HashFormat::Base16, true); } + /** + * Skips the store dir on the `drvPath` + */ + static DrvOutput from_string(std::string_view); - static DrvOutput parse(const std::string &); + /** + * Includes the store dir on `drvPath` + */ + std::string render(const StoreDirConfig & store) const; - GENERATE_CMP(DrvOutput, me->drvHash, me->outputName); + /** + * Includes the store dir on `drvPath` + */ + static DrvOutput parse(const StoreDirConfig & store, std::string_view); + + nlohmann::json toJSON(const StoreDirConfig & store) const; + static DrvOutput fromJSON(const StoreDirConfig & store, const nlohmann::json & json); + + bool operator==(const DrvOutput &) const = default; + auto operator<=>(const DrvOutput &) const = default; }; -struct Realisation { - DrvOutput id; +struct UnkeyedRealisation +{ StorePath outPath; StringSet signatures; - /** - * The realisations that are required for the current one to be valid. - * - * When importing this realisation, the store will first check that all its - * dependencies exist, and map to the correct output path - */ - std::map dependentRealisations; + nlohmann::json toJSON(const StoreDirConfig & store) const; + static UnkeyedRealisation fromJSON(const StoreDirConfig & store, const nlohmann::json & json); - nlohmann::json toJSON() const; - static Realisation fromJSON(const nlohmann::json& json, const std::string& whence); + std::string fingerprint(const StoreDirConfig & store, const DrvOutput & key) const; - std::string fingerprint() const; - void sign(const Signer &); - bool checkSignature(const PublicKeys & publicKeys, const std::string & sig) const; - size_t checkSignatures(const PublicKeys & publicKeys) const; + void sign(const StoreDirConfig & store, const DrvOutput & key, const Signer &); - static std::set closure(Store &, const std::set &); - static void closure(Store &, const std::set &, std::set & res); + bool checkSignature( + const StoreDirConfig & store, + const DrvOutput & key, + const PublicKeys & publicKeys, + const std::string & sig) const; - bool isCompatibleWith(const Realisation & other) const; + size_t checkSignatures(const StoreDirConfig & store, const DrvOutput & key, const PublicKeys & publicKeys) const; - StorePath getPath() const { return outPath; } + /** + * Just check the `outPath`. Signatures don't matter for this. + * Callers must ensure that the corresponding key is the same for + * most use-cases. + */ + bool isCompatibleWith(const UnkeyedRealisation & other) const + { + return outPath == other.outPath; + } + + const StorePath & getPath() const + { + return outPath; + } + + // TODO sketchy that it avoids signatures + GENERATE_CMP(UnkeyedRealisation, me->outPath); +}; + +struct Realisation : UnkeyedRealisation +{ + DrvOutput id; + + nlohmann::json toJSON(const StoreDirConfig & store) const; + static Realisation fromJSON(const StoreDirConfig & store, const nlohmann::json & json); - GENERATE_CMP(Realisation, me->id, me->outPath); + bool operator==(const Realisation &) const = default; + auto operator<=>(const Realisation &) const = default; }; /** @@ -84,7 +117,7 @@ struct Realisation { * 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 SingleDrvOutputs; +typedef std::map SingleDrvOutputs; /** * Collection type for multiple derivations' outputs' `Realisation`s. @@ -93,30 +126,34 @@ typedef std::map SingleDrvOutputs; * the same, so we need to identify firstly which derivation, and * secondly which output of that derivation. */ -typedef std::map DrvOutputs; +typedef std::map DrvOutputs; /** * Filter a SingleDrvOutputs to include only specific output names * * Moves the `outputs` input. */ -SingleDrvOutputs filterDrvOutputs(const OutputsSpec&, SingleDrvOutputs&&); +SingleDrvOutputs filterDrvOutputs(const OutputsSpec &, SingleDrvOutputs && outputs); - -struct OpaquePath { +struct OpaquePath +{ StorePath path; - StorePath getPath() const { return path; } + const StorePath & getPath() const + { + return path; + } - GENERATE_CMP(OpaquePath, me->path); + bool operator==(const OpaquePath &) const = default; + auto operator<=>(const OpaquePath &) const = default; }; - /** * A store path with all the history of how it went into the store */ -struct RealisedPath { - /* +struct RealisedPath +{ + /** * A path is either the result of the realisation of a derivation or * an opaque blob that has been directly added to the store */ @@ -125,33 +162,38 @@ struct RealisedPath { using Set = std::set; - RealisedPath(StorePath path) : raw(OpaquePath{path}) {} - RealisedPath(Realisation r) : raw(r) {} + RealisedPath(StorePath path) + : raw(OpaquePath{path}) + { + } + + RealisedPath(Realisation r) + : raw(r) + { + } /** * Get the raw store path associated to this */ - StorePath path() const; - - void closure(Store& store, Set& ret) const; - static void closure(Store& store, const Set& startPaths, Set& ret); - Set closure(Store& store) const; + const StorePath & path() const; - GENERATE_CMP(RealisedPath, me->raw); + bool operator==(const RealisedPath &) const = default; + auto operator<=>(const RealisedPath &) const = default; }; class MissingRealisation : public Error { public: - MissingRealisation(DrvOutput & outputId) - : MissingRealisation(outputId.outputName, outputId.strHash()) - {} - MissingRealisation(std::string_view drv, OutputName outputName) - : Error( "cannot operate on output '%s' of the " - "unbuilt derivation '%s'", - outputName, - drv) - {} + MissingRealisation(const StoreDirConfig & store, DrvOutput & outputId) + : MissingRealisation(store, outputId.drvPath, outputId.outputName) + { + } + MissingRealisation(const StoreDirConfig & store, const StorePath & drvPath, const OutputName & outputName); + MissingRealisation( + const StoreDirConfig & store, + const SingleDerivedPath & drvPath, + const StorePath & drvPathResolved, + const OutputName & outputName); }; } diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 533ea557d25..eb90f311877 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -602,12 +602,12 @@ void RemoteStore::registerDrvOutput(const Realisation & info) } void RemoteStore::queryRealisationUncached(const DrvOutput & id, - Callback> callback) noexcept + Callback> callback) noexcept { try { auto conn(getConnection()); - if (GET_PROTOCOL_MINOR(conn->protoVersion) < 27) { + if (GET_PROTOCOL_MINOR(conn->protoVersion) < 39) { warn("the daemon is too old to support content-addressing derivations, please upgrade it to 2.4"); return callback(nullptr); } @@ -616,23 +616,13 @@ void RemoteStore::queryRealisationUncached(const DrvOutput & id, conn->to << id.to_string(); conn.processStderr(); - auto real = [&]() -> std::shared_ptr { - if (GET_PROTOCOL_MINOR(conn->protoVersion) < 31) { - auto outPaths = WorkerProto::Serialise>::read( - *this, *conn); - if (outPaths.empty()) - return nullptr; - return std::make_shared(Realisation { .id = id, .outPath = *outPaths.begin() }); - } else { - auto realisations = WorkerProto::Serialise>::read( - *this, *conn); - if (realisations.empty()) - return nullptr; - return std::make_shared(*realisations.begin()); - } - }(); - - callback(std::shared_ptr(real)); + callback([&]() -> std::shared_ptr { + auto realisation = WorkerProto::Serialise>::read( + *this, *conn); + if (!realisation) + return nullptr; + return std::make_shared(*realisation); + }()); } catch (...) { return callback.rethrow(); } } @@ -724,27 +714,19 @@ std::vector RemoteStore::buildPathsWithResults( OutputPathMap outputs; auto drvPath = resolveDerivedPath(*evalStore, *bfd.drvPath); - auto drv = evalStore->readDerivation(drvPath); - const auto outputHashes = staticOutputHashes(*evalStore, drv); // FIXME: expensive auto built = resolveDerivedPath(*this, bfd, &*evalStore); for (auto & [output, outputPath] : built) { - auto outputHash = get(outputHashes, output); - if (!outputHash) - throw Error( - "the derivation '%s' doesn't have an output named '%s'", - printStorePath(drvPath), output); - auto outputId = DrvOutput{ *outputHash, output }; + auto outputId = DrvOutput{ drvPath, output }; if (experimentalFeatureSettings.isEnabled(Xp::CaDerivations)) { auto realisation = queryRealisation(outputId); if (!realisation) - throw MissingRealisation(outputId); + throw MissingRealisation(*this, outputId); res.builtOutputs.emplace(output, *realisation); } else { res.builtOutputs.emplace( output, - Realisation { - .id = outputId, + UnkeyedRealisation { .outPath = outputPath, }); } diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh index ea6cd471eb5..e6b12ec27c8 100644 --- a/src/libstore/remote-store.hh +++ b/src/libstore/remote-store.hh @@ -110,7 +110,7 @@ public: void registerDrvOutput(const Realisation & info) override; void queryRealisationUncached(const DrvOutput &, - Callback> callback) noexcept override; + Callback> callback) noexcept override; void buildPaths(const std::vector & paths, BuildMode buildMode, std::shared_ptr evalStore) override; diff --git a/src/libstore/serve-protocol.cc b/src/libstore/serve-protocol.cc index 08bfad9e405..63a76b0810c 100644 --- a/src/libstore/serve-protocol.cc +++ b/src/libstore/serve-protocol.cc @@ -25,12 +25,11 @@ BuildResult ServeProto::Serialise::read(const StoreDirConfig & stor >> status.isNonDeterministic >> status.startTime >> status.stopTime; - if (GET_PROTOCOL_MINOR(conn.version) >= 6) { - auto builtOutputs = ServeProto::Serialise::read(store, conn); - for (auto && [output, realisation] : builtOutputs) - status.builtOutputs.insert_or_assign( - std::move(output.outputName), - std::move(realisation)); + if (GET_PROTOCOL_MINOR(conn.version) >= 8) { + status.builtOutputs = ServeProto::Serialise>::read(store, conn); + } else if (GET_PROTOCOL_MINOR(conn.version) >= 6) { + // We no longer support these types of realisations + (void) ServeProto::Serialise::read(store, conn); } return status; } @@ -47,11 +46,11 @@ void ServeProto::Serialise::write(const StoreDirConfig & store, Ser << status.isNonDeterministic << status.startTime << status.stopTime; - if (GET_PROTOCOL_MINOR(conn.version) >= 6) { - DrvOutputs builtOutputs; - for (auto & [output, realisation] : status.builtOutputs) - builtOutputs.insert_or_assign(realisation.id, realisation); - ServeProto::write(store, conn, builtOutputs); + if (GET_PROTOCOL_MINOR(conn.version) >= 8) { + ServeProto::write(store, conn, status.builtOutputs); + } else if (GET_PROTOCOL_MINOR(conn.version) >= 6) { + // We no longer support these types of realisations + ServeProto::write(store, conn, StringMap{}); } } @@ -134,4 +133,80 @@ void ServeProto::Serialise::write(const StoreDirConfig } } + +UnkeyedRealisation ServeProto::Serialise::read(const StoreDirConfig & store, ReadConn conn) +{ + if (GET_PROTOCOL_MINOR(conn.version) < 39) { + throw Error("daemon protocol %d.%d is too old (< 1.29) to understand build trace", + GET_PROTOCOL_MAJOR(conn.version), + GET_PROTOCOL_MINOR(conn.version)); + } + + auto outPath = ServeProto::Serialise::read(store, conn); + auto signatures = ServeProto::Serialise::read(store, conn); + + return UnkeyedRealisation { + .outPath = std::move(outPath), + .signatures = std::move(signatures), + }; +} + +void ServeProto::Serialise::write(const StoreDirConfig & store, WriteConn conn, const UnkeyedRealisation & info) +{ + if (GET_PROTOCOL_MINOR(conn.version) < 39) { + throw Error("daemon protocol %d.%d is too old (< 1.29) to understand build trace", + GET_PROTOCOL_MAJOR(conn.version), + GET_PROTOCOL_MINOR(conn.version)); + } + ServeProto::write(store, conn, info.outPath); + ServeProto::write(store, conn, info.signatures); +} + + +DrvOutput ServeProto::Serialise::read(const StoreDirConfig & store, ReadConn conn) +{ + if (GET_PROTOCOL_MINOR(conn.version) < 39) { + throw Error("daemon protocol %d.%d is too old (< 1.29) to understand build trace", + GET_PROTOCOL_MAJOR(conn.version), + GET_PROTOCOL_MINOR(conn.version)); + } + + auto drvPath = ServeProto::Serialise::read(store, conn); + auto outputName = ServeProto::Serialise::read(store, conn); + + return DrvOutput { + .drvPath = std::move(drvPath), + .outputName = std::move(outputName), + }; +} + +void ServeProto::Serialise::write(const StoreDirConfig & store, WriteConn conn, const DrvOutput & info) +{ + if (GET_PROTOCOL_MINOR(conn.version) < 39) { + throw Error("daemon protocol %d.%d is too old (< 1.29) to understand build trace", + GET_PROTOCOL_MAJOR(conn.version), + GET_PROTOCOL_MINOR(conn.version)); + } + ServeProto::write(store, conn, info.drvPath); + ServeProto::write(store, conn, info.outputName); +} + + +Realisation ServeProto::Serialise::read(const StoreDirConfig & store, ReadConn conn) +{ + auto id = ServeProto::Serialise::read(store, conn); + auto unkeyed = ServeProto::Serialise::read(store, conn); + + return Realisation { + std::move(unkeyed), + std::move(id), + }; +} + +void ServeProto::Serialise::write(const StoreDirConfig & store, WriteConn conn, const Realisation & info) +{ + ServeProto::write(store, conn, info.id); + ServeProto::write(store, conn, static_cast(info)); +} + } diff --git a/src/libstore/serve-protocol.hh b/src/libstore/serve-protocol.hh index 8c112bb74c7..ec125f57817 100644 --- a/src/libstore/serve-protocol.hh +++ b/src/libstore/serve-protocol.hh @@ -8,7 +8,7 @@ namespace nix { #define SERVE_MAGIC_1 0x390c9deb #define SERVE_MAGIC_2 0x5452eecb -#define SERVE_PROTOCOL_VERSION (2 << 8 | 7) +#define SERVE_PROTOCOL_VERSION (2 << 8 | 8) #define GET_PROTOCOL_MAJOR(x) ((x) & 0xff00) #define GET_PROTOCOL_MINOR(x) ((x) & 0x00ff) @@ -19,6 +19,9 @@ struct Source; // items being serialised struct BuildResult; struct UnkeyedValidPathInfo; +struct DrvOutput; +struct UnkeyedRealisation; +struct Realisation; /** @@ -174,6 +177,12 @@ inline std::ostream & operator << (std::ostream & s, ServeProto::Command op) template<> DECLARE_SERVE_SERIALISER(BuildResult); template<> +DECLARE_SERVE_SERIALISER(DrvOutput); +template<> +DECLARE_SERVE_SERIALISER(UnkeyedRealisation); +template<> +DECLARE_SERVE_SERIALISER(Realisation); +template<> DECLARE_SERVE_SERIALISER(UnkeyedValidPathInfo); template<> DECLARE_SERVE_SERIALISER(ServeProto::BuildOptions); diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 236622eae37..e360c87d5bf 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -476,9 +476,8 @@ std::map> Store::queryPartialDerivationOut return outputs; auto drv = evalStore.readInvalidDerivation(path); - auto drvHashes = staticOutputHashes(*this, drv); - for (auto & [outputName, hash] : drvHashes) { - auto realisation = queryRealisation(DrvOutput{hash, outputName}); + for (auto & [outputName, _] : drv.outputs) { + auto realisation = queryRealisation(DrvOutput{path, outputName}); if (realisation) { outputs.insert_or_assign(outputName, realisation->outPath); } else { @@ -497,7 +496,7 @@ OutputPathMap Store::queryDerivationOutputMap(const StorePath & path, Store * ev OutputPathMap result; for (auto & [outName, optOutPath] : resp) { if (!optOutPath) - throw MissingRealisation(printStorePath(path), outName); + throw MissingRealisation(*this, path, outName); result.insert_or_assign(outName, *optOutPath); } return result; @@ -713,7 +712,7 @@ void Store::queryPathInfo(const StorePath & storePath, } void Store::queryRealisation(const DrvOutput & id, - Callback> callback) noexcept + Callback> callback) noexcept { try { @@ -745,18 +744,18 @@ void Store::queryRealisation(const DrvOutput & id, queryRealisationUncached( id, { [this, id, callbackPtr]( - std::future> fut) { + std::future> fut) { try { auto info = fut.get(); if (diskCache) { if (info) - diskCache->upsertRealisation(getUri(), *info); + diskCache->upsertRealisation(getUri(), {*info, id}); else diskCache->upsertAbsentRealisation(getUri(), id); } - (*callbackPtr)(std::shared_ptr(info)); + (*callbackPtr)(std::shared_ptr(info)); } catch (...) { callbackPtr->rethrow(); @@ -764,9 +763,9 @@ void Store::queryRealisation(const DrvOutput & id, } }); } -std::shared_ptr Store::queryRealisation(const DrvOutput & id) +std::shared_ptr Store::queryRealisation(const DrvOutput & id) { - using RealPtr = std::shared_ptr; + using RealPtr = std::shared_ptr; std::promise promise; queryRealisation(id, @@ -1016,36 +1015,21 @@ std::map copyPaths( SubstituteFlag substitute) { StorePathSet storePaths; - std::set toplevelRealisations; + std::vector realisations; for (auto & path : paths) { storePaths.insert(path.path()); - if (auto realisation = std::get_if(&path.raw)) { + if (auto * realisation = std::get_if(&path.raw)) { experimentalFeatureSettings.require(Xp::CaDerivations); - toplevelRealisations.insert(*realisation); + realisations.push_back(realisation); } } + auto pathsMap = copyPaths(srcStore, dstStore, storePaths, repair, checkSigs, substitute); try { // Copy the realisation closure - processGraph( - Realisation::closure(srcStore, toplevelRealisations), - [&](const Realisation & current) -> std::set { - std::set children; - for (const auto & [drvOutput, _] : current.dependentRealisations) { - auto currentChild = srcStore.queryRealisation(drvOutput); - if (!currentChild) - throw Error( - "incomplete realisation closure: '%s' is a " - "dependency of '%s' but isn't registered", - drvOutput.to_string(), current.id.to_string()); - children.insert(*currentChild); - } - return children; - }, - [&](const Realisation& current) -> void { - dstStore.registerDrvOutput(current, checkSigs); - }); + for (const auto * realisation : realisations) + dstStore.registerDrvOutput(*realisation, checkSigs); } catch (MissingExperimentalFeature & e) { // Don't fail if the remote doesn't support CA derivations is it might // not be within our control to change that, and we might still want @@ -1151,8 +1135,19 @@ void copyClosure( { if (&srcStore == &dstStore) return; - RealisedPath::Set closure; - RealisedPath::closure(srcStore, paths, closure); + StorePathSet closure0; + for (auto & path : paths) { + if (auto * opaquePath = std::get_if(&path.raw)) { + closure0.insert(opaquePath->path); + } + } + + StorePathSet closure1; + srcStore.computeFSClosure(closure0, closure1); + + RealisedPath::Set closure = paths; + for (auto && path : closure1) + closure.insert({std::move(path)}); copyPaths(srcStore, dstStore, closure, repair, checkSigs, substitute); } diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 2eba88ea046..0d1d30827b9 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -64,6 +64,7 @@ MakeError(SubstituterDisabled, Error); MakeError(InvalidStoreReference, Error); +struct UnkeyedRealisation; struct Realisation; struct RealisedPath; struct DrvOutput; @@ -296,13 +297,13 @@ public: /** * Query the information about a realisation. */ - std::shared_ptr queryRealisation(const DrvOutput &); + std::shared_ptr queryRealisation(const DrvOutput &); /** * Asynchronous version of queryRealisation(). */ void queryRealisation(const DrvOutput &, - Callback> callback) noexcept; + Callback> callback) noexcept; /** @@ -331,7 +332,7 @@ protected: virtual void queryPathInfoUncached(const StorePath & path, Callback> callback) noexcept = 0; virtual void queryRealisationUncached(const DrvOutput &, - Callback> callback) noexcept = 0; + Callback> callback) noexcept = 0; public: @@ -939,10 +940,4 @@ std::optional decodeValidPathInfo( const ContentAddress * getDerivationCA(const BasicDerivation & drv); -std::map drvOutputReferences( - Store & store, - const Derivation & drv, - const StorePath & outputPath, - Store * evalStore = nullptr); - } diff --git a/src/libstore/unix/build/local-derivation-goal.cc b/src/libstore/unix/build/local-derivation-goal.cc index 21eb1506d7f..7911b2501f6 100644 --- a/src/libstore/unix/build/local-derivation-goal.cc +++ b/src/libstore/unix/build/local-derivation-goal.cc @@ -1429,7 +1429,7 @@ struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual In { throw Error("registerDrvOutput"); } void queryRealisationUncached(const DrvOutput & id, - Callback> callback) noexcept override + Callback> callback) noexcept override // XXX: This should probably be allowed if the realisation corresponds to // an allowed derivation { @@ -1465,9 +1465,19 @@ struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual In auto results = next->buildPathsWithResults(paths, buildMode); for (auto & result : results) { - for (auto & [outputName, output] : result.builtOutputs) { - newPaths.insert(output.outPath); - newRealisations.insert(output); + if (auto * pathBuilt = std::get_if(&result.path)) { + // TODO ugly extra IO + auto drvPath = resolveDerivedPath(*next, *pathBuilt->drvPath); + for (auto & [outputName, output] : result.builtOutputs) { + newPaths.insert(output.outPath); + newRealisations.insert({ + output, + { + .drvPath = drvPath, + .outputName = outputName, + } + }); + } } } @@ -1475,7 +1485,7 @@ struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual In next->computeFSClosure(newPaths, closure); for (auto & path : closure) goal.addDependency(path); - for (auto & real : Realisation::closure(*next, newRealisations)) + for (auto & real : newRealisations) goal.addedDrvOutputs.insert(real.id); return results; @@ -2833,11 +2843,13 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs() auto oldinfo = get(initialOutputs, outputName); assert(oldinfo); auto thisRealisation = Realisation { - .id = DrvOutput { - oldinfo->outputHash, - outputName + { + .outPath = newInfo.path, + }, + DrvOutput { + .drvPath = drvPath, + .outputName = outputName, }, - .outPath = newInfo.path }; if (experimentalFeatureSettings.isEnabled(Xp::CaDerivations) && !drv->type().isImpure()) diff --git a/src/libstore/worker-protocol.cc b/src/libstore/worker-protocol.cc index f06fb2893c7..ab0ea93d655 100644 --- a/src/libstore/worker-protocol.cc +++ b/src/libstore/worker-protocol.cc @@ -195,10 +195,11 @@ void WorkerProto::Serialise::write(const StoreDirConfig & store, Wo WorkerProto::write(store, conn, res.cpuSystem); } if (GET_PROTOCOL_MINOR(conn.version) >= 28) { - DrvOutputs builtOutputs; - for (auto & [output, realisation] : res.builtOutputs) - builtOutputs.insert_or_assign(realisation.id, realisation); - WorkerProto::write(store, conn, builtOutputs); + // Don't support those types of realisations anymore. + WorkerProto::write(store, conn, StringMap{}); + } + if (GET_PROTOCOL_MINOR(conn.version) >= 39) { + WorkerProto::write(store, conn, res.builtOutputs); } } @@ -260,7 +261,7 @@ WorkerProto::ClientHandshakeInfo WorkerProto::Serialise= 35) { - res.remoteTrustsUs = WorkerProto::Serialise>::read(store, conn); + res.remoteTrustsUs = WorkerProto::Serialise>::read(store, conn); } else { // We don't know the answer; protocol to old. res.remoteTrustsUs = std::nullopt; @@ -281,4 +282,110 @@ void WorkerProto::Serialise::write(const Store } } + +UnkeyedRealisation WorkerProto::Serialise::read(const StoreDirConfig & store, ReadConn conn) +{ + if (GET_PROTOCOL_MINOR(conn.version) < 39) { + throw Error("daemon protocol %d.%d is too old (< 1.29) to understand build trace", + GET_PROTOCOL_MAJOR(conn.version), + GET_PROTOCOL_MINOR(conn.version)); + } + + auto outPath = WorkerProto::Serialise::read(store, conn); + auto signatures = WorkerProto::Serialise::read(store, conn); + + return UnkeyedRealisation { + .outPath = std::move(outPath), + .signatures = std::move(signatures), + }; +} + +void WorkerProto::Serialise::write(const StoreDirConfig & store, WriteConn conn, const UnkeyedRealisation & info) +{ + if (GET_PROTOCOL_MINOR(conn.version) < 39) { + throw Error("daemon protocol %d.%d is too old (< 1.29) to understand build trace", + GET_PROTOCOL_MAJOR(conn.version), + GET_PROTOCOL_MINOR(conn.version)); + } + WorkerProto::write(store, conn, info.outPath); + WorkerProto::write(store, conn, info.signatures); +} + + +std::optional WorkerProto::Serialise>::read(const StoreDirConfig & store, ReadConn conn) +{ + if (GET_PROTOCOL_MINOR(conn.version) < 39) { + // Hack to improve compat + (void) WorkerProto::Serialise::read(store, conn); + return std::nullopt; + } else { + auto temp = readNum(conn.from); + switch (temp) { + case 0: + return std::nullopt; + case 1: + return WorkerProto::Serialise::read(store, conn); + default: + throw Error("Invalid optional build trace from remote"); + } + } +} + +void WorkerProto::Serialise>::write(const StoreDirConfig & store, WriteConn conn, const std::optional & info) +{ + if (!info) { + conn.to << uint8_t{0}; + } else { + conn.to << uint8_t{1}; + WorkerProto::write(store, conn, *info); + } +} + + +DrvOutput WorkerProto::Serialise::read(const StoreDirConfig & store, ReadConn conn) +{ + if (GET_PROTOCOL_MINOR(conn.version) < 39) { + throw Error("daemon protocol %d.%d is too old (< 1.29) to understand build trace", + GET_PROTOCOL_MAJOR(conn.version), + GET_PROTOCOL_MINOR(conn.version)); + } + + auto drvPath = WorkerProto::Serialise::read(store, conn); + auto outputName = WorkerProto::Serialise::read(store, conn); + + return DrvOutput { + .drvPath = std::move(drvPath), + .outputName = std::move(outputName), + }; +} + +void WorkerProto::Serialise::write(const StoreDirConfig & store, WriteConn conn, const DrvOutput & info) +{ + if (GET_PROTOCOL_MINOR(conn.version) < 39) { + throw Error("daemon protocol %d.%d is too old (< 1.29) to understand build trace", + GET_PROTOCOL_MAJOR(conn.version), + GET_PROTOCOL_MINOR(conn.version)); + } + WorkerProto::write(store, conn, info.drvPath); + WorkerProto::write(store, conn, info.outputName); +} + + +Realisation WorkerProto::Serialise::read(const StoreDirConfig & store, ReadConn conn) +{ + auto id = WorkerProto::Serialise::read(store, conn); + auto unkeyed = WorkerProto::Serialise::read(store, conn); + + return Realisation { + std::move(unkeyed), + std::move(id), + }; +} + +void WorkerProto::Serialise::write(const StoreDirConfig & store, WriteConn conn, const Realisation & info) +{ + WorkerProto::write(store, conn, info.id); + WorkerProto::write(store, conn, static_cast(info)); +} + } diff --git a/src/libstore/worker-protocol.hh b/src/libstore/worker-protocol.hh index c356fa1bf37..68a534cb447 100644 --- a/src/libstore/worker-protocol.hh +++ b/src/libstore/worker-protocol.hh @@ -13,7 +13,7 @@ namespace nix { /* Note: you generally shouldn't change the protocol version. Define a new `WorkerProto::Feature` instead. */ -#define PROTOCOL_VERSION (1 << 8 | 38) +#define PROTOCOL_VERSION (1 << 8 | 39) #define GET_PROTOCOL_MAJOR(x) ((x) & 0xff00) #define GET_PROTOCOL_MINOR(x) ((x) & 0x00ff) @@ -37,6 +37,9 @@ struct BuildResult; struct KeyedBuildResult; struct ValidPathInfo; struct UnkeyedValidPathInfo; +struct DrvOutput; +struct UnkeyedRealisation; +struct Realisation; enum BuildMode : uint8_t; enum TrustedFlag : bool; @@ -262,6 +265,14 @@ DECLARE_WORKER_SERIALISER(ValidPathInfo); template<> DECLARE_WORKER_SERIALISER(UnkeyedValidPathInfo); template<> +DECLARE_WORKER_SERIALISER(DrvOutput); +template<> +DECLARE_WORKER_SERIALISER(UnkeyedRealisation); +template<> +DECLARE_WORKER_SERIALISER(Realisation); +template<> +DECLARE_WORKER_SERIALISER(std::optional); +template<> DECLARE_WORKER_SERIALISER(BuildMode); template<> DECLARE_WORKER_SERIALISER(std::optional); diff --git a/src/nix/develop.cc b/src/nix/develop.cc index deee89aa1aa..9d6fc2fcd4a 100644 --- a/src/nix/develop.cc +++ b/src/nix/develop.cc @@ -270,16 +270,8 @@ static StorePath getDerivationEnvironment(ref store, ref evalStore output.second = DerivationOutput::Deferred { }; drv.env[output.first] = ""; } - auto hashesModulo = hashDerivationModulo(*evalStore, drv, true); - for (auto & output : drv.outputs) { - Hash h = hashesModulo.hashes.at(output.first); - auto outPath = store->makeOutputPath(output.first, h, drv.name); - output.second = DerivationOutput::InputAddressed { - .path = outPath, - }; - drv.env[output.first] = store->printStorePath(outPath); - } + resolveInputAddressed(*evalStore, drv); } auto shellDrvPath = writeDerivation(*evalStore, drv); diff --git a/src/nix/realisation.cc b/src/nix/realisation.cc index a386d98eac9..fd1ef92846f 100644 --- a/src/nix/realisation.cc +++ b/src/nix/realisation.cc @@ -51,7 +51,7 @@ struct CmdRealisationInfo : BuiltPathsCommand, MixJSON for (auto & path : realisations) { nlohmann::json currentPath; if (auto realisation = std::get_if(&path.raw)) - currentPath = realisation->toJSON(); + currentPath = realisation->toJSON(*store); else currentPath["opaquePath"] = store->printStorePath(path.path()); diff --git a/src/perl/lib/Nix/Store.xs b/src/perl/lib/Nix/Store.xs index 172c3500de0..e0d937d321a 100644 --- a/src/perl/lib/Nix/Store.xs +++ b/src/perl/lib/Nix/Store.xs @@ -43,7 +43,7 @@ O_OBJECT } else { warn( \"${Package}::$func_name() -- \" - \"$var not a blessed SV reference\"); + \"$var not a blessed SV reference\"); XSRETURN_UNDEF; } HERE @@ -165,12 +165,15 @@ StoreWrapper::queryPathInfo(char * path, int base32) } SV * -StoreWrapper::queryRawRealisation(char * outputId) +StoreWrapper::queryRawRealisation(char * drvPath, char * outputName) PPCODE: try { - auto realisation = THIS->store->queryRealisation(DrvOutput::parse(outputId)); + auto realisation = THIS->store->queryRealisation(DrvOutput{ + .drvPath = THIS->store->parseStorePath(drvPath), + .outputName = outputName, + }); if (realisation) - XPUSHs(sv_2mortal(newSVpv(realisation->toJSON().dump().c_str(), 0))); + XPUSHs(sv_2mortal(newSVpv(realisation->toJSON(*THIS->store).dump().c_str(), 0))); else XPUSHs(sv_2mortal(newSVpv("", 0))); } catch (Error & e) { From 52acc3a65d422f05a09749163c0877970df91b12 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 14 Feb 2025 23:12:06 -0500 Subject: [PATCH 4/5] Fix remote protocol unit tests --- src/libstore-test-support/tests/protocol.hh | 10 ++- .../data/serve-protocol/build-result-2.8.bin | Bin 0 -> 360 bytes .../data/serve-protocol/drv-output-2.8.bin | Bin 0 -> 160 bytes .../data/serve-protocol/realisation-2.8.bin | Bin 0 -> 176 bytes .../unkeyed-realisation-2.8.bin | Bin 0 -> 96 bytes .../worker-protocol/build-result-1.39.bin | Bin 0 -> 424 bytes .../data/worker-protocol/drv-output-1.39.bin | Bin 0 -> 160 bytes .../data/worker-protocol/realisation-1.39.bin | Bin 0 -> 176 bytes .../unkeyed-realisation-1.39.bin | Bin 0 -> 96 bytes src/libstore-tests/serve-protocol.cc | 63 ++++++++++++++++-- src/libstore-tests/worker-protocol.cc | 61 +++++++++++++++-- src/libstore/nar-info-disk-cache.cc | 2 +- src/libstore/realisation.hh | 9 --- src/libstore/serve-protocol.cc | 40 +++++++---- src/libstore/worker-protocol.cc | 46 ++++++++----- 15 files changed, 180 insertions(+), 51 deletions(-) create mode 100644 src/libstore-tests/data/serve-protocol/build-result-2.8.bin create mode 100644 src/libstore-tests/data/serve-protocol/drv-output-2.8.bin create mode 100644 src/libstore-tests/data/serve-protocol/realisation-2.8.bin create mode 100644 src/libstore-tests/data/serve-protocol/unkeyed-realisation-2.8.bin create mode 100644 src/libstore-tests/data/worker-protocol/build-result-1.39.bin create mode 100644 src/libstore-tests/data/worker-protocol/drv-output-1.39.bin create mode 100644 src/libstore-tests/data/worker-protocol/realisation-1.39.bin create mode 100644 src/libstore-tests/data/worker-protocol/unkeyed-realisation-1.39.bin diff --git a/src/libstore-test-support/tests/protocol.hh b/src/libstore-test-support/tests/protocol.hh index 3f6799d1ccb..ed3dc406f26 100644 --- a/src/libstore-test-support/tests/protocol.hh +++ b/src/libstore-test-support/tests/protocol.hh @@ -64,12 +64,18 @@ public: } }; -#define VERSIONED_CHARACTERIZATION_TEST(FIXTURE, NAME, STEM, VERSION, VALUE) \ +#define VERSIONED_READ_CHARACTERIZATION_TEST(FIXTURE, NAME, STEM, VERSION, VALUE) \ TEST_F(FIXTURE, NAME ## _read) { \ readProtoTest(STEM, VERSION, VALUE); \ - } \ + } + +#define VERSIONED_WRITE_CHARACTERIZATION_TEST(FIXTURE, NAME, STEM, VERSION, VALUE) \ TEST_F(FIXTURE, NAME ## _write) { \ writeProtoTest(STEM, VERSION, VALUE); \ } +#define VERSIONED_CHARACTERIZATION_TEST(FIXTURE, NAME, STEM, VERSION, VALUE) \ + VERSIONED_READ_CHARACTERIZATION_TEST(FIXTURE, NAME, STEM, VERSION, VALUE) \ + VERSIONED_WRITE_CHARACTERIZATION_TEST(FIXTURE, NAME, STEM, VERSION, VALUE) + } diff --git a/src/libstore-tests/data/serve-protocol/build-result-2.8.bin b/src/libstore-tests/data/serve-protocol/build-result-2.8.bin new file mode 100644 index 0000000000000000000000000000000000000000..fa072599578a351e0f289c1c6bbdc5c1ff90107c GIT binary patch literal 360 zcmZQ&fBv3WB>s7x-9ts literal 0 HcmV?d00001 diff --git a/src/libstore-tests/data/serve-protocol/drv-output-2.8.bin b/src/libstore-tests/data/serve-protocol/drv-output-2.8.bin new file mode 100644 index 0000000000000000000000000000000000000000..5be0b15a34567aed217e56dbf2543f2f60a200be GIT binary patch literal 160 zcmXqJfB^lx%nJSDlKi4n{dB`}^NdR4LR_?NT7JG>N>LeDBQsQgQeqXDWenw$YaRN>LeDBQsQgQeqXDr4QwkXdVL- WR38hJPApDI12JLz!t&H25FY?R+9%Ke literal 0 HcmV?d00001 diff --git a/src/libstore-tests/data/serve-protocol/unkeyed-realisation-2.8.bin b/src/libstore-tests/data/serve-protocol/unkeyed-realisation-2.8.bin new file mode 100644 index 0000000000000000000000000000000000000000..10f4ebcb2eb1622e1c56b462f36a7db1e863b345 GIT binary patch literal 96 zcmdOAfB^lx%nJSDlKi4n{dB`}^NdR4LR_?NT7EtQ6I34yluj&8Ndqxq{KE3oA`l+{ DejyY+ literal 0 HcmV?d00001 diff --git a/src/libstore-tests/data/worker-protocol/build-result-1.39.bin b/src/libstore-tests/data/worker-protocol/build-result-1.39.bin new file mode 100644 index 0000000000000000000000000000000000000000..11bec3e6e3b7a82f7b2d792d687a807644ff9d05 GIT binary patch literal 424 zcmZQ&fB``9-Pv>4xRz8I{I`xM*FNUS#vq N^7F|y52hDn001VXF1-K% literal 0 HcmV?d00001 diff --git a/src/libstore-tests/data/worker-protocol/drv-output-1.39.bin b/src/libstore-tests/data/worker-protocol/drv-output-1.39.bin new file mode 100644 index 0000000000000000000000000000000000000000..5be0b15a34567aed217e56dbf2543f2f60a200be GIT binary patch literal 160 zcmXqJfB^lx%nJSDlKi4n{dB`}^NdR4LR_?NT7JG>N>LeDBQsQgQeqXDWenw$YaRN>LeDBQsQgQeqXDr4QwkXdVL- WR38hJPApDI12JLz!t&H25FY?R+9%Ke literal 0 HcmV?d00001 diff --git a/src/libstore-tests/data/worker-protocol/unkeyed-realisation-1.39.bin b/src/libstore-tests/data/worker-protocol/unkeyed-realisation-1.39.bin new file mode 100644 index 0000000000000000000000000000000000000000..10f4ebcb2eb1622e1c56b462f36a7db1e863b345 GIT binary patch literal 96 zcmdOAfB^lx%nJSDlKi4n{dB`}^NdR4LR_?NT7EtQ6I34yluj&8Ndqxq{KE3oA`l+{ DejyY+ literal 0 HcmV?d00001 diff --git a/src/libstore-tests/serve-protocol.cc b/src/libstore-tests/serve-protocol.cc index 52635ba3b81..a6014d92b9d 100644 --- a/src/libstore-tests/serve-protocol.cc +++ b/src/libstore-tests/serve-protocol.cc @@ -72,7 +72,7 @@ VERSIONED_CHARACTERIZATION_TEST( VERSIONED_CHARACTERIZATION_TEST( ServeProtoTest, - drvOutput, + drvOutput_2_8, "drv-output-2.8", 2 << 8 | 8, (std::tuple { @@ -90,7 +90,7 @@ VERSIONED_CHARACTERIZATION_TEST( VERSIONED_CHARACTERIZATION_TEST( ServeProtoTest, - unkeyedRealisation, + unkeyedRealisation_2_8, "unkeyed-realisation-2.8", 2 << 8 | 8, (UnkeyedRealisation { @@ -100,7 +100,7 @@ VERSIONED_CHARACTERIZATION_TEST( VERSIONED_CHARACTERIZATION_TEST( ServeProtoTest, - realisation, + realisation_2_8, "realisation-2.8", 2 << 8 | 8, (Realisation { @@ -166,9 +166,64 @@ VERSIONED_CHARACTERIZATION_TEST( t; })) -VERSIONED_CHARACTERIZATION_TEST( +/* We now do a lossy read which does not allow us to faithfully right + back, since we changed the data type. We still however want to test + that this read works, and so for that we have a one-way test. */ +VERSIONED_READ_CHARACTERIZATION_TEST( ServeProtoTest, buildResult_2_6, + "build-result-2.6", + 2 << 8 | 6, + ({ + using namespace std::literals::chrono_literals; + std::tuple t { + BuildResult { + .status = BuildResult::OutputRejected, + .errorMsg = "no idea why", + }, + BuildResult { + .status = BuildResult::NotDeterministic, + .errorMsg = "no idea why", + .timesBuilt = 3, + .isNonDeterministic = true, + .startTime = 30, + .stopTime = 50, + }, + BuildResult { + .status = BuildResult::Built, + .timesBuilt = 1, + .builtOutputs = { + { + "foo", + { + .outPath = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo" }, + }, + }, + { + "bar", + { + .outPath = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar" }, + }, + }, + }, + .startTime = 30, + .stopTime = 50, +#if 0 + // These fields are not yet serialized. + // FIXME Include in next version of protocol or document + // why they are skipped. + .cpuUser = std::chrono::milliseconds(500s), + .cpuSystem = std::chrono::milliseconds(604s), +#endif + }, + }; + t; + })) + + +VERSIONED_CHARACTERIZATION_TEST( + ServeProtoTest, + buildResult_2_8, "build-result-2.8", 2 << 8 | 8, ({ diff --git a/src/libstore-tests/worker-protocol.cc b/src/libstore-tests/worker-protocol.cc index a34399b579f..7a9d16c9168 100644 --- a/src/libstore-tests/worker-protocol.cc +++ b/src/libstore-tests/worker-protocol.cc @@ -140,7 +140,7 @@ VERSIONED_CHARACTERIZATION_TEST( VERSIONED_CHARACTERIZATION_TEST( WorkerProtoTest, - unkeyedRealisation, + unkeyedRealisation_1_39, "unkeyed-realisation-1.39", 1 << 8 | 39, (UnkeyedRealisation { @@ -150,7 +150,7 @@ VERSIONED_CHARACTERIZATION_TEST( VERSIONED_CHARACTERIZATION_TEST( WorkerProtoTest, - realisation, + realisation_1_39, "realisation-1.39", 1 << 8 | 39, (Realisation { @@ -187,7 +187,10 @@ VERSIONED_CHARACTERIZATION_TEST( t; })) -VERSIONED_CHARACTERIZATION_TEST( +/* We now do a lossy read which does not allow us to faithfully right + back, since we changed the data type. We still however want to test + that this read works, and so for that we have a one-way test. */ +VERSIONED_READ_CHARACTERIZATION_TEST( WorkerProtoTest, buildResult_1_28, "build-result-1.28", @@ -224,7 +227,8 @@ VERSIONED_CHARACTERIZATION_TEST( t; })) -VERSIONED_CHARACTERIZATION_TEST( +// See above note +VERSIONED_READ_CHARACTERIZATION_TEST( WorkerProtoTest, buildResult_1_29, "build-result-1.29", @@ -268,7 +272,8 @@ VERSIONED_CHARACTERIZATION_TEST( t; })) -VERSIONED_CHARACTERIZATION_TEST( +// See above note +VERSIONED_READ_CHARACTERIZATION_TEST( WorkerProtoTest, buildResult_1_37, "build-result-1.37", @@ -314,6 +319,52 @@ VERSIONED_CHARACTERIZATION_TEST( t; })) +VERSIONED_CHARACTERIZATION_TEST( + WorkerProtoTest, + buildResult_1_39, + "build-result-1.39", + 1 << 8 | 39, + ({ + using namespace std::literals::chrono_literals; + std::tuple t { + BuildResult { + .status = BuildResult::OutputRejected, + .errorMsg = "no idea why", + }, + BuildResult { + .status = BuildResult::NotDeterministic, + .errorMsg = "no idea why", + .timesBuilt = 3, + .isNonDeterministic = true, + .startTime = 30, + .stopTime = 50, + }, + BuildResult { + .status = BuildResult::Built, + .timesBuilt = 1, + .builtOutputs = { + { + "foo", + { + .outPath = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo" }, + }, + }, + { + "bar", + { + .outPath = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar" }, + }, + }, + }, + .startTime = 30, + .stopTime = 50, + .cpuUser = std::chrono::microseconds(500s), + .cpuSystem = std::chrono::microseconds(604s), + }, + }; + t; + })) + VERSIONED_CHARACTERIZATION_TEST( WorkerProtoTest, keyedBuildResult_1_29, diff --git a/src/libstore/nar-info-disk-cache.cc b/src/libstore/nar-info-disk-cache.cc index b334996f00a..6c16ca38dab 100644 --- a/src/libstore/nar-info-disk-cache.cc +++ b/src/libstore/nar-info-disk-cache.cc @@ -93,7 +93,7 @@ class NarInfoDiskCacheImpl : public NarInfoDiskCache Sync _state; - NarInfoDiskCacheImpl(Path dbPath = getCacheDir() + "/binary-cache-v6.sqlite") + NarInfoDiskCacheImpl(Path dbPath = getCacheDir() + "/binary-cache-v7.sqlite") { auto state(_state.lock()); diff --git a/src/libstore/realisation.hh b/src/libstore/realisation.hh index 84fe96515a4..31c75bb4dbc 100644 --- a/src/libstore/realisation.hh +++ b/src/libstore/realisation.hh @@ -119,15 +119,6 @@ struct Realisation : UnkeyedRealisation */ typedef std::map 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 DrvOutputs; - /** * Filter a SingleDrvOutputs to include only specific output names * diff --git a/src/libstore/serve-protocol.cc b/src/libstore/serve-protocol.cc index 63a76b0810c..c704be95bcd 100644 --- a/src/libstore/serve-protocol.cc +++ b/src/libstore/serve-protocol.cc @@ -6,6 +6,7 @@ #include "serve-protocol-impl.hh" #include "archive.hh" #include "path-info.hh" +#include "json-utils.hh" #include @@ -25,12 +26,22 @@ BuildResult ServeProto::Serialise::read(const StoreDirConfig & stor >> status.isNonDeterministic >> status.startTime >> status.stopTime; + if (GET_PROTOCOL_MINOR(conn.version) >= 8) { status.builtOutputs = ServeProto::Serialise>::read(store, conn); } else if (GET_PROTOCOL_MINOR(conn.version) >= 6) { - // We no longer support these types of realisations - (void) ServeProto::Serialise::read(store, conn); + for (auto & [output, realisation] : ServeProto::Serialise::read(store, conn)) { + size_t n = output.find("!"); + if (n == output.npos) + throw Error("Invalid derivation output id %s", output); + status.builtOutputs.insert_or_assign( + output.substr(n + 1), + UnkeyedRealisation{StorePath{ + getString(valueAt(getObject(nlohmann::json::parse(realisation)), "outPath")) + }}); + } } + return status; } @@ -46,6 +57,7 @@ void ServeProto::Serialise::write(const StoreDirConfig & store, Ser << status.isNonDeterministic << status.startTime << status.stopTime; + if (GET_PROTOCOL_MINOR(conn.version) >= 8) { ServeProto::write(store, conn, status.builtOutputs); } else if (GET_PROTOCOL_MINOR(conn.version) >= 6) { @@ -136,9 +148,9 @@ void ServeProto::Serialise::write(const StoreDirConfig UnkeyedRealisation ServeProto::Serialise::read(const StoreDirConfig & store, ReadConn conn) { - if (GET_PROTOCOL_MINOR(conn.version) < 39) { - throw Error("daemon protocol %d.%d is too old (< 1.29) to understand build trace", - GET_PROTOCOL_MAJOR(conn.version), + if (GET_PROTOCOL_MINOR(conn.version) < 8) { + throw Error("daemon protocol %d.%d is too old (< 2.8) to understand build trace", + GET_PROTOCOL_MAJOR(conn.version) >> 8, GET_PROTOCOL_MINOR(conn.version)); } @@ -153,9 +165,9 @@ UnkeyedRealisation ServeProto::Serialise::read(const StoreDi void ServeProto::Serialise::write(const StoreDirConfig & store, WriteConn conn, const UnkeyedRealisation & info) { - if (GET_PROTOCOL_MINOR(conn.version) < 39) { - throw Error("daemon protocol %d.%d is too old (< 1.29) to understand build trace", - GET_PROTOCOL_MAJOR(conn.version), + if (GET_PROTOCOL_MINOR(conn.version) < 8) { + throw Error("daemon protocol %d.%d is too old (< 2.8) to understand build trace", + GET_PROTOCOL_MAJOR(conn.version) >> 8, GET_PROTOCOL_MINOR(conn.version)); } ServeProto::write(store, conn, info.outPath); @@ -165,9 +177,9 @@ void ServeProto::Serialise::write(const StoreDirConfig & sto DrvOutput ServeProto::Serialise::read(const StoreDirConfig & store, ReadConn conn) { - if (GET_PROTOCOL_MINOR(conn.version) < 39) { - throw Error("daemon protocol %d.%d is too old (< 1.29) to understand build trace", - GET_PROTOCOL_MAJOR(conn.version), + if (GET_PROTOCOL_MINOR(conn.version) < 8) { + throw Error("daemon protocol %d.%d is too old (< 2.8) to understand build trace", + GET_PROTOCOL_MAJOR(conn.version) >> 8, GET_PROTOCOL_MINOR(conn.version)); } @@ -182,9 +194,9 @@ DrvOutput ServeProto::Serialise::read(const StoreDirConfig & store, R void ServeProto::Serialise::write(const StoreDirConfig & store, WriteConn conn, const DrvOutput & info) { - if (GET_PROTOCOL_MINOR(conn.version) < 39) { - throw Error("daemon protocol %d.%d is too old (< 1.29) to understand build trace", - GET_PROTOCOL_MAJOR(conn.version), + if (GET_PROTOCOL_MINOR(conn.version) < 8) { + throw Error("daemon protocol %d.%d is too old (< 2.8) to understand build trace", + GET_PROTOCOL_MAJOR(conn.version) >> 8, GET_PROTOCOL_MINOR(conn.version)); } ServeProto::write(store, conn, info.drvPath); diff --git a/src/libstore/worker-protocol.cc b/src/libstore/worker-protocol.cc index ab0ea93d655..cb2a71f2503 100644 --- a/src/libstore/worker-protocol.cc +++ b/src/libstore/worker-protocol.cc @@ -6,6 +6,7 @@ #include "worker-protocol-impl.hh" #include "archive.hh" #include "path-info.hh" +#include "json-utils.hh" #include #include @@ -124,7 +125,7 @@ void WorkerProto::Serialise::write(const StoreDirConfig & store, Wo [&](const StorePath & drvPath) { throw Error("trying to request '%s', but daemon protocol %d.%d is too old (< 1.29) to request a derivation file", store.printStorePath(drvPath), - GET_PROTOCOL_MAJOR(conn.version), + GET_PROTOCOL_MAJOR(conn.version) >> 8, GET_PROTOCOL_MINOR(conn.version)); }, [&](std::monostate) { @@ -157,6 +158,7 @@ BuildResult WorkerProto::Serialise::read(const StoreDirConfig & sto BuildResult res; res.status = static_cast(readInt(conn.from)); conn.from >> res.errorMsg; + if (GET_PROTOCOL_MINOR(conn.version) >= 29) { conn.from >> res.timesBuilt @@ -164,17 +166,27 @@ BuildResult WorkerProto::Serialise::read(const StoreDirConfig & sto >> res.startTime >> res.stopTime; } + if (GET_PROTOCOL_MINOR(conn.version) >= 37) { res.cpuUser = WorkerProto::Serialise>::read(store, conn); res.cpuSystem = WorkerProto::Serialise>::read(store, conn); } - if (GET_PROTOCOL_MINOR(conn.version) >= 28) { - auto builtOutputs = WorkerProto::Serialise::read(store, conn); - for (auto && [output, realisation] : builtOutputs) + + if (GET_PROTOCOL_MINOR(conn.version) >= 39) { + res.builtOutputs = WorkerProto::Serialise>::read(store, conn); + } else if (GET_PROTOCOL_MINOR(conn.version) >= 28) { + for (auto && [output, realisation] : WorkerProto::Serialise::read(store, conn)) { + size_t n = output.find("!"); + if (n == output.npos) + throw Error("Invalid derivation output id %s", output); res.builtOutputs.insert_or_assign( - std::move(output.outputName), - std::move(realisation)); + output.substr(n + 1), + UnkeyedRealisation{StorePath{ + getString(valueAt(getObject(nlohmann::json::parse(realisation)), "outPath")) + }}); + } } + return res; } @@ -183,6 +195,7 @@ void WorkerProto::Serialise::write(const StoreDirConfig & store, Wo conn.to << res.status << res.errorMsg; + if (GET_PROTOCOL_MINOR(conn.version) >= 29) { conn.to << res.timesBuilt @@ -190,16 +203,17 @@ void WorkerProto::Serialise::write(const StoreDirConfig & store, Wo << res.startTime << res.stopTime; } + if (GET_PROTOCOL_MINOR(conn.version) >= 37) { WorkerProto::write(store, conn, res.cpuUser); WorkerProto::write(store, conn, res.cpuSystem); } - if (GET_PROTOCOL_MINOR(conn.version) >= 28) { - // Don't support those types of realisations anymore. - WorkerProto::write(store, conn, StringMap{}); - } + if (GET_PROTOCOL_MINOR(conn.version) >= 39) { WorkerProto::write(store, conn, res.builtOutputs); + } else if (GET_PROTOCOL_MINOR(conn.version) >= 28) { + // Don't support those types of realisations anymore. + WorkerProto::write(store, conn, StringMap{}); } } @@ -286,8 +300,8 @@ void WorkerProto::Serialise::write(const Store UnkeyedRealisation WorkerProto::Serialise::read(const StoreDirConfig & store, ReadConn conn) { if (GET_PROTOCOL_MINOR(conn.version) < 39) { - throw Error("daemon protocol %d.%d is too old (< 1.29) to understand build trace", - GET_PROTOCOL_MAJOR(conn.version), + throw Error("daemon protocol %d.%d is too old (< 1.39) to understand build trace", + GET_PROTOCOL_MAJOR(conn.version) >> 8, GET_PROTOCOL_MINOR(conn.version)); } @@ -303,8 +317,8 @@ UnkeyedRealisation WorkerProto::Serialise::read(const StoreD void WorkerProto::Serialise::write(const StoreDirConfig & store, WriteConn conn, const UnkeyedRealisation & info) { if (GET_PROTOCOL_MINOR(conn.version) < 39) { - throw Error("daemon protocol %d.%d is too old (< 1.29) to understand build trace", - GET_PROTOCOL_MAJOR(conn.version), + throw Error("daemon protocol %d.%d is too old (< 1.39) to understand build trace", + GET_PROTOCOL_MAJOR(conn.version) >> 8, GET_PROTOCOL_MINOR(conn.version)); } WorkerProto::write(store, conn, info.outPath); @@ -346,7 +360,7 @@ DrvOutput WorkerProto::Serialise::read(const StoreDirConfig & store, { if (GET_PROTOCOL_MINOR(conn.version) < 39) { throw Error("daemon protocol %d.%d is too old (< 1.29) to understand build trace", - GET_PROTOCOL_MAJOR(conn.version), + GET_PROTOCOL_MAJOR(conn.version) >> 8, GET_PROTOCOL_MINOR(conn.version)); } @@ -363,7 +377,7 @@ void WorkerProto::Serialise::write(const StoreDirConfig & store, Writ { if (GET_PROTOCOL_MINOR(conn.version) < 39) { throw Error("daemon protocol %d.%d is too old (< 1.29) to understand build trace", - GET_PROTOCOL_MAJOR(conn.version), + GET_PROTOCOL_MAJOR(conn.version) >> 8, GET_PROTOCOL_MINOR(conn.version)); } WorkerProto::write(store, conn, info.drvPath); From 6f610de2626c6b3419ab40828c5c224d3c3dc946 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sat, 15 Feb 2025 13:34:38 -0500 Subject: [PATCH 5/5] WIP rework goals --- maintainers/flake-module.nix | 2 - src/libstore/build/build-trace-goal.cc | 204 ++++++++++++++++++ src/libstore/build/build-trace-goal.hh | 54 +++++ ...erivation-creation-and-realisation-goal.cc | 2 +- src/libstore/build/derivation-goal.cc | 2 +- .../build/derivation-resolution-goal.cc | 48 +++++ .../build/derivation-resolution-goal.hh | 64 ++++++ .../build/drv-output-substitution-goal.cc | 100 +-------- .../build/drv-output-substitution-goal.hh | 23 +- src/libstore/build/worker.hh | 24 ++- src/libstore/ca-specific-schema.sql | 6 +- src/libstore/meson.build | 6 +- 12 files changed, 429 insertions(+), 106 deletions(-) create mode 100644 src/libstore/build/build-trace-goal.cc create mode 100644 src/libstore/build/build-trace-goal.hh create mode 100644 src/libstore/build/derivation-resolution-goal.cc create mode 100644 src/libstore/build/derivation-resolution-goal.hh diff --git a/maintainers/flake-module.nix b/maintainers/flake-module.nix index 5ec6fe4530b..9b28f6b1213 100644 --- a/maintainers/flake-module.nix +++ b/maintainers/flake-module.nix @@ -278,8 +278,6 @@ ''^src/libstore/store-dir-config\.hh$'' ''^src/libstore/build/derivation-goal\.cc$'' ''^src/libstore/build/derivation-goal\.hh$'' - ''^src/libstore/build/drv-output-substitution-goal\.cc$'' - ''^src/libstore/build/drv-output-substitution-goal\.hh$'' ''^src/libstore/build/entry-points\.cc$'' ''^src/libstore/build/goal\.cc$'' ''^src/libstore/build/goal\.hh$'' diff --git a/src/libstore/build/build-trace-goal.cc b/src/libstore/build/build-trace-goal.cc new file mode 100644 index 00000000000..f0bb77f5d99 --- /dev/null +++ b/src/libstore/build/build-trace-goal.cc @@ -0,0 +1,204 @@ +#include "build-trace-goal.hh" +#include "finally.hh" +#include "worker.hh" +#include "substitution-goal.hh" +#include "callback.hh" +#include "util.hh" +#include "derivations.hh" +#include "derivation-resolution-goal.hh" + +namespace nix { + +BuildTraceGoal::BuildTraceGoal(const SingleDerivedPath::Built & id, Worker & worker) + : Goal{worker, DerivedPath::Opaque{.path = StorePath::dummy}} + , id{id} +{ + name = fmt("substitution of '%s'", id.to_string(worker.store)); + trace("created"); +} + +Goal::Co BuildTraceGoal::init() +{ + trace("init"); + + DrvOutput id2{ + .drvPath = StorePath::dummy, + .outputName = id.output, + }; + + // No `std::visit` with coroutines :( + if (const auto * path = std::get_if(&*id.drvPath)) { + // At least we know the drv path statically, can procede + id2.drvPath = path->path; + } else if (const auto * outputDeriving = std::get_if(&*id.drvPath)) { + // Dynamic derivation case, need to resolve that first. + + auto g = worker.makeBuildTraceGoal(outputDeriving->drvPath, outputDeriving->output); + + addWaitee(upcast_goal(g)); + co_await Suspend{}; + + if (nrFailed > 0) { + debug("The output deriving path '%s' could not be resolved", outputDeriving->to_string(worker.store)); + co_return amDone(nrNoSubstituters > 0 || nrIncompleteClosure > 0 ? ecIncompleteClosure : ecFailed); + } + + id2.drvPath = g->outputInfo->outPath; + } + + /* If the derivation already exists, we’re done */ + if ((outputInfo = worker.store.queryRealisation(id2))) { + co_return amDone(ecSuccess); + } + + + /** + * Firstly, whether we know the status, secondly, what it is + */ + std::optional drvIsResolved; + + /* If the derivation has statically-known output paths */ + if (worker.evalStore.isValidPath(id2.drvPath)) { + auto drv = worker.evalStore.readDerivation(id2.drvPath); + auto os = drv.outputsAndOptPaths(worker.store); + /* Mark what we now know */ + drvIsResolved = {drv.inputDrvs.map.empty()}; + if (auto * p = get(os, id2.outputName)) { + if (auto & outPath = p->second) { + outputInfo = std::make_shared(*outPath); + co_return amDone(ecSuccess); + } else { + /* Otherwise, not failure, just looking up build trace below. */ + } + } else { + debug( + "Derivation '%s' does not have output '%s', impossible to find build trace key-value pair", + worker.store.printStorePath(id2.drvPath), + id2.outputName); + co_return amDone(ecFailed); + } + } + + auto subs = settings.useSubstitutes ? getDefaultSubstituters() : std::list>(); + + bool substituterFailed = false; + + if (!drvIsResolved || *drvIsResolved) { + /* Since derivation might be resolved --- isn't known to be + not-resolved, it might have entries. So, let's try querying + the substituters. */ + for (const auto & sub : subs) { + trace("trying next substituter"); + + /* The callback of the curl download below can outlive `this` (if + some other error occurs), so it must not touch `this`. So put + the shared state in a separate refcounted object. */ + auto outPipe = std::make_shared(); +#ifndef _WIN32 + outPipe->create(); +#else + outPipe->createAsyncPipe(worker.ioport.get()); +#endif + + auto promise = std::make_shared>>(); + + sub->queryRealisation( + id2, {[outPipe(outPipe), promise(promise)](std::future> res) { + try { + Finally updateStats([&]() { outPipe->writeSide.close(); }); + promise->set_value(res.get()); + } catch (...) { + promise->set_exception(std::current_exception()); + } + }}); + + worker.childStarted( + shared_from_this(), + { +#ifndef _WIN32 + outPipe->readSide.get() +#else + &*outPipe +#endif + }, + true, + false); + + co_await Suspend{}; + + worker.childTerminated(this); + + std::shared_ptr outputInfo; + try { + outputInfo = promise->get_future().get(); + } catch (std::exception & e) { + printError(e.what()); + substituterFailed = true; + } + + if (!outputInfo) + continue; + + worker.store.registerDrvOutput({*outputInfo, id2}); + + trace("finished"); + co_return amDone(ecSuccess); + } + } + + /* Derivation might not be resolved, let's try doing that */ + trace("trying resolving derivation in build-trace goal"); + + auto g = worker.makeDerivationResolutionGoal(id2.drvPath); + + co_await Suspend{}; + + if (nrFailed > 0) { + /* None left. Terminate this goal and let someone else deal + with it. */ + debug( + "derivation output '%s' is required, but there is no substituter that can provide it", + id2.render(worker.store)); + + if (substituterFailed) { + worker.failedSubstitutions++; + worker.updateProgress(); + } + + /* Hack: don't indicate failure if there were no substituters. + In that case the calling derivation should just do a + build. */ + co_return amDone(substituterFailed ? ecFailed : ecNoSubstituters); + } + + /* This should be set if the goal succeeded */ + assert(g->drv); + + /* Try everything again, now with a resolved derivation */ + auto bt2 = worker.makeBuildTraceGoal(makeConstantStorePathRef(g->resolvedDrvPath), id2.outputName); + + addWaitee(bt2); + co_await Suspend{}; + + /* Set the build trace value as our own. Note the signure will not + match our key since we're the unresolved derivation, but that's + fine. We're not writing it to the DB; that's `bt2`' job. */ + if (bt2->outputInfo) + outputInfo = bt2->outputInfo; + + co_return amDone(bt2->exitCode, bt2->ex); +} + +std::string BuildTraceGoal::key() +{ + /* "a$" ensures substitution goals happen before derivation + goals. */ + return "a$" + std::string(id.to_string(worker.store)); +} + +void BuildTraceGoal::handleEOF(Descriptor fd) +{ + worker.wakeUp(shared_from_this()); +} + +} diff --git a/src/libstore/build/build-trace-goal.hh b/src/libstore/build/build-trace-goal.hh new file mode 100644 index 00000000000..8df9979e5e4 --- /dev/null +++ b/src/libstore/build/build-trace-goal.hh @@ -0,0 +1,54 @@ +#pragma once +///@file + +#include +#include + +#include "store-api.hh" +#include "goal.hh" +#include "realisation.hh" +#include "muxable-pipe.hh" + +namespace nix { + +class Worker; + +/** + * Try to recursively obtain build trace key-value pairs in order to + * resolve the given output deriving path. + */ +class BuildTraceGoal : public Goal +{ + + /** + * The output derivation path we're trying to reasolve. + */ + SingleDerivedPath::Built id; + +public: + BuildTraceGoal(const SingleDerivedPath::Built & id, Worker & worker); + + /** + * The realisation corresponding to the given output id. + * Will be filled once we can get it. + */ + std::shared_ptr outputInfo; + + Co init() override; + + void timedOut(Error && ex) override + { + unreachable(); + }; + + std::string key() override; + + void handleEOF(Descriptor fd) override; + + JobCategory jobCategory() const override + { + return JobCategory::Substitution; + }; +}; + +} diff --git a/src/libstore/build/derivation-creation-and-realisation-goal.cc b/src/libstore/build/derivation-creation-and-realisation-goal.cc index c33b7571f04..49fe1719f2d 100644 --- a/src/libstore/build/derivation-creation-and-realisation-goal.cc +++ b/src/libstore/build/derivation-creation-and-realisation-goal.cc @@ -42,7 +42,7 @@ std::string DerivationCreationAndRealisationGoal::key() i.e. a derivation named "aardvark" always comes before "baboon". And substitution goals and inner derivation goals always happen before derivation goals (due to "b$"). */ - return "c$" + std::string(pathPartOfReq(*drvReq).name()) + "$" + drvReq->to_string(worker.store); + return "d$" + std::string(pathPartOfReq(*drvReq).name()) + "$" + drvReq->to_string(worker.store); } void DerivationCreationAndRealisationGoal::timedOut(Error && ex) {} diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 71abd6d30d2..bf3c889a617 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -92,7 +92,7 @@ std::string DerivationGoal::key() i.e. a derivation named "aardvark" always comes before "baboon". And substitution goals always happen before derivation goals (due to "b$"). */ - return "b$" + std::string(drvPath.name()) + "$" + worker.store.printStorePath(drvPath); + return "c$" + std::string(drvPath.name()) + "$" + worker.store.printStorePath(drvPath); } diff --git a/src/libstore/build/derivation-resolution-goal.cc b/src/libstore/build/derivation-resolution-goal.cc new file mode 100644 index 00000000000..3e578a10989 --- /dev/null +++ b/src/libstore/build/derivation-resolution-goal.cc @@ -0,0 +1,48 @@ +#include "drv-output-substitution-goal.hh" +#include "finally.hh" +#include "worker.hh" +#include "substitution-goal.hh" +#include "callback.hh" + +namespace nix { + +DrvOutputSubstitutionGoal::DrvOutputSubstitutionGoal( + const DrvOutput & id, Worker & worker, RepairFlag repair, std::optional ca) + : Goal(worker, DerivedPath::Opaque{StorePath::dummy}) + , id(id) +{ + name = fmt("substitution of '%s'", id.render(worker.store)); + trace("created"); +} + +Goal::Co DrvOutputSubstitutionGoal::init() +{ + trace("init"); + + addWaitee(upcast_goal(worker.makeBuildTraceGoal(makeConstantStorePathRef(id.drvPath), id.outputName))); + co_await Suspend{}; + + trace("output path substituted"); + + if (nrFailed > 0) { + debug("The output path of the derivation output '%s' could not be substituted", id.render(worker.store)); + co_return amDone(nrNoSubstituters > 0 || nrIncompleteClosure > 0 ? ecIncompleteClosure : ecFailed); + } + + trace("finished"); + co_return amDone(ecSuccess); +} + +std::string DrvOutputSubstitutionGoal::key() +{ + /* "a$" ensures substitution goals happen before derivation + goals. */ + return "b$" + std::string(id.render(worker.store)); +} + +void DrvOutputSubstitutionGoal::handleEOF(Descriptor fd) +{ + worker.wakeUp(shared_from_this()); +} + +} diff --git a/src/libstore/build/derivation-resolution-goal.hh b/src/libstore/build/derivation-resolution-goal.hh new file mode 100644 index 00000000000..c76e12fa13f --- /dev/null +++ b/src/libstore/build/derivation-resolution-goal.hh @@ -0,0 +1,64 @@ +#pragma once +///@file + +#include +#include + +#include "store-api.hh" +#include "goal.hh" +#include "realisation.hh" +#include "muxable-pipe.hh" + +namespace nix { + +class Worker; + +/** + * The purpose of this is to resolve the given derivation, so that it + * only has constant deriving paths as inputs. + */ +class DerivationResolutionGoal : public Goal +{ + + /** + * The derivation we're trying to substitute + */ + StorePath drvPath; + +public: + DerivationResolutionGoal( + const DrvOutput & id, + Worker & worker, + RepairFlag repair = NoRepair, + std::optional ca = std::nullopt); + + /** + * The resolved derivation, if we succeeded. + */ + std::shared_ptr drv; + + /** + * The path to derivation above, if we succeeded. + * + * Garbage that should not be read otherwise. + */ + StorePath resolvedDrvPath = StorePath::dummy; + + Co init() override; + + void timedOut(Error && ex) override + { + unreachable(); + }; + + std::string key() override; + + void handleEOF(Descriptor fd) override; + + JobCategory jobCategory() const override + { + return JobCategory::Substitution; + }; +}; + +} diff --git a/src/libstore/build/drv-output-substitution-goal.cc b/src/libstore/build/drv-output-substitution-goal.cc index 0972555740f..3e578a10989 100644 --- a/src/libstore/build/drv-output-substitution-goal.cc +++ b/src/libstore/build/drv-output-substitution-goal.cc @@ -7,114 +7,37 @@ namespace nix { DrvOutputSubstitutionGoal::DrvOutputSubstitutionGoal( - const DrvOutput & id, - Worker & worker, - RepairFlag repair, - std::optional ca) - : Goal(worker, DerivedPath::Opaque { StorePath::dummy }) + const DrvOutput & id, Worker & worker, RepairFlag repair, std::optional ca) + : Goal(worker, DerivedPath::Opaque{StorePath::dummy}) , id(id) { name = fmt("substitution of '%s'", id.render(worker.store)); trace("created"); } - Goal::Co DrvOutputSubstitutionGoal::init() { trace("init"); - /* If the derivation already exists, we’re done */ - if (worker.store.queryRealisation(id)) { - co_return amDone(ecSuccess); - } - - auto subs = settings.useSubstitutes ? getDefaultSubstituters() : std::list>(); - - bool substituterFailed = false; - - for (const auto & sub : subs) { - trace("trying next substituter"); - - /* The callback of the curl download below can outlive `this` (if - some other error occurs), so it must not touch `this`. So put - the shared state in a separate refcounted object. */ - auto outPipe = std::make_shared(); - #ifndef _WIN32 - outPipe->create(); - #else - outPipe->createAsyncPipe(worker.ioport.get()); - #endif - - auto promise = std::make_shared>>(); - - sub->queryRealisation( - id, - { [outPipe(outPipe), promise(promise)](std::future> res) { - try { - Finally updateStats([&]() { outPipe->writeSide.close(); }); - promise->set_value(res.get()); - } catch (...) { - promise->set_exception(std::current_exception()); - } - } }); - - worker.childStarted(shared_from_this(), { - #ifndef _WIN32 - outPipe->readSide.get() - #else - &*outPipe - #endif - }, true, false); - - co_await Suspend{}; + addWaitee(upcast_goal(worker.makeBuildTraceGoal(makeConstantStorePathRef(id.drvPath), id.outputName))); + co_await Suspend{}; - worker.childTerminated(this); + trace("output path substituted"); - try { - outputInfo = promise->get_future().get(); - } catch (std::exception & e) { - printError(e.what()); - substituterFailed = true; - } - - if (!outputInfo) continue; - - addWaitee(worker.makePathSubstitutionGoal(outputInfo->outPath)); - co_await Suspend{}; - - trace("output path substituted"); - - if (nrFailed > 0) { - debug("The output path of the derivation output '%s' could not be substituted", id.render(worker.store)); - co_return amDone(nrNoSubstituters > 0 || nrIncompleteClosure > 0 ? ecIncompleteClosure : ecFailed); - } - - worker.store.registerDrvOutput({*outputInfo, id}); - - trace("finished"); - co_return amDone(ecSuccess); + if (nrFailed > 0) { + debug("The output path of the derivation output '%s' could not be substituted", id.render(worker.store)); + co_return amDone(nrNoSubstituters > 0 || nrIncompleteClosure > 0 ? ecIncompleteClosure : ecFailed); } - /* None left. Terminate this goal and let someone else deal - with it. */ - debug("derivation output '%s' is required, but there is no substituter that can provide it", id.render(worker.store)); - - if (substituterFailed) { - worker.failedSubstitutions++; - worker.updateProgress(); - } - - /* Hack: don't indicate failure if there were no substituters. - In that case the calling derivation should just do a - build. */ - co_return amDone(substituterFailed ? ecFailed : ecNoSubstituters); + trace("finished"); + co_return amDone(ecSuccess); } std::string DrvOutputSubstitutionGoal::key() { /* "a$" ensures substitution goals happen before derivation goals. */ - return "a$" + std::string(id.render(worker.store)); + return "b$" + std::string(id.render(worker.store)); } void DrvOutputSubstitutionGoal::handleEOF(Descriptor fd) @@ -122,5 +45,4 @@ void DrvOutputSubstitutionGoal::handleEOF(Descriptor fd) worker.wakeUp(shared_from_this()); } - } diff --git a/src/libstore/build/drv-output-substitution-goal.hh b/src/libstore/build/drv-output-substitution-goal.hh index cfec79a225b..f0c8e97cdb1 100644 --- a/src/libstore/build/drv-output-substitution-goal.hh +++ b/src/libstore/build/drv-output-substitution-goal.hh @@ -20,7 +20,8 @@ class Worker; * 2. Substitute the corresponding output path * 3. Register the output info */ -class DrvOutputSubstitutionGoal : public Goal { +class DrvOutputSubstitutionGoal : public Goal +{ /** * The drv output we're trying to substitute @@ -28,23 +29,25 @@ class DrvOutputSubstitutionGoal : public Goal { DrvOutput id; public: - DrvOutputSubstitutionGoal(const DrvOutput & id, Worker & worker, RepairFlag repair = NoRepair, std::optional ca = std::nullopt); - - /** - * The realisation corresponding to the given output id. - * Will be filled once we can get it. - */ - std::shared_ptr outputInfo; + DrvOutputSubstitutionGoal( + const DrvOutput & id, + Worker & worker, + RepairFlag repair = NoRepair, + std::optional ca = std::nullopt); Co init() override; - void timedOut(Error && ex) override { unreachable(); }; + void timedOut(Error && ex) override + { + unreachable(); + }; std::string key() override; void handleEOF(Descriptor fd) override; - JobCategory jobCategory() const override { + JobCategory jobCategory() const override + { return JobCategory::Substitution; }; }; diff --git a/src/libstore/build/worker.hh b/src/libstore/build/worker.hh index efd518f9995..1a4329d897e 100644 --- a/src/libstore/build/worker.hh +++ b/src/libstore/build/worker.hh @@ -18,6 +18,8 @@ struct DerivationCreationAndRealisationGoal; struct DerivationGoal; struct PathSubstitutionGoal; class DrvOutputSubstitutionGoal; +class BuildTraceGoal; +class DerivationResolutionGoal; /** * Workaround for not being able to declare a something like @@ -34,6 +36,8 @@ class DrvOutputSubstitutionGoal; GoalPtr upcast_goal(std::shared_ptr subGoal); GoalPtr upcast_goal(std::shared_ptr subGoal); GoalPtr upcast_goal(std::shared_ptr subGoal); +GoalPtr upcast_goal(std::shared_ptr subGoal); +GoalPtr upcast_goal(std::shared_ptr subGoal); typedef std::chrono::time_point steady_time_point; @@ -123,10 +127,12 @@ private: */ DerivedPathMap> outerDerivationGoals; + DerivedPathMap> buildTraceGoals; std::map> derivationGoals; std::map> substitutionGoals; std::map> drvOutputSubstitutionGoals; + std::map> derivationResolutionGoals; /** * Goals waiting for busy paths to be unlocked. @@ -232,11 +238,27 @@ public: const OutputsSpec & wantedOutputs, BuildMode buildMode = bmNormal); /** - * @ref PathSubstitutionGoal "substitution goal" + * @ref PathSubstitutionGoal "path substitution goal" */ std::shared_ptr makePathSubstitutionGoal(const StorePath & storePath, RepairFlag repair = NoRepair, std::optional ca = std::nullopt); + + /** + * @ref DrvOutputSubstitutionGoal "derivation output substitution goal" + */ std::shared_ptr makeDrvOutputSubstitutionGoal(const DrvOutput & id, RepairFlag repair = NoRepair, std::optional ca = std::nullopt); + /** + * @ref BuildTraceGoal "derivation output substitution goal" + */ + std::shared_ptr makeBuildTraceGoal( + ref drvPath, + const std::string & outputName); + + /** + * @ref DerivationResolutionGoal "derivation resolution goal" + */ + std::shared_ptr makeDerivationResolutionGoal(const StorePath & drvPath); + /** * Make a goal corresponding to the `DerivedPath`. * diff --git a/src/libstore/ca-specific-schema.sql b/src/libstore/ca-specific-schema.sql index d563b33d879..b0095fb3322 100644 --- a/src/libstore/ca-specific-schema.sql +++ b/src/libstore/ca-specific-schema.sql @@ -8,7 +8,11 @@ create table if not exists Realisations ( outputName text not null, -- symbolic output id, usually "out" outputPath integer not null, signatures text, -- space-separated list - foreign key (outputPath) references ValidPaths(id) on delete cascade + + -- No such foreign key because we may well want realisations for + -- garbage-collected dependencies + + --foreign key (outputPath) references ValidPaths(id) on delete cascade ); create index if not exists IndexRealisations on Realisations(drvPath, outputName); diff --git a/src/libstore/meson.build b/src/libstore/meson.build index d42b010614a..26a764c1132 100644 --- a/src/libstore/meson.build +++ b/src/libstore/meson.build @@ -182,8 +182,10 @@ subdir('nix-meson-build-support/common') sources = files( 'binary-cache-store.cc', 'build-result.cc', - 'build/derivation-goal.cc', + 'build/build-trace-goal.cc', 'build/derivation-creation-and-realisation-goal.cc', + 'build/derivation-goal.cc', + 'build/derivation-resolution-goal.cc', 'build/drv-output-substitution-goal.cc', 'build/entry-points.cc', 'build/goal.cc', @@ -255,8 +257,10 @@ include_dirs = [ headers = [config_h] + files( 'binary-cache-store.hh', 'build-result.hh', + 'build/build-trace-goal.hh', 'build/derivation-goal.hh', 'build/derivation-creation-and-realisation-goal.hh', + 'build/derivation-resolution-goal.hh', 'build/drv-output-substitution-goal.hh', 'build/goal.hh', 'build/substitution-goal.hh',