Skip to content

lib.types.submoduleWith: add onlyDefinesConfig parameter#437972

Open
yunfachi wants to merge 3 commits intoNixOS:masterfrom
yunfachi:patch-2
Open

lib.types.submoduleWith: add onlyDefinesConfig parameter#437972
yunfachi wants to merge 3 commits intoNixOS:masterfrom
yunfachi:patch-2

Conversation

@yunfachi
Copy link
Member

Added the onlyDefinesConfig parameter, which works like shorthandOnlyDefinesConfig but applies not only to attrset values, but also to function and path values. This is especially useful when a submodule has specialArgs and defines options.{options, config}.

Things done

  • Built on platform:
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Tested, as applicable:
  • Ran nixpkgs-review on this PR. See nixpkgs-review usage.
  • Tested basic functionality of all binary files, usually in ./result/bin/.
  • Nixpkgs Release Notes
    • Package update: when the change is major or breaking.
  • NixOS Release Notes
    • Module addition: when adding a new NixOS module.
    • Module update: when the change is significant.
  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

Add a 👍 reaction to pull requests you find important.

@yunfachi yunfachi requested review from infinisil and roberth August 28, 2025 17:14
@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: module system About "NixOS" module system internals 6.topic: lib The Nixpkgs function library 8.has: documentation This PR adds or changes documentation labels Aug 28, 2025
@nix-owners nix-owners bot requested a review from hsjobeki August 28, 2025 17:19
@yunfachi yunfachi force-pushed the patch-2 branch 2 times, most recently from f222f92 to da2a718 Compare August 29, 2025 14:46
{
_file = file;
config = value;
}
else if isFunction value && onlyDefinesConfig then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if we should do this here because we have module import logic in module.nix already.
That also handles i.e. string types and might have other divergent logic. It would be a bit suprising to get different import logic

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolutely agree. Did you mean something like this?

args: {
  _file = file;
  config = lib.applyModuleArgsIfFunction "somekey" value args;
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be a bit suprising to get different import logic

True, but I haven't made up my mind on this yet.

Maybe a different perspective: this option makes it impossible to declare any options or imports.
Perhaps we should reframe it as "definitions only" evaluation? I don't think we can make it an evalModules flag; instead evalModules would have to be made aware of two sets of modules: the ones that declare (and define) stuff, and the ones that only define.

This is a lot like types.record (as of yet WIP, #334680) and if we were to use that instead, it would also yield better performance, as it won't have to re-evaluate the options hierarchy for each instance of the submodule (i.e. when it's in an attrsOf or similar where multiple of those attributes contain the submodule, as is usually the case).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this option makes it impossible to declare any options or imports. Perhaps we should reframe it as "definitions only" evaluation?

I don't think so. The initial module can still use config to create options, right?

Comment on lines +277 to +281
- *`onlyDefinesConfig`* Whether definitions of this type should
always default to the `config` section of a module. In contrast to
`shorthandOnlyDefinesConfig`, this applies to all definition values,
including functions and paths. When a value is a function, it is invoked
with the same arguments as the module itself.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use of definitions was a bit confusing to me here, and I wasn't sure whether it referred to the "outermost" context (definitions which happen to be modules because we're talking about submodule) or the inner context, definitions in config.
I think we can reframe this as interpreting the module attributes differently. To be precise, we're interpreting differently the attributes that would take the place of the module attributes, but I think that's unnecessary precision that makes it more confusing again.
Anyway, here's what I came up with as an alternative description.

Suggested change
- *`onlyDefinesConfig`* Whether definitions of this type should
always default to the `config` section of a module. In contrast to
`shorthandOnlyDefinesConfig`, this applies to all definition values,
including functions and paths. When a value is a function, it is invoked
with the same arguments as the module itself.
- *`onlyDefinesConfig`* Whether all module attributes are interpreted
exclusively as `config` definitions.
This is similar to `shorthandOnlyDefinesConfig`, but applies to all module
attributes, including those that occur in function bodies and module files.
Module arguments are still available and behave as usual.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, the original first part was indeed incorrect, thanks! What do you think about this version?

Suggested change
- *`onlyDefinesConfig`* Whether definitions of this type should
always default to the `config` section of a module. In contrast to
`shorthandOnlyDefinesConfig`, this applies to all definition values,
including functions and paths. When a value is a function, it is invoked
with the same arguments as the module itself.
- *`onlyDefinesConfig`* Whether all module attributes are interpreted
exclusively as `config` definitions.
This is similar to `shorthandOnlyDefinesConfig`, but it applies to all
option definition values, including [paths](https://nix.dev/manual/nix/latest/language/types.html#type-path) to modules and [functions](https://nix.dev/manual/nix/latest/language/types.html#type-function)
that are invoked with module arguments.

Copy link
Member

@roberth roberth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oof, I really don't like to block things, but I really think this use case is better handled by

@yunfachi
Copy link
Member Author

yunfachi commented Sep 17, 2025

but I really think this use case is better handled by

lib.types.record is useful, but it serves a different purpose:

  1. With lib.types.submoduleWith, the initial module of the custom module system is specified, allowing config, imports, and other module attributes to be defined, not just options. Access to config._module.args is already valuable.
  2. lib.types.record definitions do not appear to support arguments. Even a name alone may not be sufficient. If argument support is out of scope, that only proves that lib.types.record and lib.types.submoduleWith { onlyDefinesConfig = true; } serve different purposes.

Both approaches are valid for different use cases. shorthandOnlyDefinesConfig exists to allow using a option named config without writing config.config = .... The motivation for introducing onlyDefinesConfig was the inability to pass module arguments to submodules when using shorthandOnlyDefinesConfig = true;, while still keeping the "onlyDefinesConfig" behavior.

@nixpkgs-ci nixpkgs-ci bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jan 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2.status: merge conflict This PR has merge conflicts with the target branch 6.topic: lib The Nixpkgs function library 6.topic: module system About "NixOS" module system internals 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: documentation This PR adds or changes documentation 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants