Skip to content

Commit 6ae3c9e

Browse files
Ericson2314amaanq
Ericson2314
and
amaanq
committed
libstore: parameterize the inputs of Derivation
Working with templates adds some overhead, but parameterizing the inputs is an important prerequisite for storing the options inside the derivation. Subsequent commits can clean this up significantly. Co-authored-by: Amaan Qureshi <git@amaanq.com>
1 parent 528c084 commit 6ae3c9e

29 files changed

Lines changed: 451 additions & 336 deletions

‎src/libexpr/primops.cc‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1747,18 +1747,18 @@ static void derivationStrictInternal(EvalState & state, std::string_view drvName
17471747
StorePathSet refs;
17481748
state.store->computeFSClosure(d.drvPath, refs);
17491749
for (auto & j : refs) {
1750-
drv.inputSrcs.insert(j);
1750+
drv.inputs.srcs.insert(j);
17511751
if (j.isDerivation()) {
1752-
drv.inputDrvs.map[j].value = state.store->readDerivation(j).outputNames();
1752+
drv.inputs.drvs.map[j].value = state.store->readDerivation(j).outputNames();
17531753
}
17541754
}
17551755
},
17561756
[&](const NixStringContextElem::Built & b) {
1757-
drv.inputDrvs.ensureSlot(*b.drvPath).value.insert(b.output);
1757+
drv.inputs.drvs.ensureSlot(*b.drvPath).value.insert(b.output);
17581758
},
17591759
[&](const NixStringContextElem::Opaque & o) {
17601760
state.ensureLazyPathCopied(o.path);
1761-
drv.inputSrcs.insert(o.path);
1761+
drv.inputs.srcs.insert(o.path);
17621762
},
17631763
},
17641764
c.raw);

‎src/libstore-tests/derivation-advanced-attrs.cc‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ class DerivationAdvancedAttrsTest : public JsonCharacterizationTest<Derivation>,
3939
this->readTest(fileName, [&](auto encoded) {
4040
auto got = parseDerivation(*this->store, std::move(encoded), "foo", this->mockXpSettings);
4141
auto options = derivationOptionsFromStructuredAttrs(
42-
*this->store, got.inputDrvs, got.env, get(got.structuredAttrs), true, this->mockXpSettings);
42+
*this->store, got.inputs.drvs, got.env, get(got.structuredAttrs), true, this->mockXpSettings);
4343
EXPECT_EQ(options.getRequiredSystemFeatures(got), expectedFeatures);
4444
});
4545
}
@@ -55,7 +55,7 @@ class DerivationAdvancedAttrsTest : public JsonCharacterizationTest<Derivation>,
5555
this->readTest(fileName, [&](auto encoded) {
5656
auto got = parseDerivation(*this->store, std::move(encoded), "foo", this->mockXpSettings);
5757
auto options = derivationOptionsFromStructuredAttrs(
58-
*this->store, got.inputDrvs, got.env, get(got.structuredAttrs), true, this->mockXpSettings);
58+
*this->store, got.inputs.drvs, got.env, get(got.structuredAttrs), true, this->mockXpSettings);
5959

6060
EXPECT_EQ(options, expected);
6161
EXPECT_EQ(options.getRequiredSystemFeatures(got), expectedSystemFeatures);
@@ -184,7 +184,7 @@ TYPED_TEST(DerivationAdvancedAttrsBothTest, advancedAttributes_defaults)
184184
auto got = parseDerivation(*this->store, std::move(encoded), "foo", this->mockXpSettings);
185185

186186
auto options = derivationOptionsFromStructuredAttrs(
187-
*this->store, got.inputDrvs, got.env, get(got.structuredAttrs), true, this->mockXpSettings);
187+
*this->store, got.inputs.drvs, got.env, get(got.structuredAttrs), true, this->mockXpSettings);
188188

189189
EXPECT_TRUE(!got.structuredAttrs);
190190

@@ -228,7 +228,7 @@ TYPED_TEST(DerivationAdvancedAttrsBothTest, advancedAttributes)
228228
auto got = parseDerivation(*this->store, std::move(encoded), "foo", this->mockXpSettings);
229229

230230
auto options = derivationOptionsFromStructuredAttrs(
231-
*this->store, got.inputDrvs, got.env, get(got.structuredAttrs), true, this->mockXpSettings);
231+
*this->store, got.inputs.drvs, got.env, get(got.structuredAttrs), true, this->mockXpSettings);
232232

233233
EXPECT_TRUE(!got.structuredAttrs);
234234

@@ -324,7 +324,7 @@ TYPED_TEST(DerivationAdvancedAttrsBothTest, advancedAttributes_structuredAttrs_d
324324
auto got = parseDerivation(*this->store, std::move(encoded), "foo", this->mockXpSettings);
325325

326326
auto options = derivationOptionsFromStructuredAttrs(
327-
*this->store, got.inputDrvs, got.env, get(got.structuredAttrs), true, this->mockXpSettings);
327+
*this->store, got.inputs.drvs, got.env, get(got.structuredAttrs), true, this->mockXpSettings);
328328

329329
EXPECT_TRUE(got.structuredAttrs);
330330

@@ -373,7 +373,7 @@ TYPED_TEST(DerivationAdvancedAttrsBothTest, advancedAttributes_structuredAttrs)
373373
auto got = parseDerivation(*this->store, std::move(encoded), "foo", this->mockXpSettings);
374374

375375
auto options = derivationOptionsFromStructuredAttrs(
376-
*this->store, got.inputDrvs, got.env, get(got.structuredAttrs), true, this->mockXpSettings);
376+
*this->store, got.inputs.drvs, got.env, get(got.structuredAttrs), true, this->mockXpSettings);
377377

378378
EXPECT_TRUE(got.structuredAttrs);
379379

‎src/libstore-tests/derivation/external-formats.cc‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -188,10 +188,10 @@ MAKE_TEST_P(DerivationJsonAtermTest);
188188
INSTANTIATE_TEST_SUITE_P(DerivationJSONATerm, DerivationJsonAtermTest, ::testing::Values([]() {
189189
Derivation drv;
190190
drv.name = "simple-derivation";
191-
drv.inputSrcs = {
191+
drv.inputs.srcs = {
192192
StorePath("c015dhfh5l0lp6wxyvdn7bmwhbbr6hr9-dep1"),
193193
};
194-
drv.inputDrvs = {
194+
drv.inputs.drvs = {
195195
.map =
196196
{
197197
{
@@ -232,10 +232,10 @@ Derivation makeDynDepDerivation()
232232
{
233233
Derivation drv;
234234
drv.name = "dyn-dep-derivation";
235-
drv.inputSrcs = {
235+
drv.inputs.srcs = {
236236
StorePath{"c015dhfh5l0lp6wxyvdn7bmwhbbr6hr9-dep1"},
237237
};
238-
drv.inputDrvs = {
238+
drv.inputs.drvs = {
239239
.map =
240240
{
241241
{

‎src/libstore-tests/derivation/invariants.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ TEST_F(FillInOutputPathsTest, preservesDeferredWithInputDrvs)
216216
{"out", ""},
217217
};
218218
// Add the real input derivation dependency
219-
drv.inputDrvs = {.map = {{depDrvPath, {.value = {"out"}}}}};
219+
drv.inputs.drvs = {.map = {{depDrvPath, {.value = {"out"}}}}};
220220

221221
// Serialize before state
222222
checkpointJson("depends-on-drv-pre", drv);
@@ -252,7 +252,7 @@ TEST_F(FillInOutputPathsTest, throwsOnPatWhenShouldBeDeffered)
252252
{"out", ""},
253253
};
254254
// Add the real input derivation dependency
255-
drv.inputDrvs = {.map = {{depDrvPath, {.value = {"out"}}}}};
255+
drv.inputs.drvs = {.map = {{depDrvPath, {.value = {"out"}}}}};
256256

257257
// Serialize before state
258258
checkpointJson("bad-depends-on-drv-pre", drv);

‎src/libstore-tests/derivations.cc‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ TEST_F(TryResolveTest, withInputs)
174174
drv.platform = "x86_64-linux";
175175
drv.builder = "/bin/bash";
176176
drv.outputs = multiOutputs;
177-
drv.inputDrvs = {
177+
drv.inputs.drvs = {
178178
.map = {
179179
{dep1DrvPath, {.value = {"out", "dev"}}},
180180
{dep2DrvPath, {.value = {"out"}}},
@@ -219,7 +219,7 @@ TEST_F(TryResolveTest, withInputs)
219219
expected.platform = "x86_64-linux";
220220
expected.builder = "/bin/bash";
221221
expected.outputs = multiOutputs;
222-
expected.inputSrcs = {dep1OutPath, dep1DevPath, dep2OutPath};
222+
expected.inputs = {dep1OutPath, dep1DevPath, dep2OutPath};
223223
expected.env = {
224224
{"DEP1_OUT", "prefix-" + store->printStorePath(dep1OutPath) + "-suffix"},
225225
{"DEP1_DEV", store->printStorePath(dep1DevPath)},
@@ -243,7 +243,7 @@ TEST_F(TryResolveTest, resolutionFailure)
243243
drv.platform = "x86_64-linux";
244244
drv.builder = "/bin/bash";
245245
drv.outputs = {{"out", caFloatingOutput()}};
246-
drv.inputDrvs = {.map = {{depDrvPath, {.value = {"out"}}}}};
246+
drv.inputs.drvs = {.map = {{depDrvPath, {.value = {"out"}}}}};
247247

248248
BuildTrace buildTrace;
249249

@@ -273,7 +273,7 @@ void TryResolveTest::exportRefGraphSubpathTest(
273273

274274
nix::checkpointJson(*this, std::string{stem} + "-before", drv);
275275

276-
auto options = derivationOptionsFromStructuredAttrs(*store, drv.inputDrvs, drv.env, parsed, true);
276+
auto options = derivationOptionsFromStructuredAttrs(*store, drv.inputs.drvs, drv.env, parsed, true);
277277

278278
nix::checkpointJson(*this, std::string{stem} + "-before-options", options);
279279

@@ -333,7 +333,7 @@ TEST_F(TryResolveTest, exportReferencesGraphPlaceholderSubpath)
333333
drv.platform = "x86_64-linux";
334334
drv.builder = "/bin/bash";
335335
drv.outputs = {{"out", caFloatingOutput()}};
336-
drv.inputDrvs = {.map = {{depDrvPath, {.value = {"out"}}}}};
336+
drv.inputs.drvs = {.map = {{depDrvPath, {.value = {"out"}}}}};
337337
drv.env = {
338338
{"exportReferencesGraph", "refs " + placeholder + "/foo"},
339339
};
@@ -351,7 +351,7 @@ TEST_F(TryResolveTest, exportReferencesGraphPlaceholderSubpath_structuredAttrs)
351351
drv.platform = "x86_64-linux";
352352
drv.builder = "/bin/bash";
353353
drv.outputs = {{"out", caFloatingOutput()}};
354-
drv.inputDrvs = {.map = {{depDrvPath, {.value = {"out"}}}}};
354+
drv.inputs.drvs = {.map = {{depDrvPath, {.value = {"out"}}}}};
355355
drv.structuredAttrs = StructuredAttrs{{
356356
{"exportReferencesGraph", nlohmann::json::object({{"refs", nlohmann::json::array({placeholder + "/foo"})}})},
357357
}};

‎src/libstore-tests/outputs-query.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,8 @@ TEST_F(OutputsQueryTest, fibonacciChainQueryCount)
7272
// d_i depends on d_{i-1} and d_{i-2}
7373
for (size_t i = 2; i <= N; ++i) {
7474
Derivation drv = makeLeafDrv("d" + std::to_string(i));
75-
drv.inputDrvs.map[drvPaths[i - 1]].value.insert("out");
76-
drv.inputDrvs.map[drvPaths[i - 2]].value.insert("out");
75+
drv.inputs.drvs.map[drvPaths[i - 1]].value.insert("out");
76+
drv.inputs.drvs.map[drvPaths[i - 2]].value.insert("out");
7777
drvPaths.push_back(store->writeDerivation(drv));
7878
}
7979

‎src/libstore-tests/worker-substitution.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ TEST_F(WorkerSubstitutionTest, floatingDerivationOutputWithDepDrv)
329329
},
330330
};
331331
// Add the dependency derivation as an input
332-
rootDrv.inputDrvs = {.map = {{depDrvPath, {.value = {"out"}}}}};
332+
rootDrv.inputs.drvs = {.map = {{depDrvPath, {.value = {"out"}}}}};
333333

334334
// Write the root derivation to the destination store
335335
auto rootDrvPath = dummyStore->writeDerivation(rootDrv);
@@ -345,7 +345,7 @@ TEST_F(WorkerSubstitutionTest, floatingDerivationOutputWithDepDrv)
345345
ASSERT_TRUE(resolvedRootDrv);
346346

347347
// Write the resolved derivation to the substituter
348-
auto resolvedRootDrvPath = substituter->writeDerivation(Derivation{*resolvedRootDrv});
348+
auto resolvedRootDrvPath = substituter->writeDerivation(resolvedRootDrv->unresolve());
349349

350350
// Snapshot the destination store before
351351
checkpointJson("issue-11928/store-before", dummyStore);

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

Lines changed: 32 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -155,13 +155,13 @@ Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution(bool storeDerivation)
155155
are (resolved) derivation outputs in a resolved derivation. */
156156
if (&worker.evalStore != &worker.store) {
157157
RealisedPath::Set inputSrcs;
158-
for (auto & i : drv->inputSrcs)
158+
for (auto & i : drv->inputs.srcs)
159159
if (worker.evalStore.isValidPath(i))
160160
inputSrcs.insert(i);
161161
copyClosure(worker.evalStore, worker.store, inputSrcs);
162162
}
163163

164-
for (auto & i : drv->inputSrcs) {
164+
for (auto & i : drv->inputs.srcs) {
165165
if (worker.store.isValidPath(i))
166166
continue;
167167
if (!worker.settings.useSubstitutes)
@@ -193,7 +193,7 @@ Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution(bool storeDerivation)
193193
/* Determine the full set of input paths. */
194194

195195
if (storeDerivation) {
196-
assert(drv->inputDrvs.map.empty());
196+
assert(drv->inputs.drvs.map.empty());
197197
/* Store the resolved derivation, as part of the record of
198198
what we're actually building */
199199
worker.store.writeDerivation(*drv);
@@ -204,7 +204,7 @@ Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution(bool storeDerivation)
204204
{
205205
/* If we get this far, we know no dynamic drvs inputs */
206206

207-
for (auto & [depDrvPath, depNode] : drv->inputDrvs.map) {
207+
for (auto & [depDrvPath, depNode] : drv->inputs.drvs.map) {
208208
for (auto & outputName : depNode.value) {
209209
/* Don't need to worry about `inputGoals`, because
210210
impure derivations are always resolved above. Can
@@ -232,7 +232,7 @@ Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution(bool storeDerivation)
232232
}
233233

234234
/* Second, the input sources. */
235-
worker.store.computeFSClosure(drv->inputSrcs, inputPaths);
235+
worker.store.computeFSClosure(drv->inputs.srcs, inputPaths);
236236

237237
debug("added input paths %s", concatMapStringsSep(", ", inputPaths, [&](auto & p) {
238238
return "'" + worker.store.printStorePath(p) + "'";
@@ -336,36 +336,26 @@ static BuildError reject(const LocalBuildRejection & rejection, std::string_view
336336

337337
Goal::Co DerivationBuildingGoal::tryToBuild(StorePathSet inputPaths)
338338
{
339+
/* Project down to the `BasicDerivation` the builder consumes. */
340+
BasicDerivation resolvedDrv{
341+
.outputs = drv->outputs,
342+
.inputs = drv->inputs.srcs,
343+
.platform = drv->platform,
344+
.builder = drv->builder,
345+
.args = drv->args,
346+
.env = drv->env,
347+
.structuredAttrs = drv->structuredAttrs,
348+
.name = drv->name,
349+
};
350+
339351
auto drvOptions = [&] {
340-
DerivationOptions<SingleDerivedPath> temp;
341352
try {
342-
temp =
343-
derivationOptionsFromStructuredAttrs(worker.store, drv->inputDrvs, drv->env, get(drv->structuredAttrs));
353+
return derivationOptionsFromStructuredAttrs(
354+
worker.store, resolvedDrv.env, get(resolvedDrv.structuredAttrs));
344355
} catch (Error & e) {
345356
e.addTrace({}, "while parsing derivation '%s'", worker.store.printStorePath(drvPath));
346357
throw;
347358
}
348-
349-
auto res = tryResolve(
350-
temp,
351-
[&](ref<const SingleDerivedPath> drvPath, const std::string & outputName) -> std::optional<StorePath> {
352-
try {
353-
return resolveDerivedPath(
354-
worker.store, SingleDerivedPath::Built{drvPath, outputName}, &worker.evalStore);
355-
} catch (Error &) {
356-
return std::nullopt;
357-
}
358-
});
359-
360-
/* The derivation must have all of its inputs gotten this point,
361-
so the resolution will surely succeed.
362-
363-
(Actually, we shouldn't even enter this goal until we have a
364-
resolved derivation, or derivation with only input addressed
365-
transitive inputs, so this should be a no-opt anyways.)
366-
*/
367-
assert(res);
368-
return *res;
369359
}();
370360

371361
std::map<std::string, InitialOutput> initialOutputs;
@@ -521,7 +511,7 @@ Goal::Co DerivationBuildingGoal::tryToBuild(StorePathSet inputPaths)
521511
if (valid)
522512
co_return doneSuccess(BuildResult::Success::AlreadyValid, checkPathValidity(initialOutputs).second);
523513

524-
switch (tryBuildHook(drvOptions)) {
514+
switch (tryBuildHook(resolvedDrv, drvOptions)) {
525515
case rpAccept:
526516
/* Yes, it has started doing so. Wait until we get
527517
EOF from the hook. */
@@ -532,8 +522,6 @@ Goal::Co DerivationBuildingGoal::tryToBuild(StorePathSet inputPaths)
532522
// We should do it ourselves.
533523
co_return Return{};
534524
case rpPostpone:
535-
/* Not now; wait until at least one child finishes or
536-
the wake-up timeout expires. */
537525
break;
538526
}
539527
}
@@ -554,7 +542,7 @@ Goal::Co DerivationBuildingGoal::tryToBuild(StorePathSet inputPaths)
554542
if (valid)
555543
break;
556544

557-
switch (tryBuildHook(drvOptions)) {
545+
switch (tryBuildHook(resolvedDrv, drvOptions)) {
558546
case rpAccept:
559547
/* Yes, it has started doing so. Wait until we get
560548
EOF from the hook. */
@@ -590,7 +578,12 @@ Goal::Co DerivationBuildingGoal::tryToBuild(StorePathSet inputPaths)
590578

591579
valid = true;
592580
co_return buildLocally(
593-
*cap, std::move(inputPaths), std::move(initialOutputs), std::move(drvOptions), std::move(outputLocks));
581+
*cap,
582+
resolvedDrv,
583+
std::move(inputPaths),
584+
std::move(initialOutputs),
585+
std::move(drvOptions),
586+
std::move(outputLocks));
594587
}
595588

596589
co_return Return{};
@@ -860,6 +853,7 @@ Goal::Co DerivationBuildingGoal::buildWithHook(
860853

861854
Goal::Co DerivationBuildingGoal::buildLocally(
862855
LocalBuildCapability localBuildCap,
856+
BasicDerivation resolvedDrv,
863857
StorePathSet inputPaths,
864858
std::map<std::string, InitialOutput> initialOutputs,
865859
DerivationOptions<StorePath> drvOptions,
@@ -975,7 +969,7 @@ Goal::Co DerivationBuildingGoal::buildLocally(
975969
DerivationBuilderParams params{
976970
.drvPath = drvPath,
977971
.buildResult = buildResult,
978-
.drv = *drv,
972+
.drv = resolvedDrv,
979973
.drvOptions = drvOptions,
980974
.inputPaths = inputPaths,
981975
.initialOutputs = initialOutputs,
@@ -1187,7 +1181,8 @@ BuildError DerivationBuildingGoal::fixupBuilderFailureErrorMessage(BuilderFailur
11871181
return BuildError{e.status, msg};
11881182
}
11891183

1190-
HookReply DerivationBuildingGoal::tryBuildHook(const DerivationOptions<StorePath> & drvOptions)
1184+
HookReply DerivationBuildingGoal::tryBuildHook(
1185+
const BasicDerivation & resolvedDrv, const DerivationOptions<StorePath> & drvOptions)
11911186
{
11921187
#ifdef _WIN32 // TODO enable build hook on Windows
11931188
return rpDecline;
@@ -1205,7 +1200,7 @@ HookReply DerivationBuildingGoal::tryBuildHook(const DerivationOptions<StorePath
12051200
/* Send the request to the hook. */
12061201
worker.hook->sink << "try" << (worker.getNrLocalBuilds() < worker.settings.maxBuildJobs ? 1 : 0)
12071202
<< drv->platform << worker.store.printStorePath(drvPath)
1208-
<< drvOptions.getRequiredSystemFeatures(*drv);
1203+
<< drvOptions.getRequiredSystemFeatures(resolvedDrv);
12091204
worker.hook->sink.flush();
12101205

12111206
/* Read the first line of input, which should be a word indicating

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ Goal::Co DerivationGoal::haveDerivation(bool storeDerivation)
5252
auto drvOptions = [&]() -> DerivationOptions<SingleDerivedPath> {
5353
try {
5454
return derivationOptionsFromStructuredAttrs(
55-
worker.store, drv->inputDrvs, drv->env, get(drv->structuredAttrs));
55+
worker.store, drv->inputs.drvs, drv->env, get(drv->structuredAttrs));
5656
} catch (Error & e) {
5757
e.addTrace({}, "while parsing derivation '%s'", worker.store.printStorePath(drvPath));
5858
throw;
@@ -160,7 +160,7 @@ Goal::Co DerivationGoal::haveDerivation(bool storeDerivation)
160160

161161
auto resolvedDrvGoal = worker.makeDerivationGoal(
162162
pathResolved,
163-
make_ref<const Derivation>(drvResolved),
163+
make_ref<const Derivation>(drvResolved.unresolve()),
164164
wantedOutput,
165165
buildMode,
166166
/*storeDerivation=*/true);

0 commit comments

Comments
 (0)