lib/types: add record (simplified submodule)

[MattSturgeon]

Description of changes

Work in progress draft, based on #257511 by @roberth, but without types.fix and with additional support for optional fields.

Pushing an early draft, I'll polish up the changes and this description shortly.

The main TODO for me is to add additional test-cases to cover more exotic useage. I've added a few inline TODO comments regarding things I'm unsure of or plan to investigate.

More TODO from https://github.com/NixOS/nixpkgs/pull/257511/files#r1621120811:

• required fields can be returned without looking at their declarations, making it a bit more lazy, which is good for performance and robustness
• if we don't have optional or wildcard fields, we can return all the fields without even looking at the definitions
• it avoids having to partition the required and optional fields at runtime, which we'd then have to do; it's more efficient this way

---

• Fixes config-driven submodules? #158598
• Closes lib/modules: add definedOptionsOnly option to evalModules #333799
• Based on [exploratory] Module system types.record, types.fix #257511
• See also RFC42-style options that default to unset, to have no attribute in config despite option #158594

cc @roberth @infinisil @GaetanLepage @traxys

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.

[aanderse]

can we nest records? something like this?

settings = record {
  fields {
    bar = int
  }
  optionalFields {
    foo = record {
      fields {
        a = int
        b = ...
      }
    }
  }
}

[MattSturgeon]

can we nest records?

Yes. Currently nesting records (or submodules) within a record is the only way to achieve nested options.

So you can't do (for example):

settings = record {
  fields {
    foo.bar = mkOption {};
  };
}

But you can do:

settings = record {
  fields {
    foo = mkOption {
      type = record {
        fields = {
          bar = mkOption {};
        };
      };
    };
  };
}

This limitation was a design decision by @roberth in the original PR #257511, for performance reasons. I suppose it could be re-evaluated in the future though.

something like this?

Almost. All fields are options, not types.

[aanderse]

thanks for the answer

i think something like this is an important but missing feature currently
i will stay subscribed to this PR and look forward to any updates if you get around to it

All fields are options, not types.

of course, i was just simplifying

[aanderse]

@MattSturgeon any updates on this? i find myself less motivated to move forward in a few areas because the solution this PR offers to RFC42 provides a much better way than what currently exists

@roberth and @infinisil - is it agreed that this PR is the proper way to address optional fields, or are either of you still not entirely convinced and reviewing other solutions?

[roberth]

I'm in favor. Besides my earlier reasons, we've seen that taggedSubmodule is rather hard to implement in a complete way, whereas a solution based on record would be simpler, and sufficient for its purposes.

Probably it can even reuse record in whole - not duplicating its implementation. Its methods can easily retrieve the type value because definitions for record have no module arguments, fix-point, or unnecessary syntax, and when it knows the type value, it can get the right record type, merge a (raw) type field into that, and defer merging to this combined type. (That was probably hard to follow because everything is called "type" - I swear I'm not talking gibberish)

Anyway, I'd like to hear @infinisil's thoughts too.

[roberth]

Thoughts and possible suggestions (if they're good; this is all very new)

[roberth]

Suggested change

# TODO: assert that at least one arg is provided, i.e. args != { }

Probably not worth checking. record { fields = { }; } isn't better than record { }.
Both are basically enum [ { } ], and that's fine.

[roberth]

Maybe I was wrong to use mkOption.
It means that we have these extra checkFields checks to cover known problems, but it also couples record and module, holding back both types.
It seems that all of the reuse we do get is from option types, not options themselves, or we wouldn't be peeling out type and default ourselves, below.
And we don't even want defaults in the optional fields.
A mkField would be simpler, more efficient, and independent of modules. It seems that we only need type and default, but internal/visible and meta could be added.

[MattSturgeon]

I agree a mkField would make sense to further de-couple records from modules. This would also allow fields to declare whether they are optional, rather than having a separate optionalFields argument.

[roberth]

Suggested change

# FIXME: isn't this handled by `mergeDefinitions` already?

No, because we aren't passing the whole option.

What about the default's priority though? This looks like it gets added as a regular definition; not a mkOptionDefault that gets replaced entirely by a regular definition.

[roberth]

Suggested change

	# TODO: should we prevent fields from having a wildcard?
	# TODO: should we prevent `fields` from having a `_wildcard`?

Seems easy and cheap enough, especially if we assert it here in the nestedTypes value, which I believe is only accessed when doing type merging, which is somewhat rare. (I'd have to check)

[roberth]

This is a weird one. It could work like the type emptyValue, and work like lazyAttrsOf here - a trade-off between omitting the attribute entirely, as one may expect, or avoiding infinite recursions, as one may expect.

attrsOf behavior: omit attribute when value is mkIf false. Infinite recursion for defs like settings = mkIf cfg.settings.foo { bar = "bar"; }

lazyAttrsOf behavior: keep the attribute even if mkIf false, but insert an empty value, or throw.

[MattSturgeon]

It sounds like we need a lazy and non-lazy mode for record types, since its users may wish for either behaviour.

In nixvim, for instance, we probably want the non-lazy behaviour where optional fields are not present in the resulting attrs.

This is because we usually want to be able to recursively map the attrs into a lua table using our version of lib.generators.toLua.

I guess we could have our toLua generator wrap recursion in tryEval and not render anything that throws, but that seems like an overly large hammer for this issue.