From dcdd232939232d04c1132b4cc242dd3dac44be8c Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Mon, 16 Mar 2020 21:05:52 +0100 Subject: [PATCH 1/3] lib/modules: Remove internal _module attribute from config The _module option is added as an internal option set, and it messes up the results of module evaluations, requiring people to manually filter _modules out. If people depend on this, they can still use config._module from inside the modules, exposing _module as an explicitly declared user option. Or alternatively with the _module attribute now returned by evalModules. --- lib/modules.nix | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/modules.nix b/lib/modules.nix index 6cbef5632bd72..48f4c04ed1bfd 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -93,7 +93,11 @@ rec { res set._definedNames else res; - result = { inherit options config; }; + result = { + inherit options; + config = removeAttrs config [ "_module" ]; + inherit (config) _module; + }; in result; # collectModules :: (modulesPath: String) -> (modules: [ Module ]) -> (args: Attrs) -> [ Module ] From 15c873b486347e7861c64fb0b5a7852be9fc82e4 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Mon, 16 Mar 2020 21:08:39 +0100 Subject: [PATCH 2/3] lib/modules: Throw better error when definitions assign to an option set --- lib/modules.nix | 4 +++- lib/tests/modules.sh | 4 ++++ lib/tests/modules/declare-option-set.nix | 3 +++ 3 files changed, 10 insertions(+), 1 deletion(-) create mode 100644 lib/tests/modules/declare-option-set.nix diff --git a/lib/modules.nix b/lib/modules.nix index 48f4c04ed1bfd..22df89f360ac8 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -299,7 +299,9 @@ rec { in throw "The option `${showOption loc}' in `${firstOption._file}' is a prefix of options in `${firstNonOption._file}'." else - mergeModules' loc decls defns + if all (def: isAttrs def.value) defns' then mergeModules' loc decls defns + else let firstInvalid = findFirst (def: ! isAttrs def.value) null defns'; + in throw "The option path `${showOption loc}' is an attribute set of options, but it is defined to not be an attribute set in `${firstInvalid.file}'. Did you define its value at the correct and complete path?" )) // { _definedNames = map (m: { inherit (m) file; names = attrNames m.config; }) configs; }; diff --git a/lib/tests/modules.sh b/lib/tests/modules.sh index 8cd632a439cda..0e2fd0bf65d2a 100755 --- a/lib/tests/modules.sh +++ b/lib/tests/modules.sh @@ -194,6 +194,10 @@ checkConfigOutput "true" config.conditionalWorks ./declare-attrsOf.nix ./attrsOf checkConfigOutput "false" config.conditionalWorks ./declare-lazyAttrsOf.nix ./attrsOf-conditional-check.nix checkConfigOutput "empty" config.value.foo ./declare-lazyAttrsOf.nix ./attrsOf-conditional-check.nix +# Check error for when an option set is defined to be a non-attribute set value +checkConfigError 'The option path .* is an attribute set of options, but it is defined to not be an attribute set in' \ + config.value ./declare-option-set.nix ./define-value-int-zero.nix + cat < Date: Mon, 16 Mar 2020 21:10:05 +0100 Subject: [PATCH 3/3] lib/modules: Fix type checks not being done before merging Co-Authored-By: Robert Hensing --- lib/modules.nix | 7 +++---- lib/tests/modules.sh | 4 ++++ 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/modules.nix b/lib/modules.nix index 22df89f360ac8..518f4047cc60d 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -416,10 +416,9 @@ rec { # Type-check the remaining definitions, and merge them. Or throw if no definitions. mergedValue = if isDefined then - foldl' (res: def: - if type.check def.value then res - else throw "The option value `${showOption loc}' in `${def.file}' is not of type `${type.description}'." - ) (type.merge loc defsFinal) defsFinal + if all (def: type.check def.value) defsFinal then type.merge loc defsFinal + else let firstInvalid = findFirst (def: ! type.check def.value) null defsFinal; + in throw "The option value `${showOption loc}' in `${firstInvalid.file}' is not of type `${type.description}'." else # (nixos-option detects this specific error message and gives it special # handling. If changed here, please change it there too.) diff --git a/lib/tests/modules.sh b/lib/tests/modules.sh index 0e2fd0bf65d2a..7713207dadda8 100755 --- a/lib/tests/modules.sh +++ b/lib/tests/modules.sh @@ -198,6 +198,10 @@ checkConfigOutput "empty" config.value.foo ./declare-lazyAttrsOf.nix ./attrsOf-c checkConfigError 'The option path .* is an attribute set of options, but it is defined to not be an attribute set in' \ config.value ./declare-option-set.nix ./define-value-int-zero.nix +# Even with multiple assignments, a type error should be thrown if any of them aren't valid +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 + cat <