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]

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.

[MattSturgeon]

I added lib.fields (lib/fields.nix) to host a mkField function, although maybe it'd be better to just have mkField in lib/options.nix?

[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)

[MattSturgeon]

The current nestedTypes is not returning the fields anymore, so this is no longer relevant.

Not sure on that design decision... If we do want to expose fields via nestedTypes, it could be as a nestedTypes.fields set.

[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.

[MattSturgeon]

This is still far from ready and I don't have much time to dedicate to it.
However, I've made a few refactors and I would appreciate some feedback on the current design & direction.

(ping @roberth @aanderse)

[aanderse]

i keep seeing (and writing) more nix code that would benefit from this so much - these settings type options we use in nixos (and beyond) really need something like this... and the documentation for this looks pretty good to me

cc @artemist because of how much they could benefit from this

[hsjobeki]

I guess then i'm also interested in this PR.

Came over from: #377718

I'll check if i find some time helping out.

[MattSturgeon]

If course! Any help is appreciated. Are you at NixCon?

I've let this sit long enough that I'll need to sit down and re-read it before continuing. IIRC the biggest blocker was finding inspiration to write new test cases.

If you're motivated to work on this, you're welcome to take over. Or with clear/frequent feedback I'll be more motivated to work on this.

[hsjobeki]

If course! Any help is appreciated. Are you at NixCon?

I'm around, mostly in building 4

[hsjobeki]

After looking into this for a while the biggest foreseeable todos are

• Documentation generation. We need to turn a mkField into an artifical option for optionAttrSetToDocList' or look for another docs solution. Otherwise records will not have documentation fields. Native support for fields would be nice but requires changing a lot of tooling, such as search.nixos.org and probably the database thats running behind it.

• A bit more Testing

[roberth]

• Documentation generation. We need to turn a mkField into an artifical option for optionAttrSetToDocList' or look for another docs solution

We chatted a bit about this at NixCon. We already have multiple representations for things that "are" options in one way or another. Allowing fields to be represented in the documentation-flavored option representations sounds more than ok to me. Ultimately the UX is pretty much the same, whether it was originally a field or mkOption option, so it makes sense to union them into a single documentation-oriented representation. In practice I think this means mostly transforming field documentation into option documentation, and that's ok.

[hsjobeki]

Fiy i also tried adding omitIfUndefined to mkOption. Just out of curiosity to explore the solution space. Which kind of works, but i couldn't find a way to not break lazyness of options.
I need to compare how records handle that in comparison. For example lazyAttrs

[aanderse]

thanks @hsjobeki - i really appreciate you tackling this! 🙇‍♂️

[roberth]

couldn't find a way to not break lazyness of options.

This can be a trap sometimes. Best to make sure that it's possible in plain Nix first.
For example, let's say an attrset is shaped like { fixed1 = ...; fixed2 = ...; } // optionals (or flipped; usually doesn't matter).
// requires the attrNames of both sides, so the attrNames of optionals can't have self-references.
The only self-references are allowed in the attribute values (regardless of their origin, fixed/optional attrs).

---

tl;dr I think we need to drop the optional attr, and split the required and optional fields, which can't be done for modules.

I think you're referring to option declarations. Which go a bit beyond what plain Nix operations do, except making sure the fixed attrs exist in the option value, which does help with self-references because freeformType does not support those afaik.

record can surely have a benefit when it comes to laziness, by not having to inspect each field declaration in order to see whether it has sub-fields or whether it is optional.
For instance the module system allows this, but only with check = false;:

nix-repl> with lib; (lib.evalModules { modules = [ { config._module.check = false; options.foo = mkOption { default = 1; }; options.bar = throw "nope"; } ]; }).config.foo
1

Records could be designed to also allow this, without having to opt in or compromising name checking, but for that we need the required fields and optional fields to be fed in as distinct arguments.

record {
  requiredFields = <...>;
  optionalFields = <...>;
}

That's probably the only way to assess the validity of a record's set of attribute names without having to evaluate any mkField declarations.
By having to evaluate as few as possible parts of the declaration, we preserve the most laziness.

Conversely, if we only have a fields attr, we need to be strict in the individual mkField declarations to find a .optional attribute, and we set our users up for slightly worse performance and robustness. I don't see any reasonable way in which the idea of this split could be retrofitted to modules.

[hsjobeki]

@roberth since we need to move optional itself out of mkField
Should we use mkOption instead ? (Would reduce the amount of things added, and avoid adding a new term)

But this also requires handling of readOnly and apply

Those are handled by evalOptionValue not sure if we want that. We could probably also use mergeModules (or evalOptionValue or handle everything ourselves in the merge function of the record type?

Or we could explicitly exclude the apply and readonly features? I would expect at least readOnly to remain supported?

[hsjobeki]

Short note for this PR: Add tests for recursive records.

Key difference: submodules dont have optional options. records can be fully optional.

-> This opens them up for defining (infinite) recursive types. That describe both finite and infinite values.

Usually recursive types have some kind of terminator, i.e. nullOr recursion then null acts as a terminator to stop the recursion.
{ } empty sets can currently be used as terminators (attrsOf, empty submodule); submodules with options cannot be used because their options are required to exist. With records that changes.
A practical type can be defined using only record where the absence of a key terminates recursion.
Example

treeType = record { 
  optional.child =  treeType;
  required.payload = types.str;
};
# --- example finite value ---
treeValue = {
   child = { payload = "child"; } # absence of a further child
   payload = "parent";
}
# --- example infinite value ---
treeValue = {
   child = { payload = "child"; child = treeValue; } # self reference
   payload = "parent";
}

Q: how far do we need to support these recursive types? I.e. through getSubOptions

Similar/related to: #422272

[MattSturgeon]

Q: how far do we need to support these recursive types? I.e. through getSubOptions

I think visible = "shallow" (and visible = false) handle that fine for other infinitely-recursive type structures.

It's already possible to have infinitely recursive submodule-option trees; I've done this myself a few times where I have tree structures, e.g.:
https://github.com/nix-community/nixvim/blob/ecb75f49d10fe2823b0822e4e95e53f80e426742/docs/modules/page.nix#L12-L23

Additionally, most pkgs/pkgs-lib/formats.nix types are recursive. They handle it by overriding their type description (since they don't typically have sub-options anyway).

Should we use mkOption instead ? (Would reduce the amount of things added, and avoid adding a new term)

But this also requires handling of readOnly and apply

The original implementation did that and just had some assertions that unsupported features were not used:

if value._type or null != "option" then
  throw "Record field `${lib.escapeNixIdentifier name}` must be declared with `mkOption`."
else if value?apply then
  throw "In field ${lib.escapeNixIdentifier name} records do not support options with `apply`"
else if value?readOnly then
  throw "In field ${lib.escapeNixIdentifier name} records do not support options with `readOnly`"