Skip to content

Commit 3b617e4

Browse files
Ericson2314Ericson2314
committed
Split DerivationGoal in two
This separation of concerns is generally good, but in particular sets up for removing `addWantedOutputs` next.
1 parent d129544 commit 3b617e4

10 files changed

Lines changed: 155 additions & 1587 deletions

File tree

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

Lines changed: 50 additions & 362 deletions
Large diffs are not rendered by default.

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

Lines changed: 39 additions & 1065 deletions
Large diffs are not rendered by default.

‎src/libstore/build/goal.cc‎

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

157157
if (ex) {
158-
if (!waiters.empty())
158+
if (!preserveException && !waiters.empty())
159159
logError(ex->info());
160160
else
161161
this->ex = std::move(*ex);

‎src/libstore/build/worker.cc‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#include "nix/store/build/substitution-goal.hh"
55
#include "nix/store/build/drv-output-substitution-goal.hh"
66
#include "nix/store/build/derivation-goal.hh"
7+
#include "nix/store/build/derivation-building-goal.hh"
78
#ifndef _WIN32 // TODO Enable building on Windows
89
# include "nix/store/build/hook-instance.hh"
910
#endif
@@ -87,6 +88,20 @@ std::shared_ptr<DerivationGoal> Worker::makeBasicDerivationGoal(const StorePath
8788
}
8889

8990

91+
std::shared_ptr<DerivationBuildingGoal> Worker::makeDerivationBuildingGoal(const StorePath & drvPath,
92+
const Derivation & drv, BuildMode buildMode)
93+
{
94+
std::weak_ptr<DerivationBuildingGoal> & goal_weak = derivationBuildingGoals[drvPath];
95+
auto goal = goal_weak.lock(); // FIXME
96+
if (!goal) {
97+
goal = std::make_shared<DerivationBuildingGoal>(drvPath, drv, *this, buildMode);
98+
goal_weak = goal;
99+
wakeUp(goal);
100+
}
101+
return goal;
102+
}
103+
104+
90105
std::shared_ptr<PathSubstitutionGoal> Worker::makePathSubstitutionGoal(const StorePath & path, RepairFlag repair, std::optional<ContentAddress> ca)
91106
{
92107
return initGoalIfNeeded(substitutionGoals[path], path, *this, repair, ca);
@@ -134,8 +149,9 @@ void Worker::removeGoal(GoalPtr goal)
134149
{
135150
if (auto drvGoal = std::dynamic_pointer_cast<DerivationGoal>(goal))
136151
nix::removeGoal(drvGoal, derivationGoals);
137-
else
138-
if (auto subGoal = std::dynamic_pointer_cast<PathSubstitutionGoal>(goal))
152+
else if (auto drvBuildingGoal = std::dynamic_pointer_cast<DerivationBuildingGoal>(goal))
153+
nix::removeGoal(drvBuildingGoal, derivationBuildingGoals);
154+
else if (auto subGoal = std::dynamic_pointer_cast<PathSubstitutionGoal>(goal))
139155
nix::removeGoal(subGoal, substitutionGoals);
140156
else if (auto subGoal = std::dynamic_pointer_cast<DrvOutputSubstitutionGoal>(goal))
141157
nix::removeGoal(subGoal, drvOutputSubstitutionGoals);
@@ -198,6 +214,9 @@ void Worker::childStarted(GoalPtr goal, const std::set<MuxablePipePollState::Com
198214
case JobCategory::Build:
199215
nrLocalBuilds++;
200216
break;
217+
case JobCategory::Administration:
218+
/* Intentionally not limited, see docs */
219+
break;
201220
default:
202221
unreachable();
203222
}
@@ -221,6 +240,9 @@ void Worker::childTerminated(Goal * goal, bool wakeSleepers)
221240
assert(nrLocalBuilds > 0);
222241
nrLocalBuilds--;
223242
break;
243+
case JobCategory::Administration:
244+
/* Intentionally not limited, see docs */
245+
break;
224246
default:
225247
unreachable();
226248
}

‎src/libstore/include/nix/store/build/derivation-building-goal.hh‎

Lines changed: 5 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -31,43 +31,11 @@ void runPostBuildHook(
3131
/**
3232
* A goal for building some or all of the outputs of a derivation.
3333
*/
34-
struct DerivationGoal : public Goal
34+
struct DerivationBuildingGoal : public Goal
3535
{
3636
/** The path of the derivation. */
3737
StorePath drvPath;
3838

39-
/**
40-
* The specific outputs that we need to build.
41-
*/
42-
OutputsSpec wantedOutputs;
43-
44-
/**
45-
* See `needRestart`; just for that field.
46-
*/
47-
enum struct NeedRestartForMoreOutputs {
48-
/**
49-
* The goal state machine is progressing based on the current value of
50-
* `wantedOutputs. No actions are needed.
51-
*/
52-
OutputsUnmodifedDontNeed,
53-
/**
54-
* `wantedOutputs` has been extended, but the state machine is
55-
* proceeding according to its old value, so we need to restart.
56-
*/
57-
OutputsAddedDoNeed,
58-
/**
59-
* The goal state machine has progressed to the point of doing a build,
60-
* in which case all outputs will be produced, so extensions to
61-
* `wantedOutputs` no longer require a restart.
62-
*/
63-
BuildInProgressWillNotNeed,
64-
};
65-
66-
/**
67-
* Whether additional wanted outputs have been added.
68-
*/
69-
NeedRestartForMoreOutputs needRestart = NeedRestartForMoreOutputs::OutputsUnmodifedDontNeed;
70-
7139
/**
7240
* The derivation stored at drvPath.
7341
*/
@@ -125,7 +93,7 @@ struct DerivationGoal : public Goal
12593

12694
BuildMode buildMode;
12795

128-
std::unique_ptr<MaintainCount<uint64_t>> mcExpectedBuilds, mcRunningBuilds;
96+
std::unique_ptr<MaintainCount<uint64_t>> mcRunningBuilds;
12997

13098
std::unique_ptr<Activity> act;
13199

@@ -141,28 +109,18 @@ struct DerivationGoal : public Goal
141109
*/
142110
std::string machineName;
143111

144-
DerivationGoal(const StorePath & drvPath,
145-
const OutputsSpec & wantedOutputs, Worker & worker,
112+
DerivationBuildingGoal(const StorePath & drvPath, const Derivation & drv,
113+
Worker & worker,
146114
BuildMode buildMode = bmNormal);
147-
DerivationGoal(const StorePath & drvPath, const BasicDerivation & drv,
148-
const OutputsSpec & wantedOutputs, Worker & worker,
149-
BuildMode buildMode = bmNormal);
150-
~DerivationGoal();
115+
~DerivationBuildingGoal();
151116

152117
void timedOut(Error && ex) override;
153118

154119
std::string key() override;
155120

156-
/**
157-
* Add wanted outputs to an already existing derivation goal.
158-
*/
159-
void addWantedOutputs(const OutputsSpec & outputs);
160-
161121
/**
162122
* The states.
163123
*/
164-
Co loadDerivation();
165-
Co haveDerivation();
166124
Co gaveUpOnSubstitution();
167125
Co tryToBuild();
168126
Co hookDone();
@@ -197,7 +155,6 @@ struct DerivationGoal : public Goal
197155
* there also is no DB entry.
198156
*/
199157
std::map<std::string, std::optional<StorePath>> queryPartialDerivationOutputMap();
200-
OutputPathMap queryDerivationOutputMap();
201158

202159
/**
203160
* Update 'initialOutputs' to determine the current status of the
@@ -218,8 +175,6 @@ struct DerivationGoal : public Goal
218175
*/
219176
void killChild();
220177

221-
Co repairClosure();
222-
223178
void started();
224179

225180
Done done(

‎src/libstore/include/nix/store/build/derivation-goal.hh‎

Lines changed: 4 additions & 107 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,6 @@ namespace nix {
1414

1515
using std::map;
1616

17-
#ifndef _WIN32 // TODO enable build hook on Windows
18-
struct HookInstance;
19-
struct DerivationBuilder;
20-
#endif
21-
22-
typedef enum {rpAccept, rpDecline, rpPostpone} HookReply;
23-
2417
/** Used internally */
2518
void runPostBuildHook(
2619
Store & store,
@@ -73,83 +66,25 @@ struct DerivationGoal : public Goal
7366
*/
7467
std::unique_ptr<Derivation> drv;
7568

76-
std::unique_ptr<StructuredAttrs> parsedDrv;
77-
std::unique_ptr<DerivationOptions> drvOptions;
78-
7969
/**
8070
* The remainder is state held during the build.
8171
*/
8272

83-
/**
84-
* Locks on (fixed) output paths.
85-
*/
86-
PathLocks outputLocks;
87-
88-
/**
89-
* All input paths (that is, the union of FS closures of the
90-
* immediate input paths).
91-
*/
92-
StorePathSet inputPaths;
93-
9473
std::map<std::string, InitialOutput> initialOutputs;
9574

96-
/**
97-
* File descriptor for the log file.
98-
*/
99-
AutoCloseFD fdLogFile;
100-
std::shared_ptr<BufferedSink> logFileSink, logSink;
101-
102-
/**
103-
* Number of bytes received from the builder's stdout/stderr.
104-
*/
105-
unsigned long logSize;
106-
107-
/**
108-
* The most recent log lines.
109-
*/
110-
std::list<std::string> logTail;
111-
112-
std::string currentLogLine;
113-
size_t currentLogLinePos = 0; // to handle carriage return
114-
115-
std::string currentHookLine;
116-
117-
#ifndef _WIN32 // TODO enable build hook on Windows
118-
/**
119-
* The build hook.
120-
*/
121-
std::unique_ptr<HookInstance> hook;
122-
123-
std::unique_ptr<DerivationBuilder> builder;
124-
#endif
125-
12675
BuildMode buildMode;
12776

128-
std::unique_ptr<MaintainCount<uint64_t>> mcExpectedBuilds, mcRunningBuilds;
129-
130-
std::unique_ptr<Activity> act;
131-
132-
/**
133-
* Activity that denotes waiting for a lock.
134-
*/
135-
std::unique_ptr<Activity> actLock;
136-
137-
std::map<ActivityId, Activity> builderActivities;
138-
139-
/**
140-
* The remote machine on which we're building.
141-
*/
142-
std::string machineName;
77+
std::unique_ptr<MaintainCount<uint64_t>> mcExpectedBuilds;
14378

14479
DerivationGoal(const StorePath & drvPath,
14580
const OutputsSpec & wantedOutputs, Worker & worker,
14681
BuildMode buildMode = bmNormal);
14782
DerivationGoal(const StorePath & drvPath, const BasicDerivation & drv,
14883
const OutputsSpec & wantedOutputs, Worker & worker,
14984
BuildMode buildMode = bmNormal);
150-
~DerivationGoal();
85+
~DerivationGoal() = default;
15186

152-
void timedOut(Error && ex) override;
87+
void timedOut(Error && ex) override { unreachable(); };
15388

15489
std::string key() override;
15590

@@ -163,33 +98,6 @@ struct DerivationGoal : public Goal
16398
*/
16499
Co loadDerivation();
165100
Co haveDerivation();
166-
Co gaveUpOnSubstitution();
167-
Co tryToBuild();
168-
Co hookDone();
169-
170-
/**
171-
* Is the build hook willing to perform the build?
172-
*/
173-
HookReply tryBuildHook();
174-
175-
/**
176-
* Open a log file and a pipe to it.
177-
*/
178-
Path openLogFile();
179-
180-
/**
181-
* Close the log file.
182-
*/
183-
void closeLogFile();
184-
185-
bool isReadDesc(Descriptor fd);
186-
187-
/**
188-
* Callback used by the worker to write to the log.
189-
*/
190-
void handleChildOutput(Descriptor fd, std::string_view data) override;
191-
void handleEOF(Descriptor fd) override;
192-
void flushLine();
193101

194102
/**
195103
* Wrappers around the corresponding Store methods that first consult the
@@ -213,26 +121,15 @@ struct DerivationGoal : public Goal
213121
*/
214122
SingleDrvOutputs assertPathValidity();
215123

216-
/**
217-
* Forcibly kill the child process, if any.
218-
*/
219-
void killChild();
220-
221124
Co repairClosure();
222125

223-
void started();
224-
225126
Done done(
226127
BuildResult::Status status,
227128
SingleDrvOutputs builtOutputs = {},
228129
std::optional<Error> ex = {});
229130

230-
void appendLogTailErrorMsg(std::string & msg);
231-
232-
StorePathSet exportReferences(const StorePathSet & storePaths);
233-
234131
JobCategory jobCategory() const override {
235-
return JobCategory::Build;
132+
return JobCategory::Administration;
236133
};
237134
};
238135

‎src/libstore/include/nix/store/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>
@@ -360,6 +370,17 @@ public:
360370
*/
361371
BuildResult getBuildResult(const DerivedPath &) const;
362372

373+
/**
374+
* Hack to say that this goal should not log `ex`, but instead keep
375+
* it around. Set by a waitee which sees itself as the designated
376+
* continuation of this goal, responsible for reporting its
377+
* successes or failures.
378+
*
379+
* @todo this is yet another not-nice hack in the goal system that
380+
* we ought to get rid of. See #11927
381+
*/
382+
bool preserveException = false;
383+
363384
/**
364385
* Exception containing an error message, if any.
365386
*/

0 commit comments

Comments
 (0)