Skip to content

Commit f4f28cd

Browse files
committed
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 c985252.
1 parent 7cfc52f commit f4f28cd

16 files changed

+45
-374
lines changed

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

Lines changed: 0 additions & 126 deletions
This file was deleted.

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

Lines changed: 0 additions & 88 deletions
This file was deleted.

src/libstore/build/derivation-goal.cc

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -137,8 +137,21 @@ 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+
140149
trace("loading derivation");
141150

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

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

1547-
auto * nodeP = fullDrv.inputDrvs.findSlot(drvReq.get());
1559+
auto * nodeP = fullDrv.inputDrvs.findSlot(DerivedPath::Opaque { .path = dg->drvPath });
15481560
if (!nodeP) return;
15491561
auto & outputs = nodeP->value;
15501562

15511563
for (auto & outputName : outputs) {
1552-
auto buildResult = dg.get().getBuildResult(DerivedPath::Built {
1553-
.drvPath = makeConstantStorePathRef(dg.get().drvPath),
1564+
auto buildResult = dg->getBuildResult(DerivedPath::Built {
1565+
.drvPath = makeConstantStorePathRef(dg->drvPath),
15541566
.outputs = OutputsSpec::Names { outputName },
15551567
});
15561568
if (buildResult.success()) {
15571569
auto i = buildResult.builtOutputs.find(outputName);
15581570
if (i != buildResult.builtOutputs.end())
15591571
inputDrvOutputs.insert_or_assign(
1560-
{ dg.get().drvPath, outputName },
1572+
{ dg->drvPath, outputName },
15611573
i->second.outPath);
15621574
}
15631575
}

src/libstore/build/derivation-goal.hh

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

5858
/**
5959
* A goal for building some or all of the outputs of a derivation.
60-
*
61-
* The derivation must already be present, either in the store in a drv
62-
* or in memory. If the derivation itself needs to be gotten first, a
63-
* `DerivationCreationAndRealisationGoal` goal must be used instead.
6460
*/
6561
struct DerivationGoal : public Goal
6662
{

src/libstore/build/entry-points.cc

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
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"
54
# include "derivation-goal.hh"
65
#endif
76
#include "local-store.hh"
@@ -30,8 +29,8 @@ void Store::buildPaths(const std::vector<DerivedPath> & reqs, BuildMode buildMod
3029
}
3130
if (i->exitCode != Goal::ecSuccess) {
3231
#ifndef _WIN32 // TODO Enable building on Windows
33-
if (auto i2 = dynamic_cast<DerivationCreationAndRealisationGoal *>(i.get()))
34-
failed.insert(i2->drvReq->to_string(*this));
32+
if (auto i2 = dynamic_cast<DerivationGoal *>(i.get()))
33+
failed.insert(printStorePath(i2->drvPath));
3534
else
3635
#endif
3736
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 (!preserveException && !waiters.empty())
178+
if (!waiters.empty())
179179
logError(ex->info());
180180
else
181181
this->ex = std::move(*ex);

src/libstore/build/goal.hh

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -50,16 +50,6 @@ 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,
6353
};
6454

6555
struct Goal : public std::enable_shared_from_this<Goal>
@@ -383,17 +373,6 @@ public:
383373
*/
384374
BuildResult getBuildResult(const DerivedPath &) const;
385375

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-
397376
/**
398377
* Exception containing an error message, if any.
399378
*/

0 commit comments

Comments
 (0)