diff options
author | Silvan Mosberger <contact@infinisil.com> | 2020-12-18 14:17:52 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-12-18 14:17:52 +0100 |
commit | 7698aa9776244ae69aa6b3714f05d676dd33b100 (patch) | |
tree | ea309cb20a06127342d2b04a2ba87c82157381af /lib | |
parent | c23db78bbd474c4d0c5c3c551877523b4a50db06 (diff) | |
parent | a6a70d14a9f7b885e65a51c5e6bd02145884ee50 (diff) | |
download | nixpkgs-7698aa9776244ae69aa6b3714f05d676dd33b100.tar nixpkgs-7698aa9776244ae69aa6b3714f05d676dd33b100.tar.gz nixpkgs-7698aa9776244ae69aa6b3714f05d676dd33b100.tar.bz2 nixpkgs-7698aa9776244ae69aa6b3714f05d676dd33b100.tar.lz nixpkgs-7698aa9776244ae69aa6b3714f05d676dd33b100.tar.xz nixpkgs-7698aa9776244ae69aa6b3714f05d676dd33b100.tar.zst nixpkgs-7698aa9776244ae69aa6b3714f05d676dd33b100.zip |
Merge pull request #97023 from Infinisil/module-assertions
Module-builtin assertions, disabling assertions and submodule assertions
Diffstat (limited to 'lib')
-rw-r--r-- | lib/modules.nix | 164 | ||||
-rw-r--r-- | lib/options.nix | 2 | ||||
-rw-r--r-- | lib/tests/misc.nix | 2 | ||||
-rwxr-xr-x | lib/tests/modules.sh | 78 | ||||
-rw-r--r-- | lib/tests/modules/assertions/condition-true.nix | 8 | ||||
-rw-r--r-- | lib/tests/modules/assertions/enable-false.nix | 9 | ||||
-rw-r--r-- | lib/tests/modules/assertions/multi.nix | 23 | ||||
-rw-r--r-- | lib/tests/modules/assertions/simple.nix | 6 | ||||
-rw-r--r-- | lib/tests/modules/assertions/submodule-attrsOf-attrsOf.nix | 13 | ||||
-rw-r--r-- | lib/tests/modules/assertions/submodule-attrsOf.nix | 13 | ||||
-rw-r--r-- | lib/tests/modules/assertions/submodule.nix | 13 | ||||
-rw-r--r-- | lib/tests/modules/assertions/underscore-attributes.nix | 8 | ||||
-rw-r--r-- | lib/tests/modules/assertions/warning.nix | 9 |
13 files changed, 303 insertions, 45 deletions
diff --git a/lib/modules.nix b/lib/modules.nix index 3f2bfd478b0..5548c5f7049 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -46,6 +46,7 @@ let showFiles showOption unknownModule + literalExample ; in @@ -72,14 +73,20 @@ rec { check ? true }: let - # This internal module declare internal options under the `_module' - # attribute. These options are fragile, as they are used by the - # module system to change the interpretation of modules. + # An internal module that's always added, defining special options which + # change the behavior of the module evaluation itself. This is under a + # `_`-prefixed namespace in order to prevent name clashes with + # user-defined options internalModule = rec { - _file = ./modules.nix; + # FIXME: Using ./modules.nix directly breaks the doc for some reason + _file = "lib/modules.nix"; key = _file; + # Most of these options are set to be internal only for prefix != [], + # aka it's a submodule evaluation. This way their docs are displayed + # only once as a top-level NixOS option, but will be hidden for all + # submodules, even though they are available there too options = { _module.args = mkOption { # Because things like `mkIf` are entirely useless for @@ -89,7 +96,7 @@ rec { # a `_module.args.pkgs = import (fetchTarball { ... }) {}` won't # start a download when `pkgs` wasn't evaluated. type = types.lazyAttrsOf types.unspecified; - internal = true; + internal = prefix != []; description = "Arguments passed to each module."; }; @@ -97,13 +104,19 @@ rec { type = types.bool; internal = true; default = check; - description = "Whether to check whether all option definitions have matching declarations."; + description = '' + Whether to check whether all option definitions have matching + declarations. + + Note that this has nothing to do with the similarly named + <option>_module.checks</option> option + ''; }; _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; + internal = prefix != []; default = null; description = '' If set, merge all definitions that don't have an associated option @@ -116,6 +129,75 @@ rec { turned off. ''; }; + + _module.checks = mkOption { + description = '' + Evaluation checks to trigger during module evaluation. The + attribute name will be displayed when it is triggered, allowing + users to disable/change these checks if necessary. See + the section on Warnings and Assertions in the manual for more + information. + ''; + example = literalExample '' + { + gpgSshAgent = { + enable = config.programs.gnupg.agent.enableSSHSupport && config.programs.ssh.startAgent; + message = "You can't use ssh-agent and GnuPG agent with SSH support enabled at the same time!"; + }; + + grafanaPassword = { + enable = config.services.grafana.database.password != ""; + message = "Grafana passwords will be stored as plaintext in the Nix store!"; + type = "warning"; + }; + } + ''; + default = {}; + internal = prefix != []; + type = types.attrsOf (types.submodule { + options.enable = mkOption { + description = '' + Whether to enable this check. Set this to false to not trigger + any errors or warning messages. This is useful for ignoring a + check in case it doesn't make sense in certain scenarios. + ''; + default = true; + type = types.bool; + }; + + options.check = mkOption { + description = '' + The condition that must succeed in order for this check to be + successful and not trigger a warning or error. + ''; + readOnly = true; + type = types.bool; + }; + + options.type = mkOption { + description = '' + The type of the check. The default + <literal>"error"</literal> type will cause evaluation to fail, + while the <literal>"warning"</literal> type will only show a + warning. + ''; + type = types.enum [ "error" "warning" ]; + default = "error"; + example = "warning"; + }; + + options.message = mkOption { + description = '' + The message to display if this check triggers. + To display option names in the message, add + <literal>options</literal> to the module function arguments + and use <literal>''${options.path.to.option}</literal>. + ''; + type = types.str; + example = "Enabling both \${options.services.foo.enable} and \${options.services.bar.enable} is not possible."; + }; + }); + }; }; config = { @@ -154,6 +236,35 @@ rec { # paths, meaning recursiveUpdate will never override any value else recursiveUpdate freeformConfig declaredConfig; + # Triggers all checks defined by _module.checks before returning its argument + triggerChecks = let + + handleCheck = errors: name: + let + value = config._module.checks.${name}; + show = + # Assertions with a _ prefix aren't meant to be configurable + if lib.hasPrefix "_" name then value.message + else "[${showOption prefix}${optionalString (prefix != []) "/"}${name}] ${value.message}"; + in + if value.enable -> value.check then errors + else if value.type == "warning" then lib.warn show errors + else if value.type == "error" then errors ++ [ show ] + else abort "Unknown check type ${value.type}"; + + errors = lib.foldl' handleCheck [] (lib.attrNames config._module.checks); + + errorMessage = '' + Failed checks: + ${lib.concatMapStringsSep "\n" (a: "- ${a}") errors} + ''; + + trigger = if errors == [] then null else throw errorMessage; + + in builtins.seq trigger; + + finalConfig = triggerChecks (removeAttrs config [ "_module" ]); + checkUnmatched = if config._module.check && config._module.freeformType == null && merged.unmatchedDefns != [] then let @@ -173,7 +284,7 @@ rec { result = builtins.seq checkUnmatched { inherit options; - config = removeAttrs config [ "_module" ]; + config = finalConfig; inherit (config) _module; }; in result; @@ -514,6 +625,8 @@ rec { definitions = map (def: def.value) res.defsFinal; files = map (def: def.file) res.defsFinal; inherit (res) isDefined; + # This allows options to be correctly displayed using `${options.path.to.it}` + __toString = _: showOption loc; }; # Merge definitions of a value of a given type. @@ -773,14 +886,15 @@ rec { visible = false; apply = x: throw "The option `${showOption optionName}' can no longer be used since it's been removed. ${replacementInstructions}"; }); - config.assertions = - let opt = getAttrFromPath optionName options; in [{ - assertion = !opt.isDefined; + config._module.checks = + let opt = getAttrFromPath optionName options; in { + ${"removed-" + showOption optionName} = lib.mkIf opt.isDefined { message = '' The option definition `${showOption optionName}' in ${showFiles opt.files} no longer has any effect; please remove it. ${replacementInstructions} ''; - }]; + }; + }; }; /* Return a module that causes a warning to be shown if the @@ -841,14 +955,18 @@ rec { })) from); config = { - warnings = filter (x: x != "") (map (f: - let val = getAttrFromPath f config; - opt = getAttrFromPath f options; - in - optionalString - (val != "_mkMergedOptionModule") - "The option `${showOption f}' defined in ${showFiles opt.files} has been changed to `${showOption to}' that has a different type. Please read `${showOption to}' documentation and update your configuration accordingly." - ) from); + _module.checks = + let warningMessages = map (f: + let val = getAttrFromPath f config; + opt = getAttrFromPath f options; + in { + ${"merged" + showOption f} = lib.mkIf (val != "_mkMergedOptionModule") { + type = "warning"; + message = "The option `${showOption f}' defined in ${showFiles opt.files} has been changed to `${showOption to}' that has a different type. Please read `${showOption to}' documentation and update your configuration accordingly."; + }; + } + ) from; + in mkMerge warningMessages; } // setAttrByPath to (mkMerge (optional (any (f: (getAttrFromPath f config) != "_mkMergedOptionModule") from) @@ -907,8 +1025,10 @@ rec { }); config = mkMerge [ { - warnings = optional (warn && fromOpt.isDefined) - "The option `${showOption from}' defined in ${showFiles fromOpt.files} has been renamed to `${showOption to}'."; + _module.checks.${"renamed-" + showOption from} = mkIf (warn && fromOpt.isDefined) { + type = "warning"; + message = "The option `${showOption from}' defined in ${showFiles fromOpt.files} has been renamed to `${showOption to}'."; + }; } (if withPriority then mkAliasAndWrapDefsWithPriority (setAttrByPath to) fromOpt diff --git a/lib/options.nix b/lib/options.nix index 87cd8b79796..5c042a6c6f2 100644 --- a/lib/options.nix +++ b/lib/options.nix @@ -192,7 +192,7 @@ rec { let ss = opt.type.getSubOptions opt.loc; in if ss != {} then optionAttrSetToDocList' opt.loc ss else []; in - [ docOption ] ++ optionals docOption.visible subOptions) (collect isOption options); + [ docOption ] ++ optionals (docOption.visible && ! docOption.internal) subOptions) (collect isOption options); /* This function recursively removes all derivation attributes from diff --git a/lib/tests/misc.nix b/lib/tests/misc.nix index 35a5801c724..2d53ed81176 100644 --- a/lib/tests/misc.nix +++ b/lib/tests/misc.nix @@ -655,7 +655,7 @@ runTests { modules = [ module ]; }).options; - locs = filter (o: ! o.internal) (optionAttrSetToDocList options); + locs = filter (o: ! o.internal) (optionAttrSetToDocList (removeAttrs options [ "_module" ])); in map (o: o.loc) locs; expected = [ [ "foo" ] [ "foo" "<name>" "bar" ] [ "foo" "bar" ] ]; }; diff --git a/lib/tests/modules.sh b/lib/tests/modules.sh index 309c5311361..775be9f7209 100755 --- a/lib/tests/modules.sh +++ b/lib/tests/modules.sh @@ -27,37 +27,50 @@ reportFailure() { fail=$((fail + 1)) } -checkConfigOutput() { +checkConfigCodeOutErr() { + local expectedExit=$1 + shift; local outputContains=$1 shift; - if evalConfig "$@" 2>/dev/null | grep --silent "$outputContains" ; then - pass=$((pass + 1)) - return 0; - else - echo 2>&1 "error: Expected result matching '$outputContains', while evaluating" + local errorContains=$1 + shift; + out=$(mktemp) + err=$(mktemp) + evalConfig "$@" 1>"$out" 2>"$err" + if [[ "$?" -ne "$expectedExit" ]]; then + echo 2>&1 "error: Expected exit code $expectedExit while evaluating" + reportFailure "$@" + return 1 + fi + + if [[ -n "$outputContains" ]] && ! grep -zP --silent "$outputContains" "$out"; then + echo 2>&1 "error: Expected output matching '$outputContains', while evaluating" reportFailure "$@" return 1 fi + + if [[ -n "$errorContains" ]] && ! grep -zP --silent "$errorContains" "$err"; then + echo 2>&1 "error: Expected error matching '$errorContains', while evaluating" + reportFailure "$@" + return 1 + fi + + pass=$((pass + 1)) + + # Clean up temp files + rm "$out" "$err" +} + +checkConfigOutput() { + local outputContains=$1 + shift; + checkConfigCodeOutErr 0 "$outputContains" "" "$@" } checkConfigError() { local errorContains=$1 - local err="" shift; - if err==$(evalConfig "$@" 2>&1 >/dev/null); then - echo 2>&1 "error: Expected error code, got exit code 0, while evaluating" - reportFailure "$@" - return 1 - else - if echo "$err" | grep -zP --silent "$errorContains" ; then - pass=$((pass + 1)) - return 0; - else - echo 2>&1 "error: Expected error matching '$errorContains', while evaluating" - reportFailure "$@" - return 1 - fi - fi + checkConfigCodeOutErr 1 "" "$errorContains" "$@" } # Check boolean option. @@ -262,6 +275,29 @@ checkConfigOutput true config.value.mkbefore ./types-anything/mk-mods.nix checkConfigOutput 1 config.value.nested.foo ./types-anything/mk-mods.nix checkConfigOutput baz config.value.nested.bar.baz ./types-anything/mk-mods.nix +## Module assertions +# Check that assertions are triggered by default for just evaluating config +checkConfigError 'Failed checks:\n- \[test\] Assertion failed' config ./assertions/simple.nix + +# Assertion is not triggered when enable is false or condition is true +checkConfigOutput '{ }' config ./assertions/condition-true.nix +checkConfigOutput '{ }' config ./assertions/enable-false.nix + +# Warnings should be displayed on standard error +checkConfigCodeOutErr 0 '{ }' 'warning: \[test\] Warning message' config ./assertions/warning.nix + +# Check that multiple assertions and warnings can be triggered at once +checkConfigError 'Failed checks:\n- \[test1\] Assertion 1 failed\n- \[test2\] Assertion 2 failed' config ./assertions/multi.nix +checkConfigError 'trace: warning: \[test3\] Warning 3 failed\ntrace: warning: \[test4\] Warning 4 failed' config ./assertions/multi.nix + +# Submodules should be able to trigger assertions and display the submodule prefix in their error +checkConfigError 'Failed checks:\n- \[foo/test\] Assertion failed' config.foo ./assertions/submodule.nix +checkConfigError 'Failed checks:\n- \[foo.bar/test\] Assertion failed' config.foo.bar ./assertions/submodule-attrsOf.nix +checkConfigError 'Failed checks:\n- \[foo.bar.baz/test\] Assertion failed' config.foo.bar.baz ./assertions/submodule-attrsOf-attrsOf.nix + +# Assertions with an attribute starting with _ shouldn't have their name displayed +checkConfigError 'Failed checks:\n- Assertion failed' config ./assertions/underscore-attributes.nix + cat <<EOF ====== module tests ====== $pass Pass diff --git a/lib/tests/modules/assertions/condition-true.nix b/lib/tests/modules/assertions/condition-true.nix new file mode 100644 index 00000000000..7ca0817a239 --- /dev/null +++ b/lib/tests/modules/assertions/condition-true.nix @@ -0,0 +1,8 @@ +{ + + _module.checks.test = { + check = true; + message = "Assertion failed"; + }; + +} diff --git a/lib/tests/modules/assertions/enable-false.nix b/lib/tests/modules/assertions/enable-false.nix new file mode 100644 index 00000000000..11f753bb32e --- /dev/null +++ b/lib/tests/modules/assertions/enable-false.nix @@ -0,0 +1,9 @@ +{ + + _module.checks.test = { + enable = false; + check = false; + message = "Assertion failed"; + }; + +} diff --git a/lib/tests/modules/assertions/multi.nix b/lib/tests/modules/assertions/multi.nix new file mode 100644 index 00000000000..1e2e14b8643 --- /dev/null +++ b/lib/tests/modules/assertions/multi.nix @@ -0,0 +1,23 @@ +{ + + _module.checks = { + test1 = { + check = false; + message = "Assertion 1 failed"; + }; + test2 = { + check = false; + message = "Assertion 2 failed"; + }; + test3 = { + check = false; + message = "Warning 3 failed"; + type = "warning"; + }; + test4 = { + check = false; + message = "Warning 4 failed"; + type = "warning"; + }; + }; +} diff --git a/lib/tests/modules/assertions/simple.nix b/lib/tests/modules/assertions/simple.nix new file mode 100644 index 00000000000..115d89a3036 --- /dev/null +++ b/lib/tests/modules/assertions/simple.nix @@ -0,0 +1,6 @@ +{ + _module.checks.test = { + check = false; + message = "Assertion failed"; + }; +} diff --git a/lib/tests/modules/assertions/submodule-attrsOf-attrsOf.nix b/lib/tests/modules/assertions/submodule-attrsOf-attrsOf.nix new file mode 100644 index 00000000000..27a63d1e432 --- /dev/null +++ b/lib/tests/modules/assertions/submodule-attrsOf-attrsOf.nix @@ -0,0 +1,13 @@ +{ lib, ... }: { + + options.foo = lib.mkOption { + default = { bar.baz = {}; }; + type = lib.types.attrsOf (lib.types.attrsOf (lib.types.submodule { + _module.checks.test = { + check = false; + message = "Assertion failed"; + }; + })); + }; + +} diff --git a/lib/tests/modules/assertions/submodule-attrsOf.nix b/lib/tests/modules/assertions/submodule-attrsOf.nix new file mode 100644 index 00000000000..aac5937cf7e --- /dev/null +++ b/lib/tests/modules/assertions/submodule-attrsOf.nix @@ -0,0 +1,13 @@ +{ lib, ... }: { + + options.foo = lib.mkOption { + default = { bar = {}; }; + type = lib.types.attrsOf (lib.types.submodule { + _module.checks.test = { + check = false; + message = "Assertion failed"; + }; + }); + }; + +} diff --git a/lib/tests/modules/assertions/submodule.nix b/lib/tests/modules/assertions/submodule.nix new file mode 100644 index 00000000000..4e7e0b1bd61 --- /dev/null +++ b/lib/tests/modules/assertions/submodule.nix @@ -0,0 +1,13 @@ +{ lib, ... }: { + + options.foo = lib.mkOption { + default = {}; + type = lib.types.submodule { + _module.checks.test = { + check = false; + message = "Assertion failed"; + }; + }; + }; + +} diff --git a/lib/tests/modules/assertions/underscore-attributes.nix b/lib/tests/modules/assertions/underscore-attributes.nix new file mode 100644 index 00000000000..f9ee5c5787b --- /dev/null +++ b/lib/tests/modules/assertions/underscore-attributes.nix @@ -0,0 +1,8 @@ +{ + + _module.checks._test = { + check = false; + message = "Assertion failed"; + }; + +} diff --git a/lib/tests/modules/assertions/warning.nix b/lib/tests/modules/assertions/warning.nix new file mode 100644 index 00000000000..72598ba3fdd --- /dev/null +++ b/lib/tests/modules/assertions/warning.nix @@ -0,0 +1,9 @@ +{ + + _module.checks.test = { + check = false; + type = "warning"; + message = "Warning message"; + }; + +} |