lib/modules: Remove _module.checks.*.triggerPath as it's not necessary

Previously this option was thought to be necessary to avoid infinite
recursion, but it actually isn't, since the check evaluation isn't fed
back into the module fixed-point.
This commit is contained in:
Silvan Mosberger 2020-11-30 21:42:52 +01:00
parent c9cc8969b4
commit 8dea4df903
No known key found for this signature in database
GPG key ID: E8F1E9EAD284E17D
7 changed files with 22 additions and 165 deletions

View file

@ -192,32 +192,6 @@ rec {
Enabling both ''${options.services.foo.enable} and ''${options.services.bar.enable} is not possible.
'';
};
options.triggerPath = mkOption {
description = ''
The <literal>config</literal> path which when evaluated should
trigger this check. By default this is
<literal>[]</literal>, meaning evaluating
<literal>config</literal> at all will trigger the check.
On NixOS this default is changed to
<literal>[ "system" "build" "toplevel"</literal> such that
only a system evaluation triggers the checks.
<warning><para>
Evaluating <literal>config</literal> from within the current
module evaluation doesn't cause a trigger. Only accessing it
from outside will do that. This means it's easy to miss
failing checks if this option doesn't have an externally-accessed
value.
</para></warning>
'';
# Mark as internal as it's easy to misuse it
internal = true;
type = types.uniq (types.listOf types.str);
# Default to [], causing checks to be triggered when
# anything is evaluated. This is a safe and convenient default.
default = [];
example = [ "system" "build" "vm" ];
};
});
};
};
@ -258,63 +232,34 @@ rec {
# paths, meaning recursiveUpdate will never override any value
else recursiveUpdate freeformConfig declaredConfig;
/*
Inject a list of checks into a config value, corresponding to their
triggerPath (meaning when that path is accessed from the result of this
function, the check triggers).
*/
injectChecks = checks: config: let
# Partition into checks that are triggered on this level and ones that aren't
parted = lib.partition (a: length a.triggerPath == 0) checks;
# Triggers all checks defined by _module.checks before returning its argument
triggerChecks = let
# From the ones that are triggered, filter out ones that aren't enabled
# and group into warnings/errors
byType = lib.groupBy (a: a.type) (filter (a: a.enable) parted.right);
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 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}";
# Triggers semantically are just lib.id, but they print warning cause errors in addition
warningTrigger = value: lib.foldr (w: warn w.show) value (byType.warning or []);
errorTrigger = value:
if byType.error or [] == [] then value else
throw ''
Failed checks:
${lib.concatMapStringsSep "\n" (a: "- ${a.show}") byType.error}
'';
# Trigger for both warnings and errors
trigger = value: warningTrigger (errorTrigger value);
errors = lib.foldl' handleCheck [] (lib.attrNames config._module.checks);
# From the non-triggered checks, split off the first element of triggerPath
# to get a mapping from nested attributes to a list of checks for that attribute
nested = lib.zipAttrs (map (a: {
${head a.triggerPath} = a // {
triggerPath = lib.tail a.triggerPath;
};
}) parted.wrong);
errorMessage = ''
Failed checks:
${lib.concatMapStringsSep "\n" (a: "- ${a}") errors}
'';
# Recursively inject checks if config is an attribute set and we
# have checks under its attributes
result =
if isAttrs config
then
mapAttrs (name: value:
if nested ? ${name}
then injectChecks nested.${name} value
else value
) config
else config;
in trigger result;
trigger = if errors == [] then null else throw errorMessage;
# List of checks for this module evaluation, where each check also
# has a `show` attribute for how to show it if triggered
checks = mapAttrsToList (name: value:
let id =
if lib.hasPrefix "_" name then ""
else "[${showOption prefix}${optionalString (prefix != []) "/"}${name}] ";
in value // {
show = "${id}${value.message}";
}
) config._module.checks;
in builtins.seq trigger;
finalConfig = injectChecks checks (removeAttrs config [ "_module" ]);
finalConfig = triggerChecks (removeAttrs config [ "_module" ]);
checkUnmatched =
if config._module.check && config._module.freeformType == null && merged.unmatchedDefns != [] then

View file

@ -278,13 +278,6 @@ 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
# Check that assertions are only triggered if they have a triggerPath that's evaluated
checkConfigError 'Failed checks:\n- \[test\] Assertion failed' config.foo ./assertions/trigger-lazy.nix
checkConfigOutput true config.bar ./assertions/trigger-lazy.nix
# The assertions enable condition should only be evaluated if the trigger is evaluated
checkConfigError 'enable evaluated' config.foo ./assertions/enable-lazy.nix
checkConfigOutput true config.bar ./assertions/enable-lazy.nix
# Assertion is not triggered when enable is false
checkConfigOutput '{ }' config ./assertions/enable-false.nix
@ -292,10 +285,6 @@ checkConfigOutput '{ }' config ./assertions/enable-false.nix
# Warnings should be displayed on standard error
checkConfigCodeOutErr 0 '{ }' 'warning: \[test\] Warning message' config ./assertions/warning.nix
# A triggerPath can be set to a submodule path
checkConfigOutput '{ baz = <CODE>; }' config.foo.bar ./assertions/trigger-submodule.nix
checkConfigError 'Failed checks:\n- \[test\] Assertion failed' config.foo.bar.baz ./assertions/trigger-submodule.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
@ -305,10 +294,6 @@ checkConfigError 'Failed checks:\n- \[foo/test\] Assertion failed' config.foo ./
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 aren't triggered when the trigger path is only evaluated from within the same module evaluation
# This behavior is necessary to allow assertions to depend on config values. This could potentially be changed in the future if all of NixOS' assertions are rewritten to not depend on any config values
checkConfigOutput true config.bar ./assertions/non-cascading.nix
# Assertions with an attribute starting with _ shouldn't have their name displayed
checkConfigError 'Failed checks:\n- Assertion failed' config ./assertions/underscore-attributes.nix

View file

@ -1,17 +0,0 @@
{ lib, ... }: {
options.foo = lib.mkOption {
default = true;
};
options.bar = lib.mkOption {
default = true;
};
config._module.checks.test = {
enable = throw "enable evaluated";
message = "Assertion failed";
triggerPath = [ "foo" ];
};
}

View file

@ -1,17 +0,0 @@
{ lib, config, ... }: {
options.foo = lib.mkOption {
default = true;
};
options.bar = lib.mkOption {
default = config.foo;
};
config._module.checks.foo = {
enable = true;
message = "Foo assertion";
triggerPath = [ "foo" ];
};
}

View file

@ -1,15 +0,0 @@
{ lib, ... }: {
options.foo = lib.mkOption {
default = true;
};
options.bar = lib.mkOption {
default = true;
};
config._module.checks.test = {
enable = true;
message = "Assertion failed";
triggerPath = [ "foo" ];
};
}

View file

@ -1,18 +0,0 @@
{ lib, ... }: {
options.foo = lib.mkOption {
default = { bar = {}; };
type = lib.types.attrsOf (lib.types.submodule {
options.baz = lib.mkOption {
default = true;
};
});
};
config._module.checks.test = {
enable = true;
message = "Assertion failed";
triggerPath = [ "foo" "bar" "baz" ];
};
}

View file

@ -29,12 +29,6 @@ with lib;
'';
};
_module.checks = mkOption {
type = types.attrsOf (types.submodule {
triggerPath = mkDefault [ "system" "build" "toplevel" ];
});
};
};
config._module.checks = lib.listToAttrs (lib.imap1 (n: value: