From b02a3d7b0808eb7f31713488af68c7f7997e77cf Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 19 Mar 2020 23:45:23 +0100 Subject: lib/modules: Scope module evaluation variables more tightly This is a purely cosmetic change so it's easier to see dependencies between variables. --- lib/modules.nix | 59 ++++++++++++++++++++++++++++++--------------------------- 1 file changed, 31 insertions(+), 28 deletions(-) (limited to 'lib') diff --git a/lib/modules.nix b/lib/modules.nix index c18fec66c705..c8f832a00f0c 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -65,34 +65,37 @@ rec { }; }; - collected = collectModules - (specialArgs.modulesPath or "") - (modules ++ [ internalModule ]) - ({ inherit config options lib; } // specialArgs); - - options = mergeModules prefix (reverseList collected); - - # Traverse options and extract the option values into the final - # config set. At the same time, check whether all option - # definitions have matching declarations. - # !!! _module.check's value can't depend on any other config values - # without an infinite recursion. One way around this is to make the - # 'config' passed around to the modules be unconditionally unchecked, - # and only do the check in 'result'. - config = yieldConfig prefix options; - yieldConfig = prefix: set: - let res = removeAttrs (mapAttrs (n: v: - if isOption v then v.value - else yieldConfig (prefix ++ [n]) v) set) ["_definedNames"]; - in - if options._module.check.value && set ? _definedNames then - foldl' (res: m: - foldl' (res: name: - if set ? ${name} then res else throw "The option `${showOption (prefix ++ [name])}' defined in `${m.file}' does not exist.") - res m.names) - res set._definedNames - else - res; + options = + let collected = collectModules + (specialArgs.modulesPath or "") + (modules ++ [ internalModule ]) + ({ inherit lib options config; } // specialArgs); + in mergeModules prefix (reverseList collected); + + config = + let + # Traverse options and extract the option values into the final + # config set. At the same time, check whether all option + # definitions have matching declarations. + # !!! _module.check's value can't depend on any other config values + # without an infinite recursion. One way around this is to make the + # 'config' passed around to the modules be unconditionally unchecked, + # and only do the check in 'result'. + yieldConfig = prefix: set: + let res = removeAttrs (mapAttrs (n: v: + if isOption v then v.value + else yieldConfig (prefix ++ [n]) v) set) ["_definedNames"]; + in + if options._module.check.value && set ? _definedNames then + foldl' (res: m: + foldl' (res: name: + if set ? ${name} then res else throw "The option `${showOption (prefix ++ [name])}' defined in `${m.file}' does not exist.") + res m.names) + res set._definedNames + else + res; + in yieldConfig prefix options; + result = { inherit options; config = removeAttrs config [ "_module" ]; -- cgit v1.2.3 From fd75dc876586bde8cdb683a6952a41132e8db166 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Wed, 18 Mar 2020 20:09:09 +0100 Subject: lib/modules: Internally collect all unmatched definitions This fundamentally changes how the module evaluation internally handles definitions without an associated option. Previously the values of these definitions were discarded and only the names were propagated. This was fine because that's all that's needed for optionally checking whether all definitions have an associated option with _module.check. However with the upcoming change of supporting freeform modules, we *do* need the values of these. With this change, the module evaluation cleanly separates definitions that match an option, and ones that do not. --- lib/modules.nix | 91 ++++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 57 insertions(+), 34 deletions(-) (limited to 'lib') diff --git a/lib/modules.nix b/lib/modules.nix index c8f832a00f0c..ecf5ef3f4918 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -65,38 +65,24 @@ rec { }; }; - options = + merged = let collected = collectModules (specialArgs.modulesPath or "") (modules ++ [ internalModule ]) ({ inherit lib options config; } // specialArgs); in mergeModules prefix (reverseList collected); - config = - let - # Traverse options and extract the option values into the final - # config set. At the same time, check whether all option - # definitions have matching declarations. - # !!! _module.check's value can't depend on any other config values - # without an infinite recursion. One way around this is to make the - # 'config' passed around to the modules be unconditionally unchecked, - # and only do the check in 'result'. - yieldConfig = prefix: set: - let res = removeAttrs (mapAttrs (n: v: - if isOption v then v.value - else yieldConfig (prefix ++ [n]) v) set) ["_definedNames"]; - in - if options._module.check.value && set ? _definedNames then - foldl' (res: m: - foldl' (res: name: - if set ? ${name} then res else throw "The option `${showOption (prefix ++ [name])}' defined in `${m.file}' does not exist.") - res m.names) - res set._definedNames - else - res; - in yieldConfig prefix options; - - result = { + options = merged.matchedOptions; + + config = mapAttrsRecursiveCond (v: ! isOption v) (_: v: v.value) options; + + checkUnmatched = + if config._module.check && merged.unmatchedDefns != [] then + let inherit (head merged.unmatchedDefns) file prefix; + in throw "The option `${showOption prefix}' defined in `${file}' does not exist." + else null; + + result = builtins.seq checkUnmatched { inherit options; config = removeAttrs config [ "_module" ]; inherit (config) _module; @@ -236,7 +222,23 @@ rec { declarations in all modules, combining them into a single set. At the same time, for each option declaration, it will merge the corresponding option definitions in all machines, returning them - in the ‘value’ attribute of each option. */ + in the ‘value’ attribute of each option. + + This returns a set like + { + # A recursive set of options along with their final values + matchedOptions = { + foo = { _type = "option"; value = "option value of foo"; ... }; + bar.baz = { _type = "option"; value = "option value of bar.baz"; ... }; + ... + }; + # A list of definitions that weren't matched by any option + unmatchedDefns = [ + { file = "file.nix"; prefix = [ "qux" ]; value = "qux"; } + ... + ]; + } + */ mergeModules = prefix: modules: mergeModules' prefix modules (concatMap (m: map (config: { file = m._file; inherit config; }) (pushDownProperties m.config)) modules); @@ -283,9 +285,9 @@ rec { defnsByName' = byName "config" (module: value: [{ inherit (module) file; inherit value; }] ) configs; - in - (flip mapAttrs declsByName (name: decls: - # We're descending into attribute ‘name’. + + resultsByName = flip mapAttrs declsByName (name: decls: + # We're descending into attribute ‘name’. let loc = prefix ++ [name]; defns = defnsByName.${name} or []; @@ -294,7 +296,10 @@ rec { in if nrOptions == length decls then let opt = fixupOptionType loc (mergeOptionDecls loc decls); - in evalOptionValue loc opt defns' + in { + matchedOptions = evalOptionValue loc opt defns'; + unmatchedDefns = []; + } else if nrOptions != 0 then let firstOption = findFirst (m: isOption m.options) "" decls; @@ -302,9 +307,27 @@ rec { in throw "The option `${showOption loc}' in `${firstOption._file}' is a prefix of options in `${firstNonOption._file}'." else - mergeModules' loc decls defns - )) - // { _definedNames = map (m: { inherit (m) file; names = attrNames m.config; }) configs; }; + mergeModules' loc decls defns); + + matchedOptions = mapAttrs (n: v: v.matchedOptions) resultsByName; + + # an attrset 'name' => list of unmatched definitions for 'name' + unmatchedDefnsByName = + # Propagate all unmatched definitions from nested option sets + mapAttrs (n: v: v.unmatchedDefns) resultsByName + # Plus the definitions for the current prefix that don't have a matching option + // removeAttrs defnsByName' (attrNames matchedOptions); + in { + inherit matchedOptions; + + # Transforms unmatchedDefnsByName into a list of definitions + unmatchedDefns = concatLists (mapAttrsToList (name: defs: + map (def: def // { + # Set this so we know when the definition first left unmatched territory + prefix = [name] ++ (def.prefix or []); + }) defs + ) unmatchedDefnsByName); + }; /* Merge multiple option declarations into a single declaration. In general, there should be only one declaration of each option. -- cgit v1.2.3 From 65e25deb06cef43d4868785a4b6a4a49dbb6cfe2 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Fri, 20 Mar 2020 00:17:41 +0100 Subject: lib/modules: Implement freeform modules For programs that have a lot of (Nix-representable) configuration options, a simple way to represent this in a NixOS module is to declare an option of a type like `attrsOf str`, representing a key-value mapping which then gets generated into a config file. However with such a type, there's no way to add type checking for only some key values. On the other end of the spectrum, one can declare a single separate option for every key value that the program supports, ending up with a module with potentially 100s of options. This has the benefit that every value gets type checked, catching mistakes at evaluation time already. However the disadvantage is that the module becomes big, becomes coupled to the program version and takes a lot of effort to write and maintain. Previously there was a middle ground between these two extremes: Declare an option of a type like `attrsOf str`, but declare additional separate options for the values you wish to have type checked, and assign their values to the `attrsOf str` option. While this works decently, it has the problem of duplicated options, since now both the additional options and the `attrsOf str` option can be used to set a key value. This leads to confusion about what should happen if both are set, which defaults should apply, and more. Now with this change, a middle ground becomes available that solves above problems: The module system now supports setting a freeform type, which gets used for all definitions that don't have an associated option. This means that you can now declare all options you wish to have type checked, while for the rest a freeform type like `attrsOf str` can be used. --- lib/modules.nix | 40 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/modules.nix b/lib/modules.nix index ecf5ef3f4918..daffe5224ab2 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -58,6 +58,23 @@ rec { default = check; description = "Whether to check whether all option definitions have matching declarations."; }; + + _module.freeformType = mkOption { + # Disallow merging for now, but could be implemented nicely with a `types.optionType` + type = types.nullOr (types.uniq types.attrs); + internal = true; + default = null; + description = '' + If set, merge all definitions that don't have an associated option + together using this type. The result then gets combined with the + values of all declared options to produce the final + config value. + + If this is null, definitions without an option + will throw an error unless is + turned off. + ''; + }; }; config = { @@ -74,10 +91,29 @@ rec { options = merged.matchedOptions; - config = mapAttrsRecursiveCond (v: ! isOption v) (_: v: v.value) options; + config = + let + + # For definitions that have an associated option + declaredConfig = mapAttrsRecursiveCond (v: ! isOption v) (_: v: v.value) options; + + # If freeformType is set, this is for definitions that don't have an associated option + freeformConfig = + let + defs = map (def: { + file = def.file; + value = setAttrByPath def.prefix def.value; + }) merged.unmatchedDefns; + in declaredConfig._module.freeformType.merge prefix defs; + + in if declaredConfig._module.freeformType == null then declaredConfig + # Because all definitions that had an associated option ended in + # declaredConfig, freeformConfig can only contain the non-option + # paths, meaning recursiveUpdate will never override any value + else recursiveUpdate freeformConfig declaredConfig; checkUnmatched = - if config._module.check && merged.unmatchedDefns != [] then + if config._module.check && config._module.freeformType == null && merged.unmatchedDefns != [] then let inherit (head merged.unmatchedDefns) file prefix; in throw "The option `${showOption prefix}' defined in `${file}' does not exist." else null; -- cgit v1.2.3 From 2d45a62899d47c109a0b8ce4ca9d33265b8a1a37 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Fri, 20 Mar 2020 00:30:59 +0100 Subject: lib/types: Make submodules use the freeform type description Submodules that have a freeform type set behave as if that was the type of the option itself (for values that don't have an option). Since the submodules options are shown as separate entries in the manual, it makes sense to show the freeform type as the submodule type. --- lib/types.nix | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'lib') diff --git a/lib/types.nix b/lib/types.nix index 6fd6de7e1fd9..1845b6ae339e 100644 --- a/lib/types.nix +++ b/lib/types.nix @@ -486,9 +486,15 @@ rec { else value ) defs; + freeformType = (evalModules { + inherit modules specialArgs; + args.name = "‹name›"; + })._module.freeformType; + in mkOptionType rec { name = "submodule"; + description = freeformType.description or name; check = x: isAttrs x || isFunction x || path.check x; merge = loc: defs: (evalModules { -- cgit v1.2.3 From 446d80d28deb9929aca2a70699b3f32c2f1297d5 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Sun, 22 Mar 2020 20:55:54 +0100 Subject: lib/tests: Add tests for freeform modules --- lib/tests/modules.sh | 21 +++++++++++++++++++++ .../modules/define-value-string-properties.nix | 12 ++++++++++++ lib/tests/modules/freeform-attrsOf.nix | 3 +++ lib/tests/modules/freeform-lazyAttrsOf.nix | 3 +++ lib/tests/modules/freeform-nested.nix | 7 +++++++ lib/tests/modules/freeform-str-dep-unstr.nix | 8 ++++++++ lib/tests/modules/freeform-unstr-dep-str.nix | 8 ++++++++ 7 files changed, 62 insertions(+) create mode 100644 lib/tests/modules/define-value-string-properties.nix create mode 100644 lib/tests/modules/freeform-attrsOf.nix create mode 100644 lib/tests/modules/freeform-lazyAttrsOf.nix create mode 100644 lib/tests/modules/freeform-nested.nix create mode 100644 lib/tests/modules/freeform-str-dep-unstr.nix create mode 100644 lib/tests/modules/freeform-unstr-dep-str.nix (limited to 'lib') diff --git a/lib/tests/modules.sh b/lib/tests/modules.sh index 6258244457aa..848b10e17f67 100755 --- a/lib/tests/modules.sh +++ b/lib/tests/modules.sh @@ -210,6 +210,27 @@ checkConfigOutput "empty" config.value.foo ./declare-lazyAttrsOf.nix ./attrsOf-c checkConfigError 'The option value .* in .* is not of type .*' \ config.value ./declare-int-unsigned-value.nix ./define-value-list.nix ./define-value-int-positive.nix +## Freeform modules +# Assigning without a declared option should work +checkConfigOutput 24 config.value ./freeform-attrsOf.nix ./define-value-string.nix +# but only if the type matches +checkConfigError 'The option value .* in .* is not of type .*' config.value ./freeform-attrsOf.nix ./define-value-list.nix +# and properties should be applied +checkConfigOutput yes config.value ./freeform-attrsOf.nix ./define-value-string-properties.nix +# Options should still be declarable, and be able to have a type that doesn't match the freeform type +checkConfigOutput false config.enable ./freeform-attrsOf.nix ./define-value-string.nix ./declare-enable.nix +checkConfigOutput 24 config.value ./freeform-attrsOf.nix ./define-value-string.nix ./declare-enable.nix +# and this should work too with nested values +checkConfigOutput false config.nest.foo ./freeform-attrsOf.nix ./freeform-nested.nix +checkConfigOutput bar config.nest.bar ./freeform-attrsOf.nix ./freeform-nested.nix +# Check whether a declared option can depend on an freeform-typed one +checkConfigOutput null config.foo ./freeform-attrsOf.nix ./freeform-str-dep-unstr.nix +checkConfigOutput 24 config.foo ./freeform-attrsOf.nix ./freeform-str-dep-unstr.nix ./define-value-string.nix +# Check whether an freeform-typed value can depend on a declared option, this can only work with lazyAttrsOf +checkConfigError 'infinite recursion encountered' config.foo ./freeform-attrsOf.nix ./freeform-unstr-dep-str.nix +checkConfigError 'The option .* is used but not defined' config.foo ./freeform-lazyAttrsOf.nix ./freeform-unstr-dep-str.nix +checkConfigOutput 24 config.foo ./freeform-lazyAttrsOf.nix ./freeform-unstr-dep-str.nix ./define-value-string.nix + cat < Date: Tue, 4 Aug 2020 00:08:43 +0200 Subject: lib/modules: Fix freeform modules when there's no definitions --- lib/modules.nix | 3 ++- lib/tests/modules.sh | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/modules.nix b/lib/modules.nix index daffe5224ab2..1d2c4a1acbb5 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -104,7 +104,8 @@ rec { file = def.file; value = setAttrByPath def.prefix def.value; }) merged.unmatchedDefns; - in declaredConfig._module.freeformType.merge prefix defs; + in if defs == [] then {} + else declaredConfig._module.freeformType.merge prefix defs; in if declaredConfig._module.freeformType == null then declaredConfig # Because all definitions that had an associated option ended in diff --git a/lib/tests/modules.sh b/lib/tests/modules.sh index 848b10e17f67..943deebe3c09 100755 --- a/lib/tests/modules.sh +++ b/lib/tests/modules.sh @@ -213,6 +213,8 @@ checkConfigError 'The option value .* in .* is not of type .*' \ ## Freeform modules # Assigning without a declared option should work checkConfigOutput 24 config.value ./freeform-attrsOf.nix ./define-value-string.nix +# No freeform assigments shouldn't make it error +checkConfigOutput '{ }' config ./freeform-attrsOf.nix # but only if the type matches checkConfigError 'The option value .* in .* is not of type .*' config.value ./freeform-attrsOf.nix ./define-value-list.nix # and properties should be applied -- cgit v1.2.3 From 42cf8130d76ac811f8d1c934f9b44976050cc90f Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Mon, 10 Aug 2020 17:25:46 +0200 Subject: lib/modules: Add syntactic sugar for config._module.freeformType This introduces `freeformType` as a top-level module attribute, allowing definitions like { freeformType = ...; options = ...; config = ...; } --- lib/modules.nix | 16 ++++++++++------ lib/tests/modules/freeform-attrsOf.nix | 2 +- lib/tests/modules/freeform-lazyAttrsOf.nix | 2 +- 3 files changed, 12 insertions(+), 8 deletions(-) (limited to 'lib') diff --git a/lib/modules.nix b/lib/modules.nix index 1d2c4a1acbb5..55a53b3909a6 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -200,12 +200,16 @@ rec { /* Massage a module into canonical form, that is, a set consisting of ‘options’, ‘config’ and ‘imports’ attributes. */ unifyModuleSyntax = file: key: m: - let addMeta = config: if m ? meta - then mkMerge [ config { meta = m.meta; } ] - else config; + let + addMeta = config: if m ? meta + then mkMerge [ config { meta = m.meta; } ] + else config; + addFreeformType = config: if m ? freeformType + then mkMerge [ config { _module.freeformType = m.freeformType; } ] + else config; in if m ? config || m ? options then - let badAttrs = removeAttrs m ["_file" "key" "disabledModules" "imports" "options" "config" "meta"]; in + let badAttrs = removeAttrs m ["_file" "key" "disabledModules" "imports" "options" "config" "meta" "freeformType"]; in if badAttrs != {} then throw "Module `${key}' has an unsupported attribute `${head (attrNames badAttrs)}'. This is caused by introducing a top-level `config' or `options' attribute. Add configuration attributes immediately on the top level instead, or move all of them (namely: ${toString (attrNames badAttrs)}) into the explicit `config' attribute." else @@ -214,7 +218,7 @@ rec { disabledModules = m.disabledModules or []; imports = m.imports or []; options = m.options or {}; - config = addMeta (m.config or {}); + config = addFreeformType (addMeta (m.config or {})); } else { _file = m._file or file; @@ -222,7 +226,7 @@ rec { disabledModules = m.disabledModules or []; imports = m.require or [] ++ m.imports or []; options = {}; - config = addMeta (removeAttrs m ["_file" "key" "disabledModules" "require" "imports"]); + config = addFreeformType (addMeta (removeAttrs m ["_file" "key" "disabledModules" "require" "imports" "freeformType"])); }; applyIfFunction = key: f: args@{ config, options, lib, ... }: if isFunction f then diff --git a/lib/tests/modules/freeform-attrsOf.nix b/lib/tests/modules/freeform-attrsOf.nix index 5dbf4a9d307c..8cc577f38a6c 100644 --- a/lib/tests/modules/freeform-attrsOf.nix +++ b/lib/tests/modules/freeform-attrsOf.nix @@ -1,3 +1,3 @@ { lib, ... }: { - config._module.freeformType = with lib.types; attrsOf (either str (attrsOf str)); + freeformType = with lib.types; attrsOf (either str (attrsOf str)); } diff --git a/lib/tests/modules/freeform-lazyAttrsOf.nix b/lib/tests/modules/freeform-lazyAttrsOf.nix index 524efa6bd930..36d6c0b13fca 100644 --- a/lib/tests/modules/freeform-lazyAttrsOf.nix +++ b/lib/tests/modules/freeform-lazyAttrsOf.nix @@ -1,3 +1,3 @@ { lib, ... }: { - config._module.freeformType = with lib.types; lazyAttrsOf (either str (lazyAttrsOf str)); + freeformType = with lib.types; lazyAttrsOf (either str (lazyAttrsOf str)); } -- cgit v1.2.3