NixOS / nixpkgs

Nix Packages collection & NixOS
MIT License
18k stars 14.01k forks source link

Always use `lazyAttrsOf`? Rename `attrsOf` to `strictAttrsOf`? #295419

Open roberth opened 7 months ago

roberth commented 7 months ago

Describe the bug

attrsOf has a fatal flaw, which is that it needs to perform a filterAttrs behind the scenes for mkIf, making it unexpectedly strict, with infinite recursions, unrelated errors and worse performance as a consequence in a significant number of cases.

I believe the rule "mkIf for submodules, optionalAttrs for attrsets" works at least as well as the status quo, but I haven't proven it, and everyone will need to be made aware of it, if Nixpkgs does switch.

Steps To Reproduce

Such definitions as the following (although adding .source on both sides makes it work, but that's not always a possibility).

environment.etc."foo" = config.environment.etc."bar"

Expected behavior

No more infinite recursions.

Additional context

Notify maintainers

@roberth @infinisil


Add a :+1: reaction to issues you find important.

roberth commented 7 months ago

Naively switching it runs into many issues, and a deprecation cycle would be disproportionally disruptive.

Perhaps a better path is to do a soft deprecation (no warnings, like attrs) and give lazyAttrsOf a nicer name, such as dictOf, indicating not just the value type, but also its role.

infinisil commented 6 months ago

I like the idea :). This then also relates to wanting types.record (https://github.com/NixOS/nixpkgs/pull/257511) for the other attribute set role.