Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion lib/tests/modules.sh
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,12 @@ checkConfigOutput '^"pkgs\.\\"123\\"\.\\"with\\\\\\"quote\\"\.hello"$' options.p
## specialArgs should work
checkConfigOutput '^"foo"$' config.submodule.foo ./declare-submoduleWith-special.nix

## shorthandOnlyDefines config behaves as expected
## onlyDefinesConfig behaves as expected
checkConfigOutput '^true$' config.submodule.config ./declare-submoduleWith-onlydefinesconfig.nix ./define-submoduleWith-onlydefinesconfig.nix
checkConfigOutput '^true$' config.submodule.config ./declare-submoduleWith-onlydefinesconfig.nix ./define-submoduleWith-onlydefinesconfig-path.nix
checkConfigError 'is not of type `boolean' config.submodule.config ./declare-submoduleWith-onlydefinesconfig.nix ./define-submoduleWith-noshorthand.nix

## shorthandOnlyDefinesConfig behaves as expected
checkConfigOutput '^true$' config.submodule.config ./declare-submoduleWith-shorthand.nix ./define-submoduleWith-shorthand.nix
checkConfigError 'is not of type `boolean' config.submodule.config ./declare-submoduleWith-shorthand.nix ./define-submoduleWith-noshorthand.nix
checkConfigError "In module ..*define-submoduleWith-shorthand.nix., you're trying to define a value of type \`bool'\n\s*rather than an attribute set for the option" config.submodule.config ./declare-submoduleWith-noshorthand.nix ./define-submoduleWith-shorthand.nix
Expand Down
21 changes: 21 additions & 0 deletions lib/tests/modules/declare-submoduleWith-onlydefinesconfig.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
{ lib, ... }:
let
sub = {
# An option named `config`; reason to have `onlyDefinesConfig = true;`
options.config = lib.mkOption {
type = lib.types.bool;
default = false;
};
config._module.args.bar = true;
};
in
{
options.submodule = lib.mkOption {
type = lib.types.submoduleWith {
modules = [ sub ];
specialArgs.foo = true;
onlyDefinesConfig = true;
};
default = { };
};
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{ foo, bar, ... }:
{
config = foo && bar;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
submodule = ./define-submoduleWith-onlydefinesconfig-path-file.nix;
}
7 changes: 7 additions & 0 deletions lib/tests/modules/define-submoduleWith-onlydefinesconfig.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
submodule =
{ foo, bar, ... }:
{
config = foo && bar;
};
}
33 changes: 26 additions & 7 deletions lib/types.nix
Original file line number Diff line number Diff line change
Expand Up @@ -1184,6 +1184,7 @@ let
{
modules,
specialArgs ? { },
onlyDefinesConfig ? false,
shorthandOnlyDefinesConfig ? false,
description ? null,
class ? null,
Expand All @@ -1195,11 +1196,21 @@ let
defs:
map (
{ value, file }:
if isAttrs value && shorthandOnlyDefinesConfig then
if isAttrs value && (onlyDefinesConfig || shorthandOnlyDefinesConfig) then
{
_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?

lib.setFunctionArgs (args: {
_file = file;
config = value args;
}) (lib.functionArgs value)
else if builtins.isPath value && onlyDefinesConfig then
lib.setFunctionArgs (args: {
_file = file;
config = import value args;
}) (lib.functionArgs (import value))
else
{
_file = file;
Expand Down Expand Up @@ -1273,12 +1284,10 @@ let
getSubOptions =
prefix:
let
docsEval = (
base.extendModules {
inherit prefix;
modules = [ noCheckForDocsModule ];
}
);
docsEval = base.extendModules {
inherit prefix;
modules = [ noCheckForDocsModule ];
};
# Intentionally shadow the freeformType from the possibly *checked*
# configuration. See `noCheckForDocsModule` comment.
inherit (docsEval._module) freeformType;
Expand Down Expand Up @@ -1309,6 +1318,7 @@ let
modules
class
specialArgs
onlyDefinesConfig
shorthandOnlyDefinesConfig
description
;
Expand All @@ -1334,6 +1344,15 @@ let
lhs.specialArgs // rhs.specialArgs
else
throw "A submoduleWith option is declared multiple times with the same specialArgs \"${toString (attrNames intersecting)}\"";
onlyDefinesConfig =
if lhs.onlyDefinesConfig == null then
rhs.onlyDefinesConfig
else if rhs.onlyDefinesConfig == null then
lhs.onlyDefinesConfig
else if lhs.onlyDefinesConfig == rhs.onlyDefinesConfig then
lhs.onlyDefinesConfig
else
throw "A submoduleWith option is declared multiple times with conflicting onlyDefinesConfig values";
shorthandOnlyDefinesConfig =
if lhs.shorthandOnlyDefinesConfig == null then
rhs.shorthandOnlyDefinesConfig
Expand Down
10 changes: 9 additions & 1 deletion nixos/doc/manual/development/option-types.section.md
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ Submodules are detailed in [Submodule](#section-option-types-submodule).
options. This is equivalent to
`types.submoduleWith { modules = toList o; shorthandOnlyDefinesConfig = true; }`.

`types.submoduleWith` { *`modules`*, *`specialArgs`* ? {}, *`shorthandOnlyDefinesConfig`* ? false }
`types.submoduleWith` { *`modules`*, *`specialArgs`* ? {}, *`onlyDefinesConfig`* ? false, *`shorthandOnlyDefinesConfig`* ? false }

: Like `types.submodule`, but more flexible and with better defaults.
It has parameters
Expand All @@ -274,6 +274,12 @@ Submodules are detailed in [Submodule](#section-option-types-submodule).
because `lib` itself is used to define `_module.args`, which
makes using `_module.args` to define it impossible.

- *`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.
Comment on lines +277 to +281
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.


- *`shorthandOnlyDefinesConfig`* Whether definitions of this type
should default to the `config` section of a module (see
[Example: Structure of NixOS Modules](#ex-module-syntax))
Expand All @@ -290,6 +296,8 @@ Submodules are detailed in [Submodule](#section-option-types-submodule).
With this option enabled, defining a non-`config` section
requires using a function:
`the-submodule = { ... }: { options = { ... }; }`.
Note that this behavior does *not* apply to [path values](https://nix.dev/manual/nix/latest/language/types.html#type-path),
similar to how it does not apply to [function values](https://nix.dev/manual/nix/latest/language/types.html#type-function).

`types.deferredModule`

Expand Down
Loading