lib.modules: add mkForAllItems

[rhendric]

Description of changes

For motivation, see Discourse.

For explanation, see additions to nixos/doc/manual/development/option-def.section.md

tl;dr: if you wish you could write (pseudocode)

swapDevices.*.options = mkDefault [ /* ... */ ];

you can now do that without the dreaded infinite recursion by writing

swapDevices = mkForAllItems {
  options = mkDefault [ /* ... */ ];
};

Is this stupid? Nixpkgs has gotten by this far without it, and there are some alternative patterns available:

• Defining a template function somewhere and using that on every element (disadvantage: undesirable coupling?)
• Redefining the option itself and injecting defaults into its type (disadvantage: overly verbose, not intuitive?)

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
  • sandbox = relaxed
  • sandbox = true
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
• Fits CONTRIBUTING.md.

---

Add a 👍 reaction to pull requests you find important.

[nixos-discourse]

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/how-to-change-the-default-nixos-option-in-a-list-of-attrsets-described-as/45561/11

[rhendric]

This thread preallocated for bikeshedding the name of this function.

forEach is used elsewhere in the libraries, but there it means ‘flipped map’, and this function is not a map at all (if it is given a function, the argument provided to that function is key, not value). So I went with ForAll to draw the distinction.

Values splits the difference between Elems for lists and Attrs for attrsets, but I'm not wedded to it.

[roberth]

Items might be better. Everything except an attribute is already value in the expression language.

types.nix currently has elemType in nestedTypes, which I think was a mistake because item is a word and elem is not. (Also elem is a function in Haskell, fwiw)

[rhendric]

Changed to Items.

[rhendric]

This thread preallocated for bikeshedding the choice of 1-based indexing.

I made an arbitrary choice here but AFAIK (not far) Nixpkgs is somewhat incoherent in which of these it uses by default. imap defaults to 1-indexed (but has explicit imap0 and imap1 options), elemAt defaults to 0-indexed (and has no elemAt1 alternative). Values displayed to users seem to be 1-indexed.

[roberth]

I feel ambivalent towards this addition.
While it's good to make this functionality more accessible, it introduces more complexity and a performance overhead.
It'd help to quantify that impact.
It might be possible to optimize the common case of no mkForAllValues.

[rhendric]

It'd help to quantify that impact.

Not sure how representative this'll be, but I ran this command before and after the patch:

NIX_SHOW_STATS=1 NIX_PATH=nixpkgs=$PWD:nixos-config=/etc/nixos/configuration.nix nix-instantiate '<nixpkgs/nixos>' -A system --eval

Selected results:

	Before	After	Δ
cpuTime	5.008959770202637	5.019791126251221	+0.22%
envs.bytes	191776744	192550400	+0.4%
envs.elements	9965909	10008938	+0.43%
envs.number	7003092	7029931	+0.38%
gc.totalBytes	1315467280	1318265568	+0.21%
list.bytes	19593080	19662352	+0.35%
list.concats	375631	382919	+1.9%
list.elements	2449135	2457794	+0.35%
nrFunctionCalls	6312840	6334283	+0.34%
nrThunks	10340995	10380775	+0.38%
sets.bytes	480846832	481106752	+0.054%
sets.elements	28250011	28260856	+0.038%
sets.number	1802916	1808316	+0.3%
values.bytes	362088648	363302568	+0.34%
values.number	15087027	15137607	+0.34%

[roberth]

options is potentially very confusing here. Maybe an alternative example is to change the way home directories are created for all normal users?
That also showcases the parameter, in combination with module parameters, which may also not be obvious how those would interact otherwise.

[rhendric]

I'd be worried about the home directory example because, as implemented, it uses the value of one attribute in a mkIf used to compute another. This is doable with mkForAllValues as well but there is some subtlety to it and I don't want that to distract.

Is the issue with swapDevices.*.options just the word options? Can we take swapDevices.*.discardPolicy instead, or something? Or how about services.wordpress.sites.<name>.uploadsDir, if we want to showcase the parameter? (I'm leaning toward both.)

[roberth]

For modules, it the old way with option merging is still relevant. Would be nice to mention here.

Suggested change

	it is used. The result of this function is used as described above.
	it is used. The result of this function is used as described above.
	If you are extending a submodule with new options, and you wish for its documentation to include your additions, you need a different solution: define the option containing the submodule, as if it didn't already exist, but without `description`.
	Then the module system will merge the declarations and generate documentation for the combined options.

[rhendric]

Sorry, I don't understand why this is connected to mkForAllValues. It's always the case that if you want to define a new option you need to declare it. Why is this something that should be explained in this section?

[roberth]

It'd help to quantify that impact.

Not sure how representative this'll be,

The CPU time and even the GC stats need to be taken with a grain of salt, but the other stats are good proxies for performance. (except concats, which is too specific)
IMO this improvement is worth the cost. 👍

I wonder what's the cost of the implicit { _module.args.name = last loc; } that we have in submodule, which is technically unnecessary after this, but that might be hard to test, and I don't think we should act on it anyway.

[roberth]

Found a micro-optimization. I couldn't think of something more significant, except maybe it's worth optimizing the cases where we have no definitions? Only if you're interested in that kind of thing.