getDerivations deduplicates derivations sometimes #10

New issue

Closed

opened 2024-03-08 12:09:06 +00:00 by pennae · 2 comments

pennae commented

2024-03-08 12:09:06 +00:00

Owner

we propose to move deduplication up one step and deduplicate only the structures containing derivations so we won't inf-recurse on self-referential sets (or lists, which getDerivations also picks apart). since this will only produce more derivations (in accordance with the structure of the input values) and existing tooling already has to be able to deal with duplication that was previously not eliminated we don't think this we be a problem, but actually a strict improvement.

thoughts?

(for context, this is holding up backports of the fix to the nixos/nix#9149 because it causes vm tests to fail. the alternative would be to hack the test for the new facts, but since that will drop existing items from getDerivations results it seems more dangerous to fix the test.)

our upstream fix for nixos/nix#9149 made a bug in getDerivations apparent: nixos/nix#10132. this happens because getDerivations deduplicates not on the drvPath or attrpath or something sensible like that, but on the *raw evaluator-internal attrset pointer* that describes both derivations and sets of derivations. getDerivation is also used *everywhere*, including in flake paths. we propose to move deduplication up one step and deduplicate only the *structures containing derivations* so we won't inf-recurse on self-referential sets (or lists, which getDerivations also picks apart). since this will only produce *more* derivations (in accordance with the structure of the input values) and existing tooling already has to be able to deal with duplication that was previously not eliminated we don't think this we be a problem, but actually a strict improvement. thoughts? (for context, this is holding up backports of the fix to the nixos/nix#9149 because it causes vm tests to fail. the alternative would be to hack the test for the new facts, but since that will drop existing items from getDerivations results it seems more dangerous to fix the test.)

puck commented

2024-03-08 12:34:02 +00:00

Owner

As this behaviour is likely to change when other evaluator changes are applied, my thoughts are that we should remove the deduplication, and only do the bare minimum of deduplication to avoid infinite recursion. There's already no guarantee that the same derivation may have the same pointer in all locations (e.g. when instantiating multiple nixpkgs somehow).