1
0
Fork 0
mirror of https://github.com/NixOS/nix synced 2025-06-24 22:11:15 +02:00

Revert "Revert "Revert "Adapt scheduler to work with dynamic derivations"""

The bug reappeared after all, and the fix introduced a different bug. I
just reverted on 2.27 first, in #12576, but upon further introspection
and discussion with @roberth, with preparing for and travelling to
Planet Nix I will not be able to fix it on `master` soon enough for a
revert to not be warranted here in the meantime also.

This reverts commit c98525235f.
This commit is contained in:
John Ericson 2025-02-27 13:42:03 -05:00
parent 7cfc52fd42
commit f4f28cdd0e
16 changed files with 45 additions and 374 deletions

View file

@ -1,126 +0,0 @@
#include "derivation-creation-and-realisation-goal.hh"
#include "worker.hh"
namespace nix {
DerivationCreationAndRealisationGoal::DerivationCreationAndRealisationGoal(
ref<SingleDerivedPath> 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<StorePath> {
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);
}
}

View file

@ -1,88 +0,0 @@
#pragma once
#include "parsed-derivations.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<SingleDerivedPath> drvReq;
/**
* The path of the derivation, once obtained.
**/
std::optional<StorePath> optDrvPath;
/**
* The goal for the corresponding concrete derivation.
**/
std::shared_ptr<DerivationGoal> 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<SingleDerivedPath> 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;
};
};
}

View file

@ -137,8 +137,21 @@ 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. */
@ -1540,24 +1553,23 @@ void DerivationGoal::waiteeDone(GoalPtr waitee, ExitCode result)
if (!useDerivation || !drv) return;
auto & fullDrv = *dynamic_cast<Derivation *>(drv.get());
std::optional info = tryGetConcreteDrvGoal(waitee);
if (!info) return;
const auto & [dg, drvReq] = *info;
auto * dg = dynamic_cast<DerivationGoal *>(&*waitee);
if (!dg) return;
auto * nodeP = fullDrv.inputDrvs.findSlot(drvReq.get());
auto * nodeP = fullDrv.inputDrvs.findSlot(DerivedPath::Opaque { .path = dg->drvPath });
if (!nodeP) return;
auto & outputs = nodeP->value;
for (auto & outputName : outputs) {
auto buildResult = dg.get().getBuildResult(DerivedPath::Built {
.drvPath = makeConstantStorePathRef(dg.get().drvPath),
auto buildResult = dg->getBuildResult(DerivedPath::Built {
.drvPath = makeConstantStorePathRef(dg->drvPath),
.outputs = OutputsSpec::Names { outputName },
});
if (buildResult.success()) {
auto i = buildResult.builtOutputs.find(outputName);
if (i != buildResult.builtOutputs.end())
inputDrvOutputs.insert_or_assign(
{ dg.get().drvPath, outputName },
{ dg->drvPath, outputName },
i->second.outPath);
}
}

View file

@ -57,10 +57,6 @@ 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
{

View file

@ -1,7 +1,6 @@
#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"
@ -30,8 +29,8 @@ void Store::buildPaths(const std::vector<DerivedPath> & reqs, BuildMode buildMod
}
if (i->exitCode != Goal::ecSuccess) {
#ifndef _WIN32 // TODO Enable building on Windows
if (auto i2 = dynamic_cast<DerivationCreationAndRealisationGoal *>(i.get()))
failed.insert(i2->drvReq->to_string(*this));
if (auto i2 = dynamic_cast<DerivationGoal *>(i.get()))
failed.insert(printStorePath(i2->drvPath));
else
#endif
if (auto i2 = dynamic_cast<PathSubstitutionGoal *>(i.get()))

View file

@ -175,7 +175,7 @@ Goal::Done Goal::amDone(ExitCode result, std::optional<Error> ex)
exitCode = result;
if (ex) {
if (!preserveException && !waiters.empty())
if (!waiters.empty())
logError(ex->info());
else
this->ex = std::move(*ex);

View file

@ -50,16 +50,6 @@ 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<Goal>
@ -383,17 +373,6 @@ 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.
*/

View file

@ -4,7 +4,6 @@
#include "substitution-goal.hh"
#include "drv-output-substitution-goal.hh"
#include "derivation-goal.hh"
#include "derivation-creation-and-realisation-goal.hh"
#ifndef _WIN32 // TODO Enable building on Windows
# include "local-derivation-goal.hh"
# include "hook-instance.hh"
@ -44,24 +43,6 @@ Worker::~Worker()
}
std::shared_ptr<DerivationCreationAndRealisationGoal> Worker::makeDerivationCreationAndRealisationGoal(
ref<SingleDerivedPath> drvReq,
const OutputsSpec & wantedOutputs,
BuildMode buildMode)
{
std::weak_ptr<DerivationCreationAndRealisationGoal> & goal_weak = outerDerivationGoals.ensureSlot(*drvReq).value;
std::shared_ptr<DerivationCreationAndRealisationGoal> goal = goal_weak.lock();
if (!goal) {
goal = std::make_shared<DerivationCreationAndRealisationGoal>(drvReq, wantedOutputs, *this, buildMode);
goal_weak = goal;
wakeUp(goal);
} else {
goal->addWantedOutputs(wantedOutputs);
}
return goal;
}
std::shared_ptr<DerivationGoal> Worker::makeDerivationGoalCommon(
const StorePath & drvPath,
const OutputsSpec & wantedOutputs,
@ -139,7 +120,10 @@ GoalPtr Worker::makeGoal(const DerivedPath & req, BuildMode buildMode)
{
return std::visit(overloaded {
[&](const DerivedPath::Built & bfd) -> GoalPtr {
return makeDerivationCreationAndRealisationGoal(bfd.drvPath, bfd.outputs, buildMode);
if (auto bop = std::get_if<DerivedPath::Opaque>(&*bfd.drvPath))
return makeDerivationGoal(bop->path, bfd.outputs, buildMode);
else
throw UnimplementedError("Building dynamic derivations in one shot is not yet implemented.");
},
[&](const DerivedPath::Opaque & bo) -> GoalPtr {
return makePathSubstitutionGoal(bo.path, buildMode == bmRepair ? Repair : NoRepair);
@ -148,46 +132,24 @@ GoalPtr Worker::makeGoal(const DerivedPath & req, BuildMode buildMode)
}
template<typename K, typename V, typename F>
static void cullMap(std::map<K, V> & goalMap, F f)
{
for (auto i = goalMap.begin(); i != goalMap.end();)
if (!f(i->second))
i = goalMap.erase(i);
else ++i;
}
template<typename K, typename G>
static void removeGoal(std::shared_ptr<G> goal, std::map<K, std::weak_ptr<G>> & goalMap)
{
/* !!! inefficient */
cullMap(goalMap, [&](const std::weak_ptr<G> & gp) -> bool {
return gp.lock() != goal;
});
}
template<typename K>
static void removeGoal(std::shared_ptr<DerivationCreationAndRealisationGoal> goal, std::map<K, DerivedPathMap<std::weak_ptr<DerivationCreationAndRealisationGoal>>::ChildNode> & goalMap);
template<typename K>
static void removeGoal(std::shared_ptr<DerivationCreationAndRealisationGoal> goal, std::map<K, DerivedPathMap<std::weak_ptr<DerivationCreationAndRealisationGoal>>::ChildNode> & goalMap)
{
/* !!! inefficient */
cullMap(goalMap, [&](DerivedPathMap<std::weak_ptr<DerivationCreationAndRealisationGoal>>::ChildNode & node) -> bool {
if (node.value.lock() == goal)
node.value.reset();
removeGoal(goal, node.childMap);
return !node.value.expired() || !node.childMap.empty();
});
for (auto i = goalMap.begin();
i != goalMap.end(); )
if (i->second.lock() == goal) {
auto j = i; ++j;
goalMap.erase(i);
i = j;
}
else ++i;
}
void Worker::removeGoal(GoalPtr goal)
{
if (auto drvGoal = std::dynamic_pointer_cast<DerivationCreationAndRealisationGoal>(goal))
nix::removeGoal(drvGoal, outerDerivationGoals.map);
else if (auto drvGoal = std::dynamic_pointer_cast<DerivationGoal>(goal))
if (auto drvGoal = std::dynamic_pointer_cast<DerivationGoal>(goal))
nix::removeGoal(drvGoal, derivationGoals);
else
if (auto subGoal = std::dynamic_pointer_cast<PathSubstitutionGoal>(goal))
@ -253,9 +215,6 @@ void Worker::childStarted(GoalPtr goal, const std::set<MuxablePipePollState::Com
case JobCategory::Build:
nrLocalBuilds++;
break;
case JobCategory::Administration:
/* Intentionally not limited, see docs */
break;
default:
unreachable();
}
@ -279,9 +238,6 @@ void Worker::childTerminated(Goal * goal, bool wakeSleepers)
assert(nrLocalBuilds > 0);
nrLocalBuilds--;
break;
case JobCategory::Administration:
/* Intentionally not limited, see docs */
break;
default:
unreachable();
}
@ -334,9 +290,9 @@ void Worker::run(const Goals & _topGoals)
for (auto & i : _topGoals) {
topGoals.insert(i);
if (auto goal = dynamic_cast<DerivationCreationAndRealisationGoal *>(i.get())) {
if (auto goal = dynamic_cast<DerivationGoal *>(i.get())) {
topPaths.push_back(DerivedPath::Built {
.drvPath = goal->drvReq,
.drvPath = makeConstantStorePathRef(goal->drvPath),
.outputs = goal->wantedOutputs,
});
} else
@ -596,22 +552,4 @@ GoalPtr upcast_goal(std::shared_ptr<DrvOutputSubstitutionGoal> subGoal)
return subGoal;
}
GoalPtr upcast_goal(std::shared_ptr<DerivationGoal> subGoal)
{
return subGoal;
}
std::optional<std::pair<std::reference_wrapper<const DerivationGoal>, std::reference_wrapper<const SingleDerivedPath>>> tryGetConcreteDrvGoal(GoalPtr waitee)
{
auto * odg = dynamic_cast<DerivationCreationAndRealisationGoal *>(&*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),
}};
}
}

View file

@ -3,7 +3,6 @@
#include "types.hh"
#include "store-api.hh"
#include "derived-path-map.hh"
#include "goal.hh"
#include "realisation.hh"
#include "muxable-pipe.hh"
@ -14,7 +13,6 @@
namespace nix {
/* Forward definition. */
struct DerivationCreationAndRealisationGoal;
struct DerivationGoal;
struct PathSubstitutionGoal;
class DrvOutputSubstitutionGoal;
@ -33,25 +31,9 @@ class DrvOutputSubstitutionGoal;
*/
GoalPtr upcast_goal(std::shared_ptr<PathSubstitutionGoal> subGoal);
GoalPtr upcast_goal(std::shared_ptr<DrvOutputSubstitutionGoal> subGoal);
GoalPtr upcast_goal(std::shared_ptr<DerivationGoal> subGoal);
typedef std::chrono::time_point<std::chrono::steady_clock> 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::pair<std::reference_wrapper<const DerivationGoal>, std::reference_wrapper<const SingleDerivedPath>>> 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
@ -121,9 +103,6 @@ private:
* Maps used to prevent multiple instantiations of a goal for the
* same derivation / path.
*/
DerivedPathMap<std::weak_ptr<DerivationCreationAndRealisationGoal>> outerDerivationGoals;
std::map<StorePath, std::weak_ptr<DerivationGoal>> derivationGoals;
std::map<StorePath, std::weak_ptr<PathSubstitutionGoal>> substitutionGoals;
std::map<DrvOutput, std::weak_ptr<DrvOutputSubstitutionGoal>> drvOutputSubstitutionGoals;
@ -217,9 +196,6 @@ public:
* @ref DerivationGoal "derivation goal"
*/
private:
std::shared_ptr<DerivationCreationAndRealisationGoal> makeDerivationCreationAndRealisationGoal(
ref<SingleDerivedPath> drvPath,
const OutputsSpec & wantedOutputs, BuildMode buildMode = bmNormal);
std::shared_ptr<DerivationGoal> makeDerivationGoalCommon(
const StorePath & drvPath, const OutputsSpec & wantedOutputs,
std::function<std::shared_ptr<DerivationGoal>()> mkDrvGoal);

View file

@ -52,7 +52,6 @@ typename DerivedPathMap<V>::ChildNode * DerivedPathMap<V>::findSlot(const Single
// instantiations
#include "derivation-creation-and-realisation-goal.hh"
namespace nix {
template<>
@ -69,7 +68,4 @@ std::strong_ordering DerivedPathMap<std::set<std::string>>::ChildNode::operator
template struct DerivedPathMap<std::set<std::string>>::ChildNode;
template struct DerivedPathMap<std::set<std::string>>;
template struct DerivedPathMap<std::weak_ptr<DerivationCreationAndRealisationGoal>>;
};

View file

@ -21,11 +21,8 @@ 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. For example, the scheduler uses
* `DerivedPathMap<std::weak_ptr<DerivationCreationAndRealisationGoal>>` to
* remember which goals correspond to which outputs. `* const Something`
* or `std::optional<Something>` would also be good choices for
* "optional" types.
* value. `* const Something` or `std::optional<Something>` would be
* good choices for "optional" types.
*/
template<typename V>
struct DerivedPathMap {

View file

@ -183,7 +183,6 @@ 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',
@ -257,7 +256,6 @@ 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',

View file

@ -18,9 +18,4 @@ clearStore
drvDep=$(nix-instantiate ./text-hashed-output.nix -A producingDrv)
# Store layer needs bugfix
requireDaemonNewerThan "2.27pre20250205"
out2=$(nix build "${drvDep}^out^out" --no-link)
test $out1 == $out2
expectStderr 1 nix build "${drvDep}^out^out" --no-link | grepQuiet "Building dynamic derivations in one shot is not yet implemented"

View file

@ -13,4 +13,4 @@ restartDaemon
NIX_BIN_DIR="$(dirname "$(type -p nix)")"
export NIX_BIN_DIR
nix build -L --file ./non-trivial.nix --no-link
expectStderr 1 nix build -L --file ./non-trivial.nix --no-link | grepQuiet "Building dynamic derivations in one shot is not yet implemented"

View file

@ -4,11 +4,8 @@ source common.sh
out1=$(nix-build ./text-hashed-output.nix -A hello --no-out-link)
# Store layer needs bugfix
requireDaemonNewerThan "2.27pre20250205"
clearStore
out2=$(nix-build ./text-hashed-output.nix -A wrapper --no-out-link)
expectStderr 1 nix-build ./text-hashed-output.nix -A wrapper --no-out-link | grepQuiet "Building dynamic derivations in one shot is not yet implemented"
diff -r $out1 $out2
# diff -r $out1 $out2

View file

@ -5,6 +5,8 @@ source common.sh
# Store layer needs bugfix
requireDaemonNewerThan "2.27pre20250205"
skipTest "dyn drv input scheduling had to be reverted for 2.27"
expected=100
if [[ -v NIX_DAEMON_PACKAGE ]]; then expected=1; fi # work around the daemon not returning a 100 status correctly