lib.types: Add deferredModule#163617
Conversation
e31e2b8 to
c5a2153
Compare
c5a2153 to
fcb8eef
Compare
|
cc @infinisil |
|
ping @infinisil |
lib/types.nix
Outdated
There was a problem hiding this comment.
Shouldn't this also allow paths?
There was a problem hiding this comment.
And how about also allowing lists of modules, instead of just a single one
There was a problem hiding this comment.
paths
done
lists
This should be covered by lib.mkMerge. I don't expect lists to be common enough to warrant the duplication.
There was a problem hiding this comment.
Hmm, it bothers me a bit that the result of this type is a list, but the input is a single element. This is in contrast to essentially all other types whose input and output types are generally the same.
Also, if the input type is also a list (which I think also makes more sense, since it's how imports, the modules argument for evalModules/submoduleWith works), then it might make sense to use the listOf type to implement it.
Which then brings me to my general dislike of lists, the main problem being that it's not possible to remove entries once added. I guess in this case it's somewhat fine though, because I believe disabledModules can be used to essentially remove modules from the list, though finding out how to actually refer to modules in the list is pretty hard.
There was a problem hiding this comment.
This reminds me of Html in Elm, which is also an inherently monoidal type that they're constantly putting in lists (the free monoid) for no good reason. Similar to their [Html] -> Html operation (<div> or <span>), our concat operation (x: { imports = x; }) is also leaky. It leaks through in error messages with anon--something-something strings.
I've embraced the leakiness and I've work with the rules:
- keep the conversions to a minimum (they're leaky)
- keep the interface simple
- we don't have a use case for defining lists of modules, and it's still covered by
mkMerge submoduledoesn't support lists, so why shoulddeferredModule?
- we don't have a use case for defining lists of modules, and it's still covered by
It'd be a different store if lists of modules were always considered to be valid modules. This would include the submodule argument, submodule definitions, imports items, evalModules { modules } items, etc.
my general dislike of lists
I share the dislike, as they imply ordering, which is often not necessary or desirable in a declarative system. In this case though, they are only as unnecessary and undesirable as in the rest of the modules system - not something we can expect to solve here.
disabledModules
The best solution to removals is not to include the things in the first place.
disabledModules should operate just fine on the parent module and on path-imported ones. Anonymous ones will be equally problematic as anonymous submodules.
though finding out how to actually refer to modules in the list is pretty hard.
Do you mean list indexing? That's not something I'd think of, but I guess someone could try - and eventually fail horribly.
I guess this might be reason to make it slightly harder? Although x.imports !! y isn't much of an obstacle over x !! y., especially compared to writing the function that only removes one element. I don't think we have that in lib?
I suppose I'm not entirely opposed to returning a single module. Given my argumentation, do you still think I should do it? (or something else)
There was a problem hiding this comment.
I suppose I'm not entirely opposed to returning a single module. Given my argumentation, do you still think I should do it? (or something else)
I haven't considered that, but I really do like that idea! By returning a single module we avoid lists in the type sense while keeping the input and output types the same, that's a 👍 👍 from me! It also aligns with the type name of types.deferredModule instead of types.deferredModules
|
Nice change :D |
|
I've found an additional use case (flake.parts) where I need "static" modules with option docs support, similar to I've noticed that we can't preserve the location info of such modules yet, just like the modules passed to |
2d59a20 to
23df9a1
Compare
infinisil
left a comment
There was a problem hiding this comment.
Other than two nits this looks good. I tested this with https://github.com/infinisil/dream2nix-modules, a demo for using the module system for dream2nix created with feedback from @DavHau at Zurihac. Turns out deferredModule is just what was needed there
lib/modules.nix
Outdated
There was a problem hiding this comment.
I think this check should be done earlier, ideally at the start of this function, since m is always required to be an attribute set, not just for this specific case here.
4d94100 to
49f374c
Compare
Nice! I've already used it in flake.parts |
`m` must always be an attrset at this point. It is basically always evaluated. This will make it throw when any of the attrs is accessed, rather than just `config`. We assume that this will improve the error message in more scenarios.
49f374c to
d9dccae
Compare
|
Thanks @infinisil! |
|
@roberth do you have any usage examples? This looks useful but the only examples I could find were two mentions in a test case that didn't reveal much. |
|
@aakropotkin You can look at the test case in this PR |
|
flake-parts |
Description of changes
Great for submodule defaults. Example:
network.defaultsin NixOps.Accepts modules, returns a list of modules, with a good default module location.
I'll quote the docs below. Also look at the example.
types.deferredModuleWhereas
submodulerepresents an option tree,deferredModulerepresents a module value, such as a module file or a configuration.It can be set multiple times.
Module authors can use its value, which is always a list of module values, in
importsor insubmoduleWith'smodulesparameter.Note that
importsmust be evaluated before the module fixpoint. Because of this, deferred modules can only be imported into "other" fixpoints, such as submodules.One use case for this type is the type of a "default" module that allow the user to affect all submodules in an
attrsOf submoduleat once. This is more convenient and discoverable than expecting the module user to type-merge with theattrsOf submoduleoption.Refs NixOS/nixops#1508
Things done
sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)nixos/doc/manual/md-to-db.shto update generated release notes