Skip to content

Commit 07f853b

Browse files
roberthroberth
authored
Merge pull request #9415 from NixOS/fix-dynamic-derivations
Revert "Revert "Adapt scheduler to work with dynamic derivations
2 parents 8b91127 + c985252 commit 07f853b

17 files changed

+397
-42
lines changed

‎src/libstore/build/derivation-creation-and-realisation-goal.cc‎

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
#include "derivation-creation-and-realisation-goal.hh"
2+
#include "worker.hh"
3+
4+
namespace nix {
5+
6+
DerivationCreationAndRealisationGoal::DerivationCreationAndRealisationGoal(
7+
ref<SingleDerivedPath> drvReq, const OutputsSpec & wantedOutputs, Worker & worker, BuildMode buildMode)
8+
: Goal(worker, DerivedPath::Built{.drvPath = drvReq, .outputs = wantedOutputs})
9+
, drvReq(drvReq)
10+
, wantedOutputs(wantedOutputs)
11+
, buildMode(buildMode)
12+
{
13+
name =
14+
fmt("outer obtaining drv from '%s' and then building outputs %s",
15+
drvReq->to_string(worker.store),
16+
std::visit(
17+
overloaded{
18+
[&](const OutputsSpec::All) -> std::string { return "* (all of them)"; },
19+
[&](const OutputsSpec::Names os) { return concatStringsSep(", ", quoteStrings(os)); },
20+
},
21+
wantedOutputs.raw));
22+
trace("created outer");
23+
24+
worker.updateProgress();
25+
}
26+
27+
DerivationCreationAndRealisationGoal::~DerivationCreationAndRealisationGoal() {}
28+
29+
static StorePath pathPartOfReq(const SingleDerivedPath & req)
30+
{
31+
return std::visit(
32+
overloaded{
33+
[&](const SingleDerivedPath::Opaque & bo) { return bo.path; },
34+
[&](const SingleDerivedPath::Built & bfd) { return pathPartOfReq(*bfd.drvPath); },
35+
},
36+
req.raw());
37+
}
38+
39+
std::string DerivationCreationAndRealisationGoal::key()
40+
{
41+
/* Ensure that derivations get built in order of their name,
42+
i.e. a derivation named "aardvark" always comes before "baboon". And
43+
substitution goals and inner derivation goals always happen before
44+
derivation goals (due to "b$"). */
45+
return "c$" + std::string(pathPartOfReq(*drvReq).name()) + "$" + drvReq->to_string(worker.store);
46+
}
47+
48+
void DerivationCreationAndRealisationGoal::timedOut(Error && ex) {}
49+
50+
void DerivationCreationAndRealisationGoal::addWantedOutputs(const OutputsSpec & outputs)
51+
{
52+
/* If we already want all outputs, there is nothing to do. */
53+
auto newWanted = wantedOutputs.union_(outputs);
54+
bool needRestart = !newWanted.isSubsetOf(wantedOutputs);
55+
wantedOutputs = newWanted;
56+
57+
if (!needRestart)
58+
return;
59+
60+
if (!optDrvPath)
61+
// haven't started steps where the outputs matter yet
62+
return;
63+
worker.makeDerivationGoal(*optDrvPath, outputs, buildMode);
64+
}
65+
66+
Goal::Co DerivationCreationAndRealisationGoal::init()
67+
{
68+
trace("outer init");
69+
70+
/* The first thing to do is to make sure that the derivation
71+
exists. If it doesn't, it may be created through a
72+
substitute. */
73+
if (auto optDrvPath = [this]() -> std::optional<StorePath> {
74+
if (buildMode != bmNormal)
75+
return std::nullopt;
76+
77+
auto drvPath = StorePath::dummy;
78+
try {
79+
drvPath = resolveDerivedPath(worker.store, *drvReq);
80+
} catch (MissingRealisation &) {
81+
return std::nullopt;
82+
}
83+
auto cond = worker.evalStore.isValidPath(drvPath) || worker.store.isValidPath(drvPath);
84+
return cond ? std::optional{drvPath} : std::nullopt;
85+
}()) {
86+
trace(
87+
fmt("already have drv '%s' for '%s', can go straight to building",
88+
worker.store.printStorePath(*optDrvPath),
89+
drvReq->to_string(worker.store)));
90+
} else {
91+
trace("need to obtain drv we want to build");
92+
addWaitee(worker.makeGoal(DerivedPath::fromSingle(*drvReq)));
93+
co_await Suspend{};
94+
}
95+
96+
trace("outer load and build derivation");
97+
98+
if (nrFailed != 0) {
99+
co_return amDone(ecFailed, Error("cannot build missing derivation '%s'", drvReq->to_string(worker.store)));
100+
}
101+
102+
StorePath drvPath = resolveDerivedPath(worker.store, *drvReq);
103+
/* Build this step! */
104+
concreteDrvGoal = worker.makeDerivationGoal(drvPath, wantedOutputs, buildMode);
105+
{
106+
auto g = upcast_goal(concreteDrvGoal);
107+
/* We will finish with it ourselves, as if we were the derivational goal. */
108+
g->preserveException = true;
109+
}
110+
optDrvPath = std::move(drvPath);
111+
addWaitee(upcast_goal(concreteDrvGoal));
112+
co_await Suspend{};
113+
114+
trace("outer build done");
115+
116+
buildResult = upcast_goal(concreteDrvGoal)
117+
->getBuildResult(DerivedPath::Built{
118+
.drvPath = drvReq,
119+
.outputs = wantedOutputs,
120+
});
121+
122+
auto g = upcast_goal(concreteDrvGoal);
123+
co_return amDone(g->exitCode, g->ex);
124+
}
125+
126+
}

‎src/libstore/build/derivation-creation-and-realisation-goal.hh‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
#pragma once
2+
3+
#include "parsed-derivations.hh"
4+
#include "user-lock.hh"
5+
#include "store-api.hh"
6+
#include "pathlocks.hh"
7+
#include "goal.hh"
8+
9+
namespace nix {
10+
11+
struct DerivationGoal;
12+
13+
/**
14+
* This goal type is essentially the serial composition (like function
15+
* composition) of a goal for getting a derivation, and then a
16+
* `DerivationGoal` using the newly-obtained derivation.
17+
*
18+
* In the (currently experimental) general inductive case of derivations
19+
* that are themselves build outputs, that first goal will be *another*
20+
* `DerivationCreationAndRealisationGoal`. In the (much more common) base-case
21+
* where the derivation has no provence and is just referred to by
22+
* (content-addressed) store path, that first goal is a
23+
* `SubstitutionGoal`.
24+
*
25+
* If we already have the derivation (e.g. if the evaluator has created
26+
* the derivation locally and then instructured the store to build it),
27+
* we can skip the first goal entirely as a small optimization.
28+
*/
29+
struct DerivationCreationAndRealisationGoal : public Goal
30+
{
31+
/**
32+
* How to obtain a store path of the derivation to build.
33+
*/
34+
ref<SingleDerivedPath> drvReq;
35+
36+
/**
37+
* The path of the derivation, once obtained.
38+
**/
39+
std::optional<StorePath> optDrvPath;
40+
41+
/**
42+
* The goal for the corresponding concrete derivation.
43+
**/
44+
std::shared_ptr<DerivationGoal> concreteDrvGoal;
45+
46+
/**
47+
* The specific outputs that we need to build.
48+
*/
49+
OutputsSpec wantedOutputs;
50+
51+
/**
52+
* The final output paths of the build.
53+
*
54+
* - For input-addressed derivations, always the precomputed paths
55+
*
56+
* - For content-addressed derivations, calcuated from whatever the
57+
* hash ends up being. (Note that fixed outputs derivations that
58+
* produce the "wrong" output still install that data under its
59+
* true content-address.)
60+
*/
61+
OutputPathMap finalOutputs;
62+
63+
BuildMode buildMode;
64+
65+
DerivationCreationAndRealisationGoal(
66+
ref<SingleDerivedPath> drvReq,
67+
const OutputsSpec & wantedOutputs,
68+
Worker & worker,
69+
BuildMode buildMode = bmNormal);
70+
virtual ~DerivationCreationAndRealisationGoal();
71+
72+
void timedOut(Error && ex) override;
73+
74+
std::string key() override;
75+
76+
/**
77+
* Add wanted outputs to an already existing derivation goal.
78+
*/
79+
void addWantedOutputs(const OutputsSpec & outputs);
80+
81+
Co init() override;
82+
83+
JobCategory jobCategory() const override
84+
{
85+
return JobCategory::Administration;
86+
};
87+
};
88+
89+
}

‎src/libstore/build/derivation-goal.cc‎

Lines changed: 7 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -137,21 +137,8 @@ Goal::Co DerivationGoal::init() {
137137
trace("init");
138138

139139
if (useDerivation) {
140-
/* The first thing to do is to make sure that the derivation
141-
exists. If it doesn't, it may be created through a
142-
substitute. */
143-
144-
if (buildMode != bmNormal || !worker.evalStore.isValidPath(drvPath)) {
145-
addWaitee(upcast_goal(worker.makePathSubstitutionGoal(drvPath)));
146-
co_await Suspend{};
147-
}
148-
149140
trace("loading derivation");
150141

151-
if (nrFailed != 0) {
152-
co_return done(BuildResult::MiscFailure, {}, Error("cannot build missing derivation '%s'", worker.store.printStorePath(drvPath)));
153-
}
154-
155142
/* `drvPath' should already be a root, but let's be on the safe
156143
side: if the user forgot to make it a root, we wouldn't want
157144
things being garbage collected while we're busy. */
@@ -1552,23 +1539,24 @@ void DerivationGoal::waiteeDone(GoalPtr waitee, ExitCode result)
15521539
if (!useDerivation || !drv) return;
15531540
auto & fullDrv = *dynamic_cast<Derivation *>(drv.get());
15541541

1555-
auto * dg = dynamic_cast<DerivationGoal *>(&*waitee);
1556-
if (!dg) return;
1542+
std::optional info = tryGetConcreteDrvGoal(waitee);
1543+
if (!info) return;
1544+
const auto & [dg, drvReq] = *info;
15571545

1558-
auto * nodeP = fullDrv.inputDrvs.findSlot(DerivedPath::Opaque { .path = dg->drvPath });
1546+
auto * nodeP = fullDrv.inputDrvs.findSlot(drvReq.get());
15591547
if (!nodeP) return;
15601548
auto & outputs = nodeP->value;
15611549

15621550
for (auto & outputName : outputs) {
1563-
auto buildResult = dg->getBuildResult(DerivedPath::Built {
1564-
.drvPath = makeConstantStorePathRef(dg->drvPath),
1551+
auto buildResult = dg.get().getBuildResult(DerivedPath::Built {
1552+
.drvPath = makeConstantStorePathRef(dg.get().drvPath),
15651553
.outputs = OutputsSpec::Names { outputName },
15661554
});
15671555
if (buildResult.success()) {
15681556
auto i = buildResult.builtOutputs.find(outputName);
15691557
if (i != buildResult.builtOutputs.end())
15701558
inputDrvOutputs.insert_or_assign(
1571-
{ dg->drvPath, outputName },
1559+
{ dg.get().drvPath, outputName },
15721560
i->second.outPath);
15731561
}
15741562
}

‎src/libstore/build/derivation-goal.hh‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,10 @@ struct InitialOutput {
5656

5757
/**
5858
* A goal for building some or all of the outputs of a derivation.
59+
*
60+
* The derivation must already be present, either in the store in a drv
61+
* or in memory. If the derivation itself needs to be gotten first, a
62+
* `DerivationCreationAndRealisationGoal` goal must be used instead.
5963
*/
6064
struct DerivationGoal : public Goal
6165
{

‎src/libstore/build/entry-points.cc‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#include "worker.hh"
22
#include "substitution-goal.hh"
33
#ifndef _WIN32 // TODO Enable building on Windows
4+
# include "derivation-creation-and-realisation-goal.hh"
45
# include "derivation-goal.hh"
56
#endif
67
#include "local-store.hh"
@@ -29,8 +30,8 @@ void Store::buildPaths(const std::vector<DerivedPath> & reqs, BuildMode buildMod
2930
}
3031
if (i->exitCode != Goal::ecSuccess) {
3132
#ifndef _WIN32 // TODO Enable building on Windows
32-
if (auto i2 = dynamic_cast<DerivationGoal *>(i.get()))
33-
failed.insert(printStorePath(i2->drvPath));
33+
if (auto i2 = dynamic_cast<DerivationCreationAndRealisationGoal *>(i.get()))
34+
failed.insert(i2->drvReq->to_string(*this));
3435
else
3536
#endif
3637
if (auto i2 = dynamic_cast<PathSubstitutionGoal *>(i.get()))

‎src/libstore/build/goal.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ Goal::Done Goal::amDone(ExitCode result, std::optional<Error> ex)
175175
exitCode = result;
176176

177177
if (ex) {
178-
if (!waiters.empty())
178+
if (!preserveException && !waiters.empty())
179179
logError(ex->info());
180180
else
181181
this->ex = std::move(*ex);

‎src/libstore/build/goal.hh‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,16 @@ enum struct JobCategory {
5050
* A substitution an arbitrary store object; it will use network resources.
5151
*/
5252
Substitution,
53+
/**
54+
* A goal that does no "real" work by itself, and just exists to depend on
55+
* other goals which *do* do real work. These goals therefore are not
56+
* limited.
57+
*
58+
* These goals cannot infinitely create themselves, so there is no risk of
59+
* a "fork bomb" type situation (which would be a problem even though the
60+
* goal do no real work) either.
61+
*/
62+
Administration,
5363
};
5464

5565
struct Goal : public std::enable_shared_from_this<Goal>
@@ -373,6 +383,17 @@ public:
373383
*/
374384
BuildResult getBuildResult(const DerivedPath &) const;
375385

386+
/**
387+
* Hack to say that this goal should not log `ex`, but instead keep
388+
* it around. Set by a waitee which sees itself as the designated
389+
* continuation of this goal, responsible for reporting its
390+
* successes or failures.
391+
*
392+
* @todo this is yet another not-nice hack in the goal system that
393+
* we ought to get rid of. See #11927
394+
*/
395+
bool preserveException = false;
396+
376397
/**
377398
* Exception containing an error message, if any.
378399
*/

0 commit comments

Comments
 (0)