Skip to content

Commit 3969fcb

Browse files
committed
Introduce DerivationCreationAndRealisationGoal, get rid of addWantedOutputs
`DerivationGoal` now only tracks a single output, and is back to tracking a plain store path `drvPath`, not a deriving path one. Its `addWantedOutputs` method is gone. These changes will allow subsequent PRs to simplify it greatly. Because the purpose of each goal is back to being immutable, we can also once again make `Goal::buildResult` a public field, and get rid of the `getBuildResult` method. This simplifies things also. `DerivationCreationAndRealisationGoal` is a cheap "trampoline" goal. It takes immutable sets of wanted outputs, and just kicks of `DerivationGoal`s for them. Since now "actual work" is done in these goals, it is not wasteful to have separate ones for separate sets of outputs, even if those outputs (and the derivations they are from) overlap. This separation of concerns will make it possible for future work on issues like #11928, and to continue the path of having more goal types, but each goal type does fewer things (issue #12628). This commit in some sense reverts f4f28cd, but that one kept around `addWantedOutputs`. I am quite sure it was having two layers of goals with `addWantedOutputs` that caused the issues --- restarting logic like `addWantedOutputs` has is very tempermental! In this version of the change, we have *zero* layers of `addWantedOutputs` --- no goal type needs it, or otherwise has a mutable objective --- and so I think this change is safe.
1 parent 37b92ea commit 3969fcb

14 files changed

+405
-332
lines changed

src/libfetchers/include/nix/fetchers/input-cache.hh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
#include "fetchers.hh"
1+
#include "nix/fetchers/fetchers.hh"
22

33
namespace nix::fetchers {
44

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

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#include "nix/store/build/derivation-building-goal.hh"
2-
#include "nix/store/build/derivation-goal.hh"
2+
#include "nix/store/build/derivation-creation-and-realisation-goal.hh"
33
#ifndef _WIN32 // TODO enable build hook on Windows
44
# include "nix/store/build/hook-instance.hh"
55
# include "nix/store/build/derivation-builder.hh"
@@ -264,7 +264,7 @@ Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution()
264264
auto mEntry = get(inputGoals, drvPath);
265265
if (!mEntry) return std::nullopt;
266266

267-
auto buildResult = (*mEntry)->getBuildResult(DerivedPath::Built{drvPath, OutputsSpec::Names{outputName}});
267+
auto & buildResult = (*mEntry)->buildResult;
268268
if (!buildResult.success()) return std::nullopt;
269269

270270
auto i = get(buildResult.builtOutputs, outputName);
@@ -295,29 +295,25 @@ Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution()
295295
});
296296

297297
// FIXME wanted outputs
298-
auto resolvedDrvGoal = worker.makeDerivationGoal(
299-
makeConstantStorePathRef(pathResolved), OutputsSpec::All{}, buildMode);
298+
auto resolvedDrvGoal = worker.makeDerivationCreationAndRealisationGoal(
299+
pathResolved, OutputsSpec::All{}, drvResolved, buildMode);
300300
{
301301
Goals waitees{resolvedDrvGoal};
302302
co_await await(std::move(waitees));
303303
}
304304

305305
trace("resolved derivation finished");
306306

307-
auto resolvedDrv = *resolvedDrvGoal->drv;
308-
auto resolvedResult = resolvedDrvGoal->getBuildResult(DerivedPath::Built{
309-
.drvPath = makeConstantStorePathRef(pathResolved),
310-
.outputs = OutputsSpec::All{},
311-
});
307+
auto resolvedResult = resolvedDrvGoal->buildResult;
312308

313309
SingleDrvOutputs builtOutputs;
314310

315311
if (resolvedResult.success()) {
316-
auto resolvedHashes = staticOutputHashes(worker.store, resolvedDrv);
312+
auto resolvedHashes = staticOutputHashes(worker.store, drvResolved);
317313

318314
StorePathSet outputPaths;
319315

320-
for (auto & outputName : resolvedDrv.outputNames()) {
316+
for (auto & outputName : drvResolved.outputNames()) {
321317
auto initialOutput = get(initialOutputs, outputName);
322318
auto resolvedHash = get(resolvedHashes, outputName);
323319
if ((!initialOutput) || (!resolvedHash))
@@ -338,7 +334,7 @@ Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution()
338334

339335
throw Error(
340336
"derivation '%s' doesn't have expected output '%s' (derivation-goal.cc/realisation)",
341-
resolvedDrvGoal->drvReq->to_string(worker.store), outputName);
337+
worker.store.printStorePath(pathResolved), outputName);
342338
}();
343339

344340
if (!drv->type().isImpure()) {
Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
1+
#include "nix/store/build/derivation-creation-and-realisation-goal.hh"
2+
#include "nix/store/build/worker.hh"
3+
#include "nix/store/derivations.hh"
4+
5+
namespace nix {
6+
7+
DerivationCreationAndRealisationGoal::DerivationCreationAndRealisationGoal(
8+
ref<const SingleDerivedPath> drvReq, const OutputsSpec & wantedOutputs, Worker & worker, BuildMode buildMode)
9+
: Goal(worker, init())
10+
, drvReq(drvReq)
11+
, wantedOutputs(wantedOutputs)
12+
, buildMode(buildMode)
13+
{
14+
commonInit();
15+
}
16+
17+
DerivationCreationAndRealisationGoal::DerivationCreationAndRealisationGoal(
18+
const StorePath & drvPath,
19+
const OutputsSpec & wantedOutputs,
20+
const Derivation & drv,
21+
Worker & worker,
22+
BuildMode buildMode)
23+
: Goal(worker, haveDerivation(drvPath, drv))
24+
, drvReq(makeConstantStorePathRef(drvPath))
25+
, wantedOutputs(wantedOutputs)
26+
, buildMode(buildMode)
27+
{
28+
commonInit();
29+
}
30+
31+
void DerivationCreationAndRealisationGoal::commonInit()
32+
{
33+
name =
34+
fmt("outer obtaining drv from '%s' and then building outputs %s",
35+
drvReq->to_string(worker.store),
36+
std::visit(
37+
overloaded{
38+
[&](const OutputsSpec::All) -> std::string { return "* (all of them)"; },
39+
[&](const OutputsSpec::Names os) { return concatStringsSep(", ", quoteStrings(os)); },
40+
},
41+
wantedOutputs.raw));
42+
trace("created outer");
43+
44+
worker.updateProgress();
45+
}
46+
47+
DerivationCreationAndRealisationGoal::~DerivationCreationAndRealisationGoal() {}
48+
49+
static StorePath pathPartOfReq(const SingleDerivedPath & req)
50+
{
51+
return std::visit(
52+
overloaded{
53+
[&](const SingleDerivedPath::Opaque & bo) { return bo.path; },
54+
[&](const SingleDerivedPath::Built & bfd) { return pathPartOfReq(*bfd.drvPath); },
55+
},
56+
req.raw());
57+
}
58+
59+
std::string DerivationCreationAndRealisationGoal::key()
60+
{
61+
/* Ensure that derivations get built in order of their name,
62+
i.e. a derivation named "aardvark" always comes before "baboon". And
63+
substitution goals and inner derivation goals always happen before
64+
derivation goals (due to "b$"). */
65+
return "c$" + std::string(pathPartOfReq(*drvReq).name()) + "$" + DerivedPath::Built{
66+
.drvPath = drvReq,
67+
.outputs = wantedOutputs,
68+
}.to_string(worker.store);
69+
}
70+
71+
void DerivationCreationAndRealisationGoal::timedOut(Error && ex) {}
72+
73+
Goal::Co DerivationCreationAndRealisationGoal::init()
74+
{
75+
trace("outer init");
76+
77+
/* The first thing to do is to make sure that the derivation
78+
exists. If it doesn't, it may be created through a
79+
substitute. */
80+
if (auto optDrvPath = [this]() -> std::optional<StorePath> {
81+
if (buildMode != bmNormal)
82+
return std::nullopt;
83+
84+
auto drvPath = StorePath::dummy;
85+
try {
86+
drvPath = resolveDerivedPath(worker.store, *drvReq);
87+
} catch (MissingRealisation &) {
88+
return std::nullopt;
89+
}
90+
auto cond = worker.evalStore.isValidPath(drvPath) || worker.store.isValidPath(drvPath);
91+
return cond ? std::optional{drvPath} : std::nullopt;
92+
}()) {
93+
trace(
94+
fmt("already have drv '%s' for '%s', can go straight to building",
95+
worker.store.printStorePath(*optDrvPath),
96+
drvReq->to_string(worker.store)));
97+
} else {
98+
trace("need to obtain drv we want to build");
99+
Goals waitees{worker.makeGoal(DerivedPath::fromSingle(*drvReq))};
100+
co_await await(std::move(waitees));
101+
}
102+
103+
trace("outer load and build derivation");
104+
105+
if (nrFailed != 0) {
106+
co_return amDone(ecFailed, Error("cannot build missing derivation '%s'", drvReq->to_string(worker.store)));
107+
}
108+
109+
StorePath drvPath = resolveDerivedPath(worker.store, *drvReq);
110+
111+
/* `drvPath' should already be a root, but let's be on the safe
112+
side: if the user forgot to make it a root, we wouldn't want
113+
things being garbage collected while we're busy. */
114+
worker.evalStore.addTempRoot(drvPath);
115+
116+
/* Get the derivation. It is probably in the eval store, but it might be inthe main store:
117+
118+
- Resolved derivation are resolved against main store realisations, and so must be stored there.
119+
120+
- Dynamic derivations are built, and so are found in the main store.
121+
*/
122+
auto drv = [&] {
123+
for (auto * drvStore : {&worker.evalStore, &worker.store})
124+
if (drvStore->isValidPath(drvPath))
125+
return drvStore->readDerivation(drvPath);
126+
assert(false);
127+
}();
128+
129+
co_return haveDerivation(std::move(drvPath), std::move(drv));
130+
}
131+
132+
Goal::Co DerivationCreationAndRealisationGoal::haveDerivation(StorePath drvPath, Derivation drv)
133+
{
134+
auto resolvedWantedOutputs = std::visit(
135+
overloaded{
136+
[&](const OutputsSpec::Names & names) -> OutputsSpec::Names { return names; },
137+
[&](const OutputsSpec::All &) -> OutputsSpec::Names {
138+
StringSet outputs;
139+
for (auto & [outputName, _] : drv.outputs)
140+
outputs.insert(outputName);
141+
return outputs;
142+
},
143+
},
144+
wantedOutputs.raw);
145+
146+
Goals concreteDrvGoals;
147+
148+
/* Build this step! */
149+
150+
for (auto & output : resolvedWantedOutputs) {
151+
auto g = upcast_goal(worker.makeDerivationGoal(drvPath, drv, output, buildMode));
152+
g->preserveException = true;
153+
/* We will finish with it ourselves, as if we were the derivational goal. */
154+
concreteDrvGoals.insert(std::move(g));
155+
}
156+
157+
optDrvPath = std::move(drvPath);
158+
159+
// Copy on purpose
160+
co_await await(Goals(concreteDrvGoals));
161+
162+
trace("outer build done");
163+
164+
auto & g = *concreteDrvGoals.begin();
165+
buildResult = g->buildResult;
166+
for (auto & g2 : concreteDrvGoals) {
167+
for (auto && [x, y] : g2->buildResult.builtOutputs)
168+
buildResult.builtOutputs.insert_or_assign(x, y);
169+
}
170+
171+
co_return amDone(g->exitCode, g->ex);
172+
}
173+
}

0 commit comments

Comments
 (0)