From 9eecf2d05711b8e8677af8ceda17f6b9938b3eb3 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 19 Mar 2020 03:48:46 +0100 Subject: [PATCH 1/2] Revert "lib/modules: Throw better error when definitions assign to an option set" This reverts commit 15c873b486347e7861c64fb0b5a7852be9fc82e4. This was causing infinite recursion when depending on nested options --- lib/modules.nix | 4 +--- lib/tests/modules.sh | 3 --- lib/tests/modules/declare-option-set.nix | 3 --- 3 files changed, 1 insertion(+), 9 deletions(-) delete mode 100644 lib/tests/modules/declare-option-set.nix diff --git a/lib/modules.nix b/lib/modules.nix index 518f4047cc60..c18fec66c705 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -299,9 +299,7 @@ rec { in throw "The option `${showOption loc}' in `${firstOption._file}' is a prefix of options in `${firstNonOption._file}'." else - 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?" + mergeModules' loc decls defns )) // { _definedNames = map (m: { inherit (m) file; names = attrNames m.config; }) configs; }; diff --git a/lib/tests/modules.sh b/lib/tests/modules.sh index 1b3d01646d84..e80a3eccecd1 100755 --- a/lib/tests/modules.sh +++ b/lib/tests/modules.sh @@ -199,9 +199,6 @@ 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 # 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 .*' \ diff --git a/lib/tests/modules/declare-option-set.nix b/lib/tests/modules/declare-option-set.nix deleted file mode 100644 index fddc650ffa90..000000000000 --- a/lib/tests/modules/declare-option-set.nix +++ /dev/null @@ -1,3 +0,0 @@ -{ - options.value = {}; -} From 742e3fc0020573deecc04029764b288fdbaed4cc Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 19 Mar 2020 03:46:50 +0100 Subject: [PATCH 2/2] lib/tests: Check for nested option-dependent definitions --- lib/tests/modules.sh | 3 +++ lib/tests/modules/declare-enable-nested.nix | 14 ++++++++++++++ .../declare-int-positive-value-nested.nix | 9 +++++++++ .../modules/define-option-dependently-nested.nix | 16 ++++++++++++++++ 4 files changed, 42 insertions(+) create mode 100644 lib/tests/modules/declare-enable-nested.nix create mode 100644 lib/tests/modules/declare-int-positive-value-nested.nix create mode 100644 lib/tests/modules/define-option-dependently-nested.nix diff --git a/lib/tests/modules.sh b/lib/tests/modules.sh index e80a3eccecd1..e81cf016ee9a 100755 --- a/lib/tests/modules.sh +++ b/lib/tests/modules.sh @@ -189,6 +189,9 @@ checkConfigOutput "true" config.enable ./import-from-store.nix checkConfigOutput true config.enable ./define-option-dependently.nix ./declare-enable.nix ./declare-int-positive-value.nix checkConfigOutput 360 config.value ./define-option-dependently.nix ./declare-enable.nix ./declare-int-positive-value.nix checkConfigOutput 7 config.value ./define-option-dependently.nix ./declare-int-positive-value.nix +checkConfigOutput true config.set.enable ./define-option-dependently-nested.nix ./declare-enable-nested.nix ./declare-int-positive-value-nested.nix +checkConfigOutput 360 config.set.value ./define-option-dependently-nested.nix ./declare-enable-nested.nix ./declare-int-positive-value-nested.nix +checkConfigOutput 7 config.set.value ./define-option-dependently-nested.nix ./declare-int-positive-value-nested.nix # Check attrsOf and lazyAttrsOf. Only lazyAttrsOf should be lazy, and only # attrsOf should work with conditional definitions diff --git a/lib/tests/modules/declare-enable-nested.nix b/lib/tests/modules/declare-enable-nested.nix new file mode 100644 index 000000000000..c8da8273cba1 --- /dev/null +++ b/lib/tests/modules/declare-enable-nested.nix @@ -0,0 +1,14 @@ +{ lib, ... }: + +{ + options.set = { + enable = lib.mkOption { + default = false; + example = true; + type = lib.types.bool; + description = '' + Some descriptive text + ''; + }; + }; +} diff --git a/lib/tests/modules/declare-int-positive-value-nested.nix b/lib/tests/modules/declare-int-positive-value-nested.nix new file mode 100644 index 000000000000..72d2fb89fc3b --- /dev/null +++ b/lib/tests/modules/declare-int-positive-value-nested.nix @@ -0,0 +1,9 @@ +{ lib, ... }: + +{ + options.set = { + value = lib.mkOption { + type = lib.types.ints.positive; + }; + }; +} diff --git a/lib/tests/modules/define-option-dependently-nested.nix b/lib/tests/modules/define-option-dependently-nested.nix new file mode 100644 index 000000000000..69ee4255534a --- /dev/null +++ b/lib/tests/modules/define-option-dependently-nested.nix @@ -0,0 +1,16 @@ +{ lib, options, ... }: + +# Some modules may be distributed separately and need to adapt to other modules +# that are distributed and versioned separately. +{ + + # Always defined, but the value depends on the presence of an option. + config.set = { + value = if options ? set.enable then 360 else 7; + } + # Only define if possible. + // lib.optionalAttrs (options ? set.enable) { + enable = true; + }; + +}