libexpr: mild cleanup to getDerivations

Shuffled the logic around a bit so the shorter code paths are early
returns, added comments, etc.

Should be NFC.

Change-Id: Ie3ddb3d0eddd614d6f8c37bf9a4d5a50282084ea
This commit is contained in:
Qyriad 2024-07-07 17:56:14 -06:00
parent 6a30ea0cc4
commit 5ffed6d06a

View file

@ -1,6 +1,7 @@
#include "get-drvs.hh" #include "get-drvs.hh"
#include "eval-inline.hh" #include "eval-inline.hh"
#include "derivations.hh" #include "derivations.hh"
#include "eval.hh"
#include "store-api.hh" #include "store-api.hh"
#include "path-with-outputs.hh" #include "path-with-outputs.hh"
@ -418,15 +419,46 @@ static void getDerivations(EvalState & state, Value & vIn,
Value v; Value v;
state.autoCallFunction(autoArgs, vIn, v); state.autoCallFunction(autoArgs, vIn, v);
/* Process the expression. */ bool shouldRecurse = getDerivation(state, v, pathPrefix, drvs, ignoreAssertionFailures);
if (!getDerivation(state, v, pathPrefix, drvs, ignoreAssertionFailures)) ; if (!shouldRecurse) {
// We're done here.
return;
}
else if (v.type() == nAttrs) { if (v.type() == nList) {
// NOTE we can't really deduplicate here because small lists don't have stable addresses
// and can cause spurious duplicate detections due to v being on the stack.
for (auto [n, elem] : enumerate(v.listItems())) {
std::string joinedAttrPath = addToPath(pathPrefix, fmt("%d", n));
bool shouldRecurse = getDerivation(state, *elem, joinedAttrPath, drvs, ignoreAssertionFailures);
if (shouldRecurse) {
getDerivations(
state,
*elem,
joinedAttrPath,
autoArgs,
drvs,
done,
ignoreAssertionFailures
);
}
}
return;
} else if (v.type() != nAttrs) {
state.error<TypeError>(
"expression does not evaluate to a derivation (or a list or set of those)"
).debugThrow();
}
/* Dont consider sets we've already seen, e.g. y in /* Dont consider sets we've already seen, e.g. y in
`rec { x.d = derivation {...}; y = x; }`. */ `rec { x.d = derivation {...}; y = x; }`. */
if (!done.insert(v.attrs).second) return; auto const &[_, didInsert] = done.insert(v.attrs);
if (!didInsert) {
return;
}
// FIXME: what the fuck???
/* !!! undocumented hackery to support combining channels in /* !!! undocumented hackery to support combining channels in
nix-env.cc. */ nix-env.cc. */
bool combineChannels = v.attrs->find(state.symbols.create("_combineChannels")) != v.attrs->end(); bool combineChannels = v.attrs->find(state.symbols.create("_combineChannels")) != v.attrs->end();
@ -436,38 +468,45 @@ static void getDerivations(EvalState & state, Value & vIn,
there are names clashes between derivations, the derivation there are names clashes between derivations, the derivation
bound to the attribute with the "lower" name should take bound to the attribute with the "lower" name should take
precedence). */ precedence). */
for (auto & i : v.attrs->lexicographicOrder(state.symbols)) { for (auto & attr : v.attrs->lexicographicOrder(state.symbols)) {
debug("evaluating attribute '%1%'", state.symbols[i->name]); debug("evaluating attribute '%1%'", state.symbols[attr->name]);
if (!std::regex_match(std::string(state.symbols[i->name]), attrRegex)) // FIXME: only consider attrs with identifier-like names?? Why???
if (!std::regex_match(std::string(state.symbols[attr->name]), attrRegex)) {
continue; continue;
std::string pathPrefix2 = addToPath(pathPrefix, state.symbols[i->name]); }
if (combineChannels) std::string joinedAttrPath = addToPath(pathPrefix, state.symbols[attr->name]);
getDerivations(state, *i->value, pathPrefix2, autoArgs, drvs, done, ignoreAssertionFailures); if (combineChannels) {
else if (getDerivation(state, *i->value, pathPrefix2, drvs, ignoreAssertionFailures)) { getDerivations(state, *attr->value, joinedAttrPath, autoArgs, drvs, done, ignoreAssertionFailures);
} else if (getDerivation(state, *attr->value, joinedAttrPath, drvs, ignoreAssertionFailures)) {
/* If the value of this attribute is itself a set, /* If the value of this attribute is itself a set,
should we recurse into it? => Only if it has a should we recurse into it? => Only if it has a
`recurseForDerivations = true' attribute. */ `recurseForDerivations = true' attribute. */
if (i->value->type() == nAttrs) { if (attr->value->type() == nAttrs) {
Bindings::iterator j = i->value->attrs->find(state.sRecurseForDerivations); Attr * recurseForDrvs = attr->value->attrs->get(state.sRecurseForDerivations);
if (j != i->value->attrs->end() && state.forceBool(*j->value, j->pos, "while evaluating the attribute `recurseForDerivations`")) if (recurseForDrvs == nullptr) {
getDerivations(state, *i->value, pathPrefix2, autoArgs, drvs, done, ignoreAssertionFailures); continue;
}
}
} }
bool shouldRecurse = state.forceBool(
*recurseForDrvs->value,
attr->pos,
fmt("while evaluating the '%s' attribute", Magenta("recurseForDerivations"))
);
if (!shouldRecurse) {
continue;
} }
else if (v.type() == nList) { getDerivations(
// NOTE we can't really deduplicate here because small lists don't have stable addresses state,
// and can cause spurious duplicate detections due to v being on the stack. *attr->value,
for (auto [n, elem] : enumerate(v.listItems())) { joinedAttrPath,
std::string pathPrefix2 = addToPath(pathPrefix, fmt("%d", n)); autoArgs,
if (getDerivation(state, *elem, pathPrefix2, drvs, ignoreAssertionFailures)) drvs,
getDerivations(state, *elem, pathPrefix2, autoArgs, drvs, done, ignoreAssertionFailures); done,
ignoreAssertionFailures
);
}
} }
} }
else
state.error<TypeError>("expression does not evaluate to a derivation (or a set or list of those)").debugThrow();
} }