lib.types: init attrsWith

[hsjobeki]

Description of changes

Unify the code path between attrsOf and lazyAtrrsOf

• Set lazy = true to receive set lazy type
• Also now allow to set the "<name>" placeholder for option docs. This is particularly useful when "<name>" doesn't make sense or when dealing with nested attrsOf submodule. Which would yield "<name>.<name>"

Apart from that everything should behave the same.

I first tried to wrap the attrsOf type and only override getSubOptions with a function taking a custom prefix. But it seems i am missing something.

Context

• Closes <name> as prefix in types.attrsOf types.submodule is often unhelpful #295872

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.

[DavHau]

LGTM

[roberth]

Apart from that everything should behave the same.

Seems like a wasted opportunity actually. The type could accept a function from name to type so you really have a name, that you can even feed into the submodule specialArgs using a different name name, etc.

Anyway, I guess what I'm getting at is that the attrs types should be factored into a single more capable function, because this property is not mutually exclusive with the other property about laziness (attrsOf/lazyAttrsOf).

See also

• <name> as prefix in types.attrsOf types.submodule is often unhelpful #295872

I'll add that to the description as well, because this would close that.

[hsjobeki]

Seems like a wasted opportunity actually. The type could accept a function from name to type so you really have a name, that you can even feed into the submodule specialArgs using a different name name, etc.

So before i start factoring this, and make sure the checks pass.

Do you mean something like this?

    # Interface
    # elemTypeFn :: String -> OptionType
    namedAttrsOf = attrName: elemTypeFn: mkOptionType rec {
     # ... 
    }

 # Simple Usage with submodule taking a function itself
 # I choose username as concrete name name here.
 # Couldn't make up a nice name for `namedAttr` but `name` is probably bad, since it already used in types.submodule in a different way.
   mkOption {
      type = namedAttrsOf "username" (attrName: submoduleWith {
          specialArgs = {
             inherit attrName;
          };
          modules = [
           # ... other modules receiving the name
          ];
        }
      );
   };

[roberth]

I was thinking something along the lines of

attrsWith {
  name = "username";
  itemType = submoduleWith { modules = [ <...> ]; };
  # or, perhaps later:
  # itemTypeFunction = name: submoduleWith { modules = f name; specialArgs = g name; };
  # and perhaps later:
  # lazy = true;
};

This is more extensible and will let us cover lazyAttrsOf as well.

Also, instead of attrsWith we could do dict, because a submodule value is also an attrset but very different. (See also the issue and the defintiion of dictionary in https://nix.dev/manual/nix/2.24/development/json-guideline)

[hsjobeki]

@roberth okay. I refactored everthing accordingly. Lets see if all checks pass.
I also added some smoke eval tests.

Also when looking at the usage. It might be more consistent if we name elemType -> just type. Unsure because the parent attribute as also called type.

options = {
  foo = mkOption {
    type = attrsWith {
      elemType = submodule {
      ...

[roberth]

In summary

• This would be a good opportunity to fix showOption properly
• Some microoptimization

[roberth]

Right, we actually have two types of option paths: abstract ones containing placeholders, and real ones that are just data.
Treating them the same isn't quite correct, and merging a workaround could complicate a real fix.
Also note that some of these path items are not module options but attrsOf attributes, and "*" will occur as an attribute name in certain configurations to represent a "pattern" that matches everything; for example when an HTTP server doesn't responds for an unknown virtual host, etc.

We could solve this in at least two ways

a. Leave the "option path" type as is, but use two functions

• leave showOption as is, suitable for concrete option paths
• add showAbstractOption, which implements these new rules
  b. Only improve the representation
• represent abstract path items with a value like { _type = "optionPathPlaceholder"; metaVariable = "name"; __toString = this: "<${this.metaVariable}>"; }

I think (b) may have good backward compatibility and it solves the problem for module options; not just attrsOf keys.
__toString is mostly for compatibility with existing code that kind of works when a placeholder string is passed. This happens when generating option docs.

[roberth]

This is pre-existing tech debt, so while this is a good opportunity to fix it before making it slightly worse, perhaps this should be done in a follow-up PR instead.

[hsjobeki]

I'd prefer to do this in a follow up PR if you are okay with it. Should i remove the changes from this one?

[roberth]

thought:
Most of the tests are in lib/tests/modules{,.sh}.
We should put all module system tests in one place. (but not in this PR)

[infinisil]

This is great, nice work!

In addition to the issue I'm pointing out below, this should also have docs in https://nixos.org/manual/nixos/stable/#sec-option-types-composed. Otherwise I think this is good :)

[infinisil]

Type merging is not well behaved right now. Here's the start of a test suite for that:

(import ./lib).evalModules {
  modules = [

    {
      imports = [
        ({ lib, ... }: {
          options.mergedLazy = lib.mkOption {
            type = lib.types.attrsWith {
              lazy = true;
              elemType = lib.types.int;
            };
          };
        })

        ({ lib, ... }: {
          options.mergedLazy = lib.mkOption {
            type = lib.types.attrsWith {
              lazy = true;
              elemType = lib.types.int;
            };
          };
        })

        ({ config, ... }: {
          # Can only evaluate if lazy
          mergedLazy.bar = config.mergedLazy.baz + 1;
          mergedLazy.baz = 10;
        })
      ];
    }

    {
      imports = [
        ({ lib, ... }: {
          options.mergedName = lib.mkOption {
            type = lib.types.attrsWith {
              name = "id";
              elemType = lib.types.submodule {
                options.nested = lib.mkOption {};
              };
            };
          };
        })
        ({ lib, ... }: {
          options.mergedName = lib.mkOption {
            type = lib.types.attrsOf (lib.types.submodule {});
          };
        })
        ({ lib, options, ... }: {
          options.nameWhenMerged = lib.mkOption {
            default = (options.mergedName.type.getSubOptions options.mergedName.loc).nested.loc;
          };
        })
      ];
    }
  ];
}

Merging of lazy = true doesn't work:

$ nix-instantiate --eval test.nix -A config.mergedLazy --strict
error: infinite recursion encountered

lazy should only be merged successfully if the values of both types are the same.

Merging of name doesn't work:

$ nix-instantiate --eval test.nix -A config.nameWhenMerged --strict
[ "mergedName" "<name>" "nested" ]

This should return <id> instead. name should be merged by prioritising non-default values.

[hsjobeki]

Right. Didnt think that the merging is done by <name> literally. Thanks for the test suite, i'll try to fix the merging.

[hsjobeki]

@infinisil I am unsure how to solve the option name merging. Are we sure this is a valid module definition?

I think to fix this we need to change how modules.mergeOptionDecls works?

# Option definition 1
({ lib, ... }: {
          options.mergedName = lib.mkOption {
            type = lib.types.attrsWith {
              name = "id";
              elemType = lib.types.submodule {
                options.nested = lib.mkOption {};
              };
            };
          };
})
# Option definition 2 (same option, same type, different <name> placeholder)
({ lib, ... }: {
          options.mergedName = lib.mkOption {
            type = lib.types.attrsOf (lib.types.submodule {});
          };
})

And we might also need to make sure merging nested paths and conflicting placeholders

e.g. [<bar> <name> <name>] [<name> <foo> <baz>] [<foo> <name> <name>] -> [<?> <foo> <baz>]

So basically we need to decide which of the two functions we will take, when merging two options declarations?

getSubOptions = prefix: elemType.getSubOptions (prefix ++ ["<${name}>"]); (name = "id")
getSubOptions = prefix: elemType.getSubOptions (prefix ++ ["<${name}>"]); (name = "name")

I couldnt find a way yet. But if you have a solution i'd be very thankfull.

[infinisil]

I went ahead and pushed a commit with how I think it should be implemented, I hope you don't mind!

[hsjobeki]

Yes. Big thanks for doing the name merging.

I was confused if i was supposed to add additional attributes to the functor.

[hsjobeki]

@infinisil Just added some little documentation and fixed the missing functor attributes (wrapped, type), that got removed from my previous commit. Nixos manual should be able to build.

[infinisil]

#344216 (comment) really needs to be added as tests, because those wrapped and type attributes aren't needed and actually break it 😅. lib/tests/modules.sh would be a good entry-point for testing that.

[hsjobeki]

#344216 (comment) really needs to be added as tests, because those wrapped and type attributes aren't needed and actually break it 😅. lib/tests/modules.sh would be a good entry-point for testing that.

Is it breaking? I ran those tests that you gave me and they where all fine. I'll run the other ones lib/tests/modules.sh as well. Shouldn't those run with CI because all jobs passed as well?

EDIT: Just ran nix-build lib/tests/release.nix which seems to run the lib/tests/modules.sh in turn. Just exited with tests ok

[infinisil]

I mean that in the comment I linked, I showed you two additional test cases that should be added to modules.sh in this PR