Skip to content

Commit

Permalink
Allow computed drvs in inputDrvs
Browse files Browse the repository at this point in the history 
We use the same nested map representation we used for goals, in order to
save space and be backwards compatible.
  • Loading branch information
@Ericson2314
Ericson2314 committed Jul 31, 2023
1 parent 0cb4543 commit 50ae527
Show file tree
Hide file tree
Showing 13 changed files with 423 additions and 148 deletions.
2 changes: 1 addition & 1 deletion src/build-remote/build-remote.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ static int main_build_remote(int argc, char * * argv)
//
// 2. Changing the `inputSrcs` set changes the associated
// output ids, which break CA derivations
if (!drv.inputDrvs.empty())
if (!drv.inputDrvs.map.empty())
drv.inputSrcs = store->parseStorePathSet(inputs);
optResult = sshStore->buildDerivation(*drvPath, (const BasicDerivation &) drv);
auto & result = *optResult;
Expand Down
12 changes: 6 additions & 6 deletions src/libexpr/primops.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1248,15 +1248,15 @@ drvName, Bindings * attrs, Value & v)
state.store->computeFSClosure(d.drvPath, refs);
for (auto & j : refs) {
drv.inputSrcs.insert(j);
if (j.isDerivation())
drv.inputDrvs[j] = state.store->readDerivation(j).outputNames();
if (j.isDerivation()) {
drv.inputDrvs.map[j] = DerivedPathMap<StringSet>::Node {
.value = state.store->readDerivation(j).outputNames(),
};
}
}
},
[&](const NixStringContextElem::Built & b) {
if (auto * p = std::get_if<DerivedPath::Opaque>(&*b.drvPath))
drv.inputDrvs[p->path].insert(b.output);
else
throw UnimplementedError("Dependencies on the outputs of dynamic derivations are not yet supported");
drv.inputDrvs.ensureSlot(*b.drvPath).value.insert(b.output);
},
[&](const NixStringContextElem::Opaque & o) {
drv.inputSrcs.insert(o.path);
Expand Down
71 changes: 48 additions & 23 deletions src/libstore/build/derivation-goal.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -351,24 +351,37 @@ void DerivationGoal::gaveUpOnSubstitution()
/* The inputs must be built before we can build this goal. */
inputDrvOutputs.clear();
if (useDerivation)
for (auto & i : dynamic_cast<Derivation *>(drv.get())->inputDrvs) {
{
std::function<void(ref<SingleDerivedPath>, const DerivedPathMap<StringSet>::Node &)> accumDerivedPath;

accumDerivedPath = [&](ref<SingleDerivedPath> inputDrv, const DerivedPathMap<StringSet>::Node & inputNode) {
if (!inputNode.value.empty())
addWaitee(worker.makeGoal(
DerivedPath::Built {
.drvPath = inputDrv,
.outputs = inputNode.value,
},
buildMode == bmRepair ? bmRepair : bmNormal));
for (const auto & [outputName, childNode] : inputNode.childMap)
accumDerivedPath(
make_ref<SingleDerivedPath>(SingleDerivedPath::Built { inputDrv, outputName }),
childNode);
};

for (const auto & [inputDrvPath, inputNode] : dynamic_cast<Derivation *>(drv.get())->inputDrvs.map) {
/* Ensure that pure, non-fixed-output derivations don't
depend on impure derivations. */
if (experimentalFeatureSettings.isEnabled(Xp::ImpureDerivations) && drv->type().isPure() && !drv->type().isFixed()) {
auto inputDrv = worker.evalStore.readDerivation(i.first);
auto inputDrv = worker.evalStore.readDerivation(inputDrvPath);
if (!inputDrv.type().isPure())
throw Error("pure derivation '%s' depends on impure derivation '%s'",
worker.store.printStorePath(drvPath),
worker.store.printStorePath(i.first));
worker.store.printStorePath(inputDrvPath));
}

addWaitee(worker.makeGoal(
DerivedPath::Built {
.drvPath = makeConstantStorePathRef(i.first),
.outputs = i.second
},
buildMode == bmRepair ? bmRepair : bmNormal));
accumDerivedPath(makeConstantStorePathRef(inputDrvPath), inputNode);
}
}

/* Copy the input sources from the eval store to the build
store. */
Expand Down Expand Up @@ -501,7 +514,7 @@ void DerivationGoal::inputsRealised()
return ia.deferred;
},
[&](const DerivationType::ContentAddressed & ca) {
return !fullDrv.inputDrvs.empty() && (
return !fullDrv.inputDrvs.map.empty() && (
ca.fixed
/* Can optionally resolve if fixed, which is good
for avoiding unnecessary rebuilds. */
Expand All @@ -515,7 +528,7 @@ void DerivationGoal::inputsRealised()
}
}, drvType.raw());

if (resolveDrv && !fullDrv.inputDrvs.empty()) {
if (resolveDrv && !fullDrv.inputDrvs.map.empty()) {
experimentalFeatureSettings.require(Xp::CaDerivations);

/* We are be able to resolve this derivation based on the
Expand Down Expand Up @@ -552,11 +565,13 @@ void DerivationGoal::inputsRealised()
return;
}

for (auto & [depDrvPath, wantedDepOutputs] : fullDrv.inputDrvs) {
std::function<void(const StorePath &, const DerivedPathMap<StringSet>::Node &)> accumDerivedPath;

accumDerivedPath = [&](const StorePath & depDrvPath, const DerivedPathMap<StringSet>::Node & inputNode) {
/* Add the relevant output closures of the input derivation
`i' as input paths. Only add the closures of output paths
that are specified as inputs. */
for (auto & j : wantedDepOutputs) {
auto getOutput = [&](const std::string & outputName) {
/* TODO (impure derivations-induced tech debt):
Tracking input derivation outputs statefully through the
goals is error prone and has led to bugs.
Expand All @@ -568,21 +583,30 @@ void DerivationGoal::inputsRealised()
a representation in the store, which is a usability problem
in itself. When implementing this logic entirely with lookups
make sure that they're cached. */
if (auto outPath = get(inputDrvOutputs, { depDrvPath, j })) {
worker.store.computeFSClosure(*outPath, inputPaths);
if (auto outPath = get(inputDrvOutputs, { depDrvPath, outputName })) {
return *outPath;
}
else {
auto outMap = worker.evalStore.queryDerivationOutputMap(depDrvPath);
auto outMapPath = outMap.find(j);
auto outMapPath = outMap.find(outputName);
if (outMapPath == outMap.end()) {
throw Error(
"derivation '%s' requires non-existent output '%s' from input derivation '%s'",
worker.store.printStorePath(drvPath), j, worker.store.printStorePath(depDrvPath));
worker.store.printStorePath(drvPath), outputName, worker.store.printStorePath(depDrvPath));
}
worker.store.computeFSClosure(outMapPath->second, inputPaths);
return outMapPath->second;
}
}
}
};

for (auto & outputName : inputNode.value)
worker.store.computeFSClosure(getOutput(outputName), inputPaths);

for (auto & [outputName, childNode] : inputNode.childMap)
accumDerivedPath(getOutput(outputName), childNode);
};

for (auto & [depDrvPath, depNode] : fullDrv.inputDrvs.map)
accumDerivedPath(depDrvPath, depNode);
}

/* Second, the input sources. */
Expand Down Expand Up @@ -1485,10 +1509,11 @@ void DerivationGoal::waiteeDone(GoalPtr waitee, ExitCode result)
// By the time we get here, the outer goals should have.
assert(dg);

auto outputs = fullDrv.inputDrvs.find(dg->drvPath);
if (outputs == fullDrv.inputDrvs.end()) return;
auto & outputs = fullDrv.inputDrvs.ensureSlot(*odg->drvReq).value;
// FIXME: need a `findSlot`
//if (outputs == fullDrv.inputDrvs.end()) return;

for (auto & outputName : outputs->second) {
for (auto & outputName : outputs) {
auto buildResult = dg->getBuildResult(DerivedPath::Built {
.drvPath = makeConstantStorePathRef(dg->drvPath),
.outputs = OutputsSpec::Names { outputName },
Expand Down
Loading

0 comments on commit 50ae527

Please sign in to comment.