From aa613427b7ec84ddb9f07c21482dbdc6c169855d Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 5 Dec 2019 00:00:47 +0100 Subject: [PATCH 1/8] lib/modules: file -> _file for a more idempotent unifyModuleSyntax This will be useful for doing more complicated module evaluations --- lib/modules.nix | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/modules.nix b/lib/modules.nix index 44db77b5d1c..287142c77d3 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -110,7 +110,7 @@ rec { in builtins.genericClosure { startSet = toClosureList unknownModule "" modules; - operator = m: toClosureList m.file m.key m.imports; + operator = m: toClosureList m._file m.key m.imports; }; /* Massage a module into canonical form, that is, a set consisting @@ -125,7 +125,7 @@ rec { if badAttrs != {} then throw "Module `${key}' has an unsupported attribute `${head (attrNames badAttrs)}'. This is caused by assignments to the top-level attributes `config' or `options'." else - { file = m._file or file; + { _file = m._file or file; key = toString m.key or key; disabledModules = m.disabledModules or []; imports = m.imports or []; @@ -133,7 +133,7 @@ rec { config = mkMerge [ (m.config or {}) metaSet ]; } else - { file = m._file or file; + { _file = m._file or file; key = toString m.key or key; disabledModules = m.disabledModules or []; imports = m.require or [] ++ m.imports or []; @@ -189,7 +189,7 @@ rec { in the ‘value’ attribute of each option. */ mergeModules = prefix: modules: mergeModules' prefix modules - (concatMap (m: map (config: { inherit (m) file; inherit config; }) (pushDownProperties m.config)) modules); + (concatMap (m: map (config: { file = m._file; inherit config; }) (pushDownProperties m.config)) modules); mergeModules' = prefix: options: configs: let @@ -223,7 +223,7 @@ rec { ) {} modules; # an attrset 'name' => list of submodules that declare ‘name’. declsByName = byName "options" (module: option: - [{ inherit (module) file; options = option; }] + [{ inherit (module) _file; options = option; }] ) options; # an attrset 'name' => list of submodules that define ‘name’. defnsByName = byName "config" (module: value: @@ -250,7 +250,7 @@ rec { firstOption = findFirst (m: isOption m.options) "" decls; firstNonOption = findFirst (m: !isOption m.options) "" decls; in - throw "The option `${showOption loc}' in `${firstOption.file}' is a prefix of options in `${firstNonOption.file}'." + throw "The option `${showOption loc}' in `${firstOption._file}' is a prefix of options in `${firstNonOption._file}'." else mergeModules' loc decls defns )) @@ -284,7 +284,7 @@ rec { bothHave "apply" || (bothHave "type" && (! typesMergeable)) then - throw "The option `${showOption loc}' in `${opt.file}' is already declared in ${showFiles res.declarations}." + throw "The option `${showOption loc}' in `${opt._file}' is already declared in ${showFiles res.declarations}." else let /* Add the modules of the current option to the list of modules @@ -298,11 +298,11 @@ rec { else packSubmodule file { options = opt; }; getSubModules = opt.options.type.getSubModules or null; submodules = - if getSubModules != null then map (packSubmodule opt.file) getSubModules ++ res.options - else if opt.options ? options then map (coerceOption opt.file) options' ++ res.options + if getSubModules != null then map (packSubmodule opt._file) getSubModules ++ res.options + else if opt.options ? options then map (coerceOption opt._file) options' ++ res.options else res.options; in opt.options // res // - { declarations = res.declarations ++ [opt.file]; + { declarations = res.declarations ++ [opt._file]; options = submodules; } // typeSet ) { inherit loc; declarations = []; options = []; } opts; From 3cc77ce756dc3dbcbd20daa74a081fa1151e2f78 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 5 Dec 2019 05:15:17 +0100 Subject: [PATCH 2/8] lib/modules: Make unifyModuleSyntax fully idempotent Because why not --- lib/modules.nix | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/modules.nix b/lib/modules.nix index 287142c77d3..6bfc314991b 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -116,9 +116,9 @@ rec { /* Massage a module into canonical form, that is, a set consisting of ‘options’, ‘config’ and ‘imports’ attributes. */ unifyModuleSyntax = file: key: m: - let metaSet = if m ? meta - then { meta = m.meta; } - else {}; + let addMeta = config: if m ? meta + then mkMerge [ config { meta = m.meta; } ] + else config; in if m ? config || m ? options then let badAttrs = removeAttrs m ["_file" "key" "disabledModules" "imports" "options" "config" "meta"]; in @@ -130,7 +130,7 @@ rec { disabledModules = m.disabledModules or []; imports = m.imports or []; options = m.options or {}; - config = mkMerge [ (m.config or {}) metaSet ]; + config = addMeta (m.config or {}); } else { _file = m._file or file; @@ -138,7 +138,7 @@ rec { disabledModules = m.disabledModules or []; imports = m.require or [] ++ m.imports or []; options = {}; - config = mkMerge [ (removeAttrs m ["_file" "key" "disabledModules" "require" "imports"]) metaSet ]; + config = addMeta (removeAttrs m ["_file" "key" "disabledModules" "require" "imports"]); }; applyIfFunction = key: f: args@{ config, options, lib, ... }: if isFunction f then From 5002e6afbc70c6bf80efa5f997707b877cb59cdd Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Wed, 4 Dec 2019 23:41:39 +0100 Subject: [PATCH 3/8] lib/types: Add types.submoduleWith for more flexibility than types.submodule --- lib/types.nix | 62 ++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 47 insertions(+), 15 deletions(-) diff --git a/lib/types.nix b/lib/types.nix index 5e9a28ac4f0..de3c4f0d603 100644 --- a/lib/types.nix +++ b/lib/types.nix @@ -358,25 +358,41 @@ rec { }; # A submodule (like typed attribute set). See NixOS manual. - submodule = opts: + submodule = modules: submoduleWith { + shorthandOnlyDefinesConfig = true; + modules = toList modules; + }; + + submoduleWith = + { modules + , specialArgs ? {} + , shorthandOnlyDefinesConfig ? false + }@attrs: let - opts' = toList opts; inherit (lib.modules) evalModules; + + coerce = unify: value: if isFunction value + then setFunctionArgs (args: unify (value args)) (functionArgs value) + else unify (if shorthandOnlyDefinesConfig then { config = value; } else value); + + allModules = defs: modules ++ imap1 (n: { value, file }: + # Annotate the value with the location of its definition for better error messages + coerce (lib.modules.unifyModuleSyntax file "${toString file}-${toString n}") value + ) defs; + in mkOptionType rec { name = "submodule"; check = x: isAttrs x || isFunction x; merge = loc: defs: - let - coerce = def: if isFunction def then def else { config = def; }; - modules = opts' ++ map (def: { _file = def.file; imports = [(coerce def.value)]; }) defs; - in (evalModules { - inherit modules; + (evalModules { + modules = allModules defs; + inherit specialArgs; args.name = last loc; prefix = loc; }).config; getSubOptions = prefix: (evalModules - { modules = opts'; inherit prefix; + { inherit modules prefix specialArgs; # This is a work-around due to the fact that some sub-modules, # such as the one included in an attribute set, expects a "args" # attribute to be given to the sub-module. As the option @@ -394,13 +410,29 @@ rec { # It shouldn't cause an issue since this is cosmetic for the manual. args.name = "‹name›"; }).options; - getSubModules = opts'; - substSubModules = m: submodule m; - functor = (defaultFunctor name) // { - # Merging of submodules is done as part of mergeOptionDecls, as we have to annotate - # each submodule with its location. - payload = []; - binOp = lhs: rhs: []; + getSubModules = modules; + substSubModules = m: submoduleWith (attrs // { + modules = m; + }); + functor = defaultFunctor name // { + type = types.submoduleWith; + payload = { + modules = modules; + specialArgs = specialArgs; + shorthandOnlyDefinesConfig = shorthandOnlyDefinesConfig; + }; + binOp = lhs: rhs: { + modules = lhs.modules ++ rhs.modules; + specialArgs = + let intersecting = builtins.intersectAttrs lhs.specialArgs rhs.specialArgs; + in if intersecting == {} + then lhs.specialArgs // rhs.specialArgs + else throw "A submoduleWith option is declared multiple times with the same specialArgs \"${toString (attrNames intersecting)}\""; + shorthandOnlyDefinesConfig = + if lhs.shorthandOnlyDefinesConfig == rhs.shorthandOnlyDefinesConfig + then lhs.shorthandOnlyDefinesConfig + else throw "A submoduleWith option is declared multiple times with conflicting shorthandOnlyDefinesConfig values"; + }; }; }; From 5414b4018bf1161ad9bd0e98d0ffbde8aa435fe5 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 5 Dec 2019 03:29:51 +0100 Subject: [PATCH 4/8] lib/modules: Don't pack submodules specially This has the beneficial side effect of allowing paths to be used as modules in types.{submodule,submoduleWith} --- lib/default.nix | 2 +- lib/modules.nix | 26 ++++++++++---------------- 2 files changed, 11 insertions(+), 17 deletions(-) diff --git a/lib/default.nix b/lib/default.nix index 8af53152586..e31edeaaf9e 100644 --- a/lib/default.nix +++ b/lib/default.nix @@ -102,7 +102,7 @@ let commitIdFromGitRepo cleanSourceWith pathHasContext canCleanSource; inherit (modules) evalModules closeModules unifyModuleSyntax - applyIfFunction unpackSubmodule packSubmodule mergeModules + applyIfFunction mergeModules mergeModules' mergeOptionDecls evalOptionValue mergeDefinitions pushDownProperties dischargeProperties filterOverrides sortProperties fixupOptionType mkIf mkAssert mkMerge mkOverride diff --git a/lib/modules.nix b/lib/modules.nix index 6bfc314991b..48788ae933d 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -103,7 +103,7 @@ rec { toClosureList = file: parentKey: imap1 (n: x: if isAttrs x || isFunction x then let key = "${parentKey}:anon-${toString n}"; in - unifyModuleSyntax file key (unpackSubmodule (applyIfFunction key) x args) + unifyModuleSyntax file key (applyIfFunction key x args) else let file = toString x; key = toString x; in unifyModuleSyntax file key (applyIfFunction key (import x) args)); @@ -171,17 +171,6 @@ rec { else f; - /* We have to pack and unpack submodules. We cannot wrap the expected - result of the function as we would no longer be able to list the arguments - of the submodule. (see applyIfFunction) */ - unpackSubmodule = unpack: m: args: - if isType "submodule" m then - { _file = m.file; } // (unpack m.submodule args) - else unpack m args; - - packSubmodule = file: m: - { _type = "submodule"; file = file; submodule = m; }; - /* Merge a list of modules. This will recurse over the option declarations in all modules, combining them into a single set. At the same time, for each option declaration, it will merge the @@ -267,7 +256,14 @@ rec { 'opts' is a list of modules. Each module has an options attribute which correspond to the definition of 'loc' in 'opt.file'. */ - mergeOptionDecls = loc: opts: + mergeOptionDecls = + let + packSubmodule = file: m: + { _file = file; imports = [ m ]; }; + coerceOption = file: opt: + if isFunction opt then packSubmodule file opt + else packSubmodule file { options = opt; }; + in loc: opts: foldl' (res: opt: let t = res.type; t' = opt.options.type; @@ -293,9 +289,7 @@ rec { current option declaration as the file use for the submodule. If the submodule defines any filename, then we ignore the enclosing option file. */ options' = toList opt.options.options; - coerceOption = file: opt: - if isFunction opt then packSubmodule file opt - else packSubmodule file { options = opt; }; + getSubModules = opt.options.type.getSubModules or null; submodules = if getSubModules != null then map (packSubmodule opt._file) getSubModules ++ res.options From 90c82bfee7bf4eb8f9d529f3bb61ea31a004e4cb Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Sat, 14 Dec 2019 00:15:23 +0100 Subject: [PATCH 5/8] nixos/docs: Add docs for types.submoduleWith --- nixos/doc/manual/development/option-types.xml | 56 ++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-) diff --git a/nixos/doc/manual/development/option-types.xml b/nixos/doc/manual/development/option-types.xml index 8fcbb627342..1ec7e3efad7 100644 --- a/nixos/doc/manual/development/option-types.xml +++ b/nixos/doc/manual/development/option-types.xml @@ -259,12 +259,66 @@ A set of sub options o. o can be an attribute set or a function returning an attribute set. Submodules are used in composed types to - create modular options. Submodule are detailed in + create modular options. This is equivalent to + types.submoduleWith { modules = toList o; shorthandOnlyDefinesConfig = true; }. + Submodules are detailed in . + + + types.submoduleWith { + modules, + specialArgs ? {}, + shorthandOnlyDefinesConfig ? false } + + + + Like types.submodule, but more flexible and with better defaults. + It has parameters + + + modules + A list of modules to use by default for this submodule type. This gets combined + with all option definitions to build the final list of modules that will be included. + + Only options defined with this argument are included in rendered documentation. + + + + specialArgs + An attribute set of extra arguments to be passed to the module functions. + The option _module.args should be used instead + for most arguments since it allows overriding. specialArgs should only be + used for arguments that can't go through the module fixed-point, because of + infinite recursion or other problems. An example is overriding the + lib argument, because lib itself is used + to define _module.args, which makes using + _module.args to define it impossible. + + + shorthandOnlyDefinesConfig + Whether definitions of this type should default to the config + section of a module (see ) if it is an attribute + set. Enabling this only has a benefit when the submodule defines an option named + config or options. In such a case it would + allow the option to be set with the-submodule.config = "value" + instead of requiring the-submodule.config.config = "value". + This is because only when modules don't set the + config or options keys, all keys are interpreted + as option definitions in the config section. Enabling this option + implicitly puts all attributes in the config section. + + + With this option enabled, defining a non-config section requires + using a function: the-submodule = { ... }: { options = { ... }; }. + + + + + From bc42515736154efd577baafa592734ff1bc198bc Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 5 Dec 2019 06:49:23 +0100 Subject: [PATCH 6/8] nixos/syncthing: Fix submodule name usage Module arguments should be taken from the arguments directly. This allows evalModule's specialArgs to override them if necessary --- nixos/modules/services/networking/syncthing.nix | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/nixos/modules/services/networking/syncthing.nix b/nixos/modules/services/networking/syncthing.nix index b3f2af5b179..47b10e408c0 100644 --- a/nixos/modules/services/networking/syncthing.nix +++ b/nixos/modules/services/networking/syncthing.nix @@ -112,12 +112,12 @@ in { addresses = [ "tcp://192.168.0.10:51820" ]; }; }; - type = types.attrsOf (types.submodule ({ config, ... }: { + type = types.attrsOf (types.submodule ({ name, ... }: { options = { name = mkOption { type = types.str; - default = config._module.args.name; + default = name; description = '' Name of the device ''; @@ -175,7 +175,7 @@ in { devices = [ "bigbox" ]; }; }; - type = types.attrsOf (types.submodule ({ config, ... }: { + type = types.attrsOf (types.submodule ({ name, ... }: { options = { enable = mkOption { @@ -190,7 +190,7 @@ in { path = mkOption { type = types.str; - default = config._module.args.name; + default = name; description = '' The path to the folder which should be shared. ''; @@ -198,7 +198,7 @@ in { id = mkOption { type = types.str; - default = config._module.args.name; + default = name; description = '' The id of the folder. Must be the same on all devices. ''; @@ -206,7 +206,7 @@ in { label = mkOption { type = types.str; - default = config._module.args.name; + default = name; description = '' The label of the folder. ''; From eec83d41e3e7d9ad5bc1086198d972d55bab1203 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Tue, 31 Dec 2019 04:25:35 +0100 Subject: [PATCH 7/8] lib/types: Allow paths as submodule values --- lib/types.nix | 8 +++++--- nixos/doc/manual/development/option-types.xml | 6 +++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/types.nix b/lib/types.nix index de3c4f0d603..847a4e902ca 100644 --- a/lib/types.nix +++ b/lib/types.nix @@ -376,14 +376,16 @@ rec { else unify (if shorthandOnlyDefinesConfig then { config = value; } else value); allModules = defs: modules ++ imap1 (n: { value, file }: - # Annotate the value with the location of its definition for better error messages - coerce (lib.modules.unifyModuleSyntax file "${toString file}-${toString n}") value + if isAttrs value || isFunction value then + # Annotate the value with the location of its definition for better error messages + coerce (lib.modules.unifyModuleSyntax file "${toString file}-${toString n}") value + else value ) defs; in mkOptionType rec { name = "submodule"; - check = x: isAttrs x || isFunction x; + check = x: isAttrs x || isFunction x || path.check x; merge = loc: defs: (evalModules { modules = allModules defs; diff --git a/nixos/doc/manual/development/option-types.xml b/nixos/doc/manual/development/option-types.xml index 1ec7e3efad7..173fdfcbbc8 100644 --- a/nixos/doc/manual/development/option-types.xml +++ b/nixos/doc/manual/development/option-types.xml @@ -257,9 +257,9 @@ A set of sub options o. - o can be an attribute set or a function - returning an attribute set. Submodules are used in composed types to - create modular options. This is equivalent to + o can be an attribute set, a function + returning an attribute set, or a path to a file containing such a value. Submodules are used in + composed types to create modular options. This is equivalent to types.submoduleWith { modules = toList o; shorthandOnlyDefinesConfig = true; }. Submodules are detailed in Date: Wed, 1 Jan 2020 01:11:45 +0100 Subject: [PATCH 8/8] lib/tests: Add submoduleWith tests --- lib/tests/modules.sh | 18 +++++++++++ .../modules/declare-submoduleWith-modules.nix | 30 +++++++++++++++++++ .../declare-submoduleWith-noshorthand.nix | 13 ++++++++ .../modules/declare-submoduleWith-path.nix | 12 ++++++++ .../declare-submoduleWith-shorthand.nix | 14 +++++++++ .../modules/declare-submoduleWith-special.nix | 17 +++++++++++ .../define-submoduleWith-noshorthand.nix | 3 ++ .../define-submoduleWith-shorthand.nix | 3 ++ 8 files changed, 110 insertions(+) create mode 100644 lib/tests/modules/declare-submoduleWith-modules.nix create mode 100644 lib/tests/modules/declare-submoduleWith-noshorthand.nix create mode 100644 lib/tests/modules/declare-submoduleWith-path.nix create mode 100644 lib/tests/modules/declare-submoduleWith-shorthand.nix create mode 100644 lib/tests/modules/declare-submoduleWith-special.nix create mode 100644 lib/tests/modules/define-submoduleWith-noshorthand.nix create mode 100644 lib/tests/modules/define-submoduleWith-shorthand.nix diff --git a/lib/tests/modules.sh b/lib/tests/modules.sh index cf344122cf4..4690e380ce3 100755 --- a/lib/tests/modules.sh +++ b/lib/tests/modules.sh @@ -164,6 +164,24 @@ checkConfigOutput "true" config.enableAlias ./alias-with-priority.nix checkConfigOutput "false" config.enable ./alias-with-priority-can-override.nix checkConfigOutput "false" config.enableAlias ./alias-with-priority-can-override.nix +# submoduleWith + +## specialArgs should work +checkConfigOutput "foo" config.submodule.foo ./declare-submoduleWith-special.nix + +## shorthandOnlyDefines config 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 'value is a boolean while a set was expected' config.submodule.config ./declare-submoduleWith-noshorthand.nix ./define-submoduleWith-shorthand.nix +checkConfigOutput "true" config.submodule.config ./declare-submoduleWith-noshorthand.nix ./define-submoduleWith-noshorthand.nix + +## submoduleWith should merge all modules in one swoop +checkConfigOutput "true" config.submodule.inner ./declare-submoduleWith-modules.nix +checkConfigOutput "true" config.submodule.outer ./declare-submoduleWith-modules.nix + +## Paths should be allowed as values and work as expected +checkConfigOutput "true" config.submodule.enable ./declare-submoduleWith-path.nix + cat <