kloenk/lix
0
0
Fork 0
forked from lix-project/lix
Code Pull requests Activity

remove getDerivations deduplication

Browse source 
deduplication does not currently work fully, showing derivations
multiple times if they have different underlying values. this can happen
by selecting the same derivation twice for two different attributes of a
set, using inherit-from (which reduces to the previous), importing
nixpkgs twice, or any other number of things.

since users already have to deal with duplicates for this reason it
won't hurt to add *more* duplicates. the alternative would be to
deduplicate fully, which would drop derivations that are currently
returned and those pose a regression risk.

Change-Id: I64b397351237e10375d270f1bddecb71f62aa131
This commit is contained in:
eldritch horrors 2024-03-08 13:56:43 +01:00
parent 2a84123631
commit 71e0114708
2 changed files with 16 additions and 14 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
flake.nix
View file

@ -552,7 +552,7 @@
type -p nix-env
# Note: we're filtering out nixos-install-tools because https://github.com/NixOS/nixpkgs/pull/153594#issuecomment-1020530593.
time nix-env --store dummy:// -f ${nixpkgs-regression} -qaP --drv-path | sort | grep -v nixos-install-tools > packages
[[ $(sha1sum < packages | cut -c1-40) = ff451c521e61e4fe72bdbe2d0ca5d1809affa733 ]]
[[ $(sha1sum < packages | cut -c1-40) = 402242fca90874112b34718b8199d844e8b03d12 ]]
mkdir $out
'';

28
src/libexpr/get-drvs.cc
View file

@ -296,22 +296,16 @@ void DrvInfo::setMeta(const std::string & name, Value * v)
typedef std::set<Bindings *> Done;
/* Evaluate value `v'. If it evaluates to a set of type `derivation',
then put information about it in `drvs' (unless it's already in `done').
The result boolean indicates whether it makes sense
/* The result boolean indicates whether it makes sense
for the caller to recursively search for derivations in `v'. */
static bool getDerivation(EvalState & state, Value & v,
const std::string & attrPath, DrvInfos & drvs, Done & done,
const std::string & attrPath, DrvInfos & drvs,
bool ignoreAssertionFailures)
{
try {
state.forceValue(v, v.determinePos(noPos));
if (!state.isDerivation(v)) return true;
/* Remove spurious duplicates (e.g., a set like `rec { x =
derivation {...}; y = x;}'. */
if (!done.insert(v.attrs).second) return false;
DrvInfo drv(state, attrPath, v.attrs);
drv.queryName();
@ -330,9 +324,8 @@ static bool getDerivation(EvalState & state, Value & v,
std::optional<DrvInfo> getDerivation(EvalState & state, Value & v,
bool ignoreAssertionFailures)
{
Done done;
DrvInfos drvs;
getDerivation(state, v, "", drvs, done, ignoreAssertionFailures);
getDerivation(state, v, "", drvs, ignoreAssertionFailures);
if (drvs.size() != 1) return {};
return std::move(drvs.front());
}
@ -347,6 +340,9 @@ static std::string addToPath(const std::string & s1, const std::string & s2)
static std::regex attrRegex("[A-Za-z_][A-Za-z0-9-_+]*");
/* Evaluate value `v'. If it evaluates to a set of type `derivation',
then put information about it in `drvs'. If it evaluates to a different
kind of set recurse (unless it's already in `done'). */
static void getDerivations(EvalState & state, Value & vIn,
const std::string & pathPrefix, Bindings & autoArgs,
DrvInfos & drvs, Done & done,
@ -356,10 +352,14 @@ static void getDerivations(EvalState & state, Value & vIn,
state.autoCallFunction(autoArgs, vIn, v);
/* Process the expression. */
if (!getDerivation(state, v, pathPrefix, drvs, done, ignoreAssertionFailures)) ;
if (!getDerivation(state, v, pathPrefix, drvs, ignoreAssertionFailures)) ;
else if (v.type() == nAttrs) {
/* Dont consider sets we've already seen, e.g. y in
`rec { x.d = derivation {...}; y = x; }`. */
if (!done.insert(v.attrs).second) return;
/* !!! undocumented hackery to support combining channels in
nix-env.cc. */
bool combineChannels = v.attrs->find(state.symbols.create("_combineChannels")) != v.attrs->end();
@ -376,7 +376,7 @@ static void getDerivations(EvalState & state, Value & vIn,
std::string pathPrefix2 = addToPath(pathPrefix, state.symbols[i->name]);
if (combineChannels)
getDerivations(state, *i->value, pathPrefix2, autoArgs, drvs, done, ignoreAssertionFailures);
else if (getDerivation(state, *i->value, pathPrefix2, drvs, done, ignoreAssertionFailures)) {
else if (getDerivation(state, *i->value, pathPrefix2, drvs, ignoreAssertionFailures)) {
/* If the value of this attribute is itself a set,
should we recurse into it? => Only if it has a
`recurseForDerivations = true' attribute. */
@ -390,9 +390,11 @@ static void getDerivations(EvalState & state, Value & vIn,
}
else 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 pathPrefix2 = addToPath(pathPrefix, fmt("%d", n));
if (getDerivation(state, *elem, pathPrefix2, drvs, done, ignoreAssertionFailures))
if (getDerivation(state, *elem, pathPrefix2, drvs, ignoreAssertionFailures))
getDerivations(state, *elem, pathPrefix2, autoArgs, drvs, done, ignoreAssertionFailures);
}
}