mirror of
https://github.com/NixOS/nix
synced 2025-06-25 14:51:16 +02:00
Rework derivation input resolution
I refactored the way that input resolution works in `DerivationGoal`. To be honest, it is probably unclear to the reader whether this new way is better or worse. I suppose *intrinsic* motivation, I can say that - the more structured use of `inputGoal` (a local variable) is better than the shotgrun approach with `inputDrvOutputs` - A virtual `waiteeDone` was a hack, and now it's gone. However, the *real* motivation of this is not the above things, but that it is needed for my mammoth refactor fixing #11897 and #11928. It is nice that this step could come first, rather than making that refactor even bigger.
This commit is contained in:
John Ericson
parent
8fdb50761d
commit
a58e0584f5
6 changed files with 91 additions and 130 deletions
|
|
@ -322,6 +322,18 @@ Goal::Co DerivationGoal::haveDerivation()
|
|||
}
|
||||
|
||||
|
||||
/**
|
||||
* Used for `inputGoals` local variable below
|
||||
*/
|
||||
struct value_comparison
|
||||
{
|
||||
template <typename T>
|
||||
bool operator()(const ref<T> & lhs, const ref<T> & rhs) const {
|
||||
return *lhs < *rhs;
|
||||
}
|
||||
};
|
||||
|
||||
|
||||
/* At least one of the output paths could not be
|
||||
produced using a substitute. So we have to build instead. */
|
||||
Goal::Co DerivationGoal::gaveUpOnSubstitution()
|
||||
|
|
@ -330,19 +342,22 @@ Goal::Co DerivationGoal::gaveUpOnSubstitution()
|
|||
is no need to restart. */
|
||||
needRestart = NeedRestartForMoreOutputs::BuildInProgressWillNotNeed;
|
||||
|
||||
/* The inputs must be built before we can build this goal. */
|
||||
inputDrvOutputs.clear();
|
||||
std::map<ref<const SingleDerivedPath>, GoalPtr, value_comparison> inputGoals;
|
||||
|
||||
if (useDerivation) {
|
||||
std::function<void(ref<const SingleDerivedPath>, const DerivedPathMap<StringSet>::ChildNode &)> addWaiteeDerivedPath;
|
||||
|
||||
addWaiteeDerivedPath = [&](ref<const SingleDerivedPath> inputDrv, const DerivedPathMap<StringSet>::ChildNode & inputNode) {
|
||||
if (!inputNode.value.empty())
|
||||
addWaitee(worker.makeGoal(
|
||||
if (!inputNode.value.empty()) {
|
||||
auto g = worker.makeGoal(
|
||||
DerivedPath::Built {
|
||||
.drvPath = inputDrv,
|
||||
.outputs = inputNode.value,
|
||||
},
|
||||
buildMode == bmRepair ? bmRepair : bmNormal));
|
||||
buildMode == bmRepair ? bmRepair : bmNormal);
|
||||
inputGoals.insert_or_assign(inputDrv, g);
|
||||
addWaitee(std::move(g));
|
||||
}
|
||||
for (const auto & [outputName, childNode] : inputNode.childMap)
|
||||
addWaiteeDerivedPath(
|
||||
make_ref<SingleDerivedPath>(SingleDerivedPath::Built { inputDrv, outputName }),
|
||||
|
|
@ -430,7 +445,12 @@ Goal::Co DerivationGoal::gaveUpOnSubstitution()
|
|||
[&](const DerivationType::Impure &) {
|
||||
return true;
|
||||
}
|
||||
}, drvType.raw);
|
||||
}, drvType.raw)
|
||||
/* no inputs are outputs of dynamic derivations */
|
||||
|| std::ranges::any_of(
|
||||
fullDrv.inputDrvs.map.begin(),
|
||||
fullDrv.inputDrvs.map.end(),
|
||||
[](auto & pair) { return !pair.second.childMap.empty(); });
|
||||
|
||||
if (resolveDrv && !fullDrv.inputDrvs.map.empty()) {
|
||||
experimentalFeatureSettings.require(Xp::CaDerivations);
|
||||
|
|
@ -438,7 +458,19 @@ Goal::Co DerivationGoal::gaveUpOnSubstitution()
|
|||
/* We are be able to resolve this derivation based on the
|
||||
now-known results of dependencies. If so, we become a
|
||||
stub goal aliasing that resolved derivation goal. */
|
||||
std::optional attempt = fullDrv.tryResolve(worker.store, inputDrvOutputs);
|
||||
std::optional attempt = fullDrv.tryResolve(worker.store,
|
||||
[&](ref<const SingleDerivedPath> drvPath, const std::string & outputName) -> std::optional<StorePath> {
|
||||
auto mEntry = get(inputGoals, drvPath);
|
||||
if (!mEntry) return std::nullopt;
|
||||
|
||||
auto buildResult = (*mEntry)->getBuildResult(DerivedPath::Built{drvPath, OutputsSpec::Names{outputName}});
|
||||
if (!buildResult.success()) return std::nullopt;
|
||||
|
||||
auto i = get(buildResult.builtOutputs, outputName);
|
||||
if (!i) return std::nullopt;
|
||||
|
||||
return i->outPath;
|
||||
});
|
||||
if (!attempt) {
|
||||
/* TODO (impure derivations-induced tech debt) (see below):
|
||||
The above attempt should have found it, but because we manage
|
||||
|
|
@ -469,28 +501,14 @@ Goal::Co DerivationGoal::gaveUpOnSubstitution()
|
|||
co_return resolvedFinished();
|
||||
}
|
||||
|
||||
std::function<void(const StorePath &, const DerivedPathMap<StringSet>::ChildNode &)> accumInputPaths;
|
||||
/* If we get this far, we know no dynamic drvs inputs */
|
||||
|
||||
accumInputPaths = [&](const StorePath & depDrvPath, const DerivedPathMap<StringSet>::ChildNode & 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. */
|
||||
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.
|
||||
For a robust nix, we need to move towards the `else` branch,
|
||||
which does not rely on goal state to match up with the
|
||||
reality of the store, which is our real source of truth.
|
||||
However, the impure derivations feature still relies on this
|
||||
fragile way of doing things, because its builds do not have
|
||||
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, outputName })) {
|
||||
return *outPath;
|
||||
}
|
||||
else {
|
||||
for (auto & [depDrvPath, depNode] : fullDrv.inputDrvs.map) {
|
||||
for (auto & outputName : depNode.value) {
|
||||
/* Don't need to worry about `inputGoals`, because
|
||||
impure derivations are always resolved above. Can
|
||||
just use DB. This case only happens in the (older)
|
||||
input addressed and fixed output derivation cases. */
|
||||
auto outMap = [&]{
|
||||
for (auto * drvStore : { &worker.evalStore, &worker.store })
|
||||
if (drvStore->isValidPath(depDrvPath))
|
||||
|
|
@ -504,19 +522,10 @@ Goal::Co DerivationGoal::gaveUpOnSubstitution()
|
|||
"derivation '%s' requires non-existent output '%s' from input derivation '%s'",
|
||||
worker.store.printStorePath(drvPath), outputName, worker.store.printStorePath(depDrvPath));
|
||||
}
|
||||
return outMapPath->second;
|
||||
|
||||
worker.store.computeFSClosure(outMapPath->second, inputPaths);
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
for (auto & outputName : inputNode.value)
|
||||
worker.store.computeFSClosure(getOutput(outputName), inputPaths);
|
||||
|
||||
for (auto & [outputName, childNode] : inputNode.childMap)
|
||||
accumInputPaths(getOutput(outputName), childNode);
|
||||
};
|
||||
|
||||
for (auto & [depDrvPath, depNode] : fullDrv.inputDrvs.map)
|
||||
accumInputPaths(depDrvPath, depNode);
|
||||
}
|
||||
|
||||
/* Second, the input sources. */
|
||||
|
|
@ -1545,34 +1554,4 @@ Goal::Done DerivationGoal::done(
|
|||
return amDone(buildResult.success() ? ecSuccess : ecFailed, std::move(ex));
|
||||
}
|
||||
|
||||
|
||||
void DerivationGoal::waiteeDone(GoalPtr waitee, ExitCode result)
|
||||
{
|
||||
Goal::waiteeDone(waitee, result);
|
||||
|
||||
if (!useDerivation || !drv) return;
|
||||
auto & fullDrv = *dynamic_cast<Derivation *>(drv.get());
|
||||
|
||||
auto * dg = dynamic_cast<DerivationGoal *>(&*waitee);
|
||||
if (!dg) return;
|
||||
|
||||
auto * nodeP = fullDrv.inputDrvs.findSlot(DerivedPath::Opaque { .path = dg->drvPath });
|
||||
if (!nodeP) return;
|
||||
auto & outputs = nodeP->value;
|
||||
|
||||
for (auto & outputName : outputs) {
|
||||
auto buildResult = dg->getBuildResult(DerivedPath::Built {
|
||||
.drvPath = makeConstantStorePathRef(dg->drvPath),
|
||||
.outputs = OutputsSpec::Names { outputName },
|
||||
});
|
||||
if (buildResult.success()) {
|
||||
auto i = buildResult.builtOutputs.find(outputName);
|
||||
if (i != buildResult.builtOutputs.end())
|
||||
inputDrvOutputs.insert_or_assign(
|
||||
{ dg->drvPath, outputName },
|
||||
i->second.outPath);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
|||
|
|
@ -78,13 +78,6 @@ struct DerivationGoal : public Goal
|
|||
*/
|
||||
OutputsSpec wantedOutputs;
|
||||
|
||||
/**
|
||||
* Mapping from input derivations + output names to actual store
|
||||
* paths. This is filled in by waiteeDone() as each dependency
|
||||
* finishes, before `trace("all inputs realised")` is reached.
|
||||
*/
|
||||
std::map<std::pair<StorePath, std::string>, StorePath> inputDrvOutputs;
|
||||
|
||||
/**
|
||||
* See `needRestart`; just for that field.
|
||||
*/
|
||||
|
|
@ -331,8 +324,6 @@ struct DerivationGoal : public Goal
|
|||
SingleDrvOutputs builtOutputs = {},
|
||||
std::optional<Error> ex = {});
|
||||
|
||||
void waiteeDone(GoalPtr waitee, ExitCode result) override;
|
||||
|
||||
StorePathSet exportReferences(const StorePathSet & storePaths);
|
||||
|
||||
JobCategory jobCategory() const override {
|
||||
|
|
|
|||
|
|
@ -396,7 +396,7 @@ public:
|
|||
|
||||
void addWaitee(GoalPtr waitee);
|
||||
|
||||
virtual void waiteeDone(GoalPtr waitee, ExitCode result);
|
||||
void waiteeDone(GoalPtr waitee, ExitCode result);
|
||||
|
||||
virtual void handleChildOutput(Descriptor fd, std::string_view data)
|
||||
{
|
||||
|
|
|
|||
|
|
@ -552,4 +552,9 @@ GoalPtr upcast_goal(std::shared_ptr<DrvOutputSubstitutionGoal> subGoal)
|
|||
return subGoal;
|
||||
}
|
||||
|
||||
GoalPtr upcast_goal(std::shared_ptr<DerivationGoal> subGoal)
|
||||
{
|
||||
return subGoal;
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1052,49 +1052,36 @@ static void rewriteDerivation(Store & store, BasicDerivation & drv, const String
|
|||
|
||||
std::optional<BasicDerivation> Derivation::tryResolve(Store & store, Store * evalStore) const
|
||||
{
|
||||
std::map<std::pair<StorePath, std::string>, StorePath> inputDrvOutputs;
|
||||
|
||||
std::function<void(const StorePath &, const DerivedPathMap<StringSet>::ChildNode &)> accum;
|
||||
accum = [&](auto & inputDrv, auto & node) {
|
||||
for (auto & [outputName, outputPath] : store.queryPartialDerivationOutputMap(inputDrv, evalStore)) {
|
||||
if (outputPath) {
|
||||
inputDrvOutputs.insert_or_assign({inputDrv, outputName}, *outputPath);
|
||||
if (auto p = get(node.childMap, outputName))
|
||||
accum(*outputPath, *p);
|
||||
return tryResolve(
|
||||
store,
|
||||
[&](ref<const SingleDerivedPath> drvPath, const std::string & outputName) -> std::optional<StorePath> {
|
||||
try {
|
||||
return resolveDerivedPath(store, SingleDerivedPath::Built{drvPath, outputName}, evalStore);
|
||||
} catch (Error &) {
|
||||
return std::nullopt;
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
for (auto & [inputDrv, node] : inputDrvs.map)
|
||||
accum(inputDrv, node);
|
||||
|
||||
return tryResolve(store, inputDrvOutputs);
|
||||
});
|
||||
}
|
||||
|
||||
static bool tryResolveInput(
|
||||
Store & store, StorePathSet & inputSrcs, StringMap & inputRewrites,
|
||||
const DownstreamPlaceholder * placeholderOpt,
|
||||
const StorePath & inputDrv, const DerivedPathMap<StringSet>::ChildNode & inputNode,
|
||||
const std::map<std::pair<StorePath, std::string>, StorePath> & inputDrvOutputs)
|
||||
ref<const SingleDerivedPath> drvPath, const DerivedPathMap<StringSet>::ChildNode & inputNode,
|
||||
std::function<std::optional<StorePath>(ref<const SingleDerivedPath> drvPath, const std::string & outputName)> queryResolutionChain)
|
||||
{
|
||||
auto getOutput = [&](const std::string & outputName) {
|
||||
auto * actualPathOpt = get(inputDrvOutputs, { inputDrv, outputName });
|
||||
if (!actualPathOpt)
|
||||
warn("output %s of input %s missing, aborting the resolving",
|
||||
outputName,
|
||||
store.printStorePath(inputDrv)
|
||||
);
|
||||
return actualPathOpt;
|
||||
};
|
||||
|
||||
auto getPlaceholder = [&](const std::string & outputName) {
|
||||
return placeholderOpt
|
||||
? DownstreamPlaceholder::unknownDerivation(*placeholderOpt, outputName)
|
||||
: DownstreamPlaceholder::unknownCaOutput(inputDrv, outputName);
|
||||
: [&]{
|
||||
auto * p = std::get_if<SingleDerivedPath::Opaque>(&drvPath->raw());
|
||||
// otherwise we should have had a placeholder to build-upon already
|
||||
assert(p);
|
||||
return DownstreamPlaceholder::unknownCaOutput(p->path, outputName);
|
||||
}();
|
||||
};
|
||||
|
||||
for (auto & outputName : inputNode.value) {
|
||||
auto actualPathOpt = getOutput(outputName);
|
||||
auto actualPathOpt = queryResolutionChain(drvPath, outputName);
|
||||
if (!actualPathOpt) return false;
|
||||
auto actualPath = *actualPathOpt;
|
||||
if (experimentalFeatureSettings.isEnabled(Xp::CaDerivations)) {
|
||||
|
|
@ -1106,13 +1093,12 @@ static bool tryResolveInput(
|
|||
}
|
||||
|
||||
for (auto & [outputName, childNode] : inputNode.childMap) {
|
||||
auto actualPathOpt = getOutput(outputName);
|
||||
if (!actualPathOpt) return false;
|
||||
auto actualPath = *actualPathOpt;
|
||||
auto nextPlaceholder = getPlaceholder(outputName);
|
||||
if (!tryResolveInput(store, inputSrcs, inputRewrites,
|
||||
&nextPlaceholder, actualPath, childNode,
|
||||
inputDrvOutputs))
|
||||
&nextPlaceholder,
|
||||
make_ref<const SingleDerivedPath>(SingleDerivedPath::Built{drvPath, outputName}),
|
||||
childNode,
|
||||
queryResolutionChain))
|
||||
return false;
|
||||
}
|
||||
return true;
|
||||
|
|
@ -1120,7 +1106,7 @@ static bool tryResolveInput(
|
|||
|
||||
std::optional<BasicDerivation> Derivation::tryResolve(
|
||||
Store & store,
|
||||
const std::map<std::pair<StorePath, std::string>, StorePath> & inputDrvOutputs) const
|
||||
std::function<std::optional<StorePath>(ref<const SingleDerivedPath> drvPath, const std::string & outputName)> queryResolutionChain) const
|
||||
{
|
||||
BasicDerivation resolved { *this };
|
||||
|
||||
|
|
@ -1129,7 +1115,7 @@ std::optional<BasicDerivation> Derivation::tryResolve(
|
|||
|
||||
for (auto & [inputDrv, inputNode] : inputDrvs.map)
|
||||
if (!tryResolveInput(store, resolved.inputSrcs, inputRewrites,
|
||||
nullptr, inputDrv, inputNode, inputDrvOutputs))
|
||||
nullptr, make_ref<const SingleDerivedPath>(SingleDerivedPath::Opaque{inputDrv}), inputNode, queryResolutionChain))
|
||||
return std::nullopt;
|
||||
|
||||
rewriteDerivation(store, resolved, inputRewrites);
|
||||
|
|
|
|||
|
|
@ -369,7 +369,7 @@ struct Derivation : BasicDerivation
|
|||
*/
|
||||
std::optional<BasicDerivation> tryResolve(
|
||||
Store & store,
|
||||
const std::map<std::pair<StorePath, std::string>, StorePath> & inputDrvOutputs) const;
|
||||
std::function<std::optional<StorePath>(ref<const SingleDerivedPath> drvPath, const std::string & outputName)> queryResolutionChain) const;
|
||||
|
||||
/**
|
||||
* Check that the derivation is valid and does not present any
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue