Skip to content

Commit b013442

Browse files
infinisilinfinisil
committed
lib/modules: Implement module-builtin assertions
lib/options: Don't show internal suboption in the manual Initially NixOS#82897 prevented non-visible options from being rendered in the manual, but visible-but-internal options were still being recursed into. This fixes this, aligning the recurse condition here with the one in make-options-doc/default.nix lib/modules: Implement module-builtin assertions This implements assertions/warnings supported by the module system directly, instead of just being a NixOS option (see nixos/modules/misc/assertions.nix). This has the following benefits: - It allows cleanly redoing the user interface. The new implementation specifically allows disabling assertions or converting them to warnings instead. - Assertions/warnings can now be thrown easily from within submodules, which previously wasn't possible and needed workarounds. nixos/assertions: Use module-builtin assertion implementation lib/modules: Use module-builtin assertions for mkRemovedOptionModule and co. nixos/modules: Allow options to be coerced to a string for convenience nixos/modules: Expose the internal module in the top-level documentation nixos/docs: Update assertion docs for new module-builtin ones lib/tests: Implement generalized checkConfigCodeOutErr for module tests lib/tests: Add tests for module-builtin assertions lib/modules: Rename _module.assertions to _module.checks 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. lib/modules: _module.check should always be internal Honestly this option should probably just be removed lib/modules: Introduce _module.checks.*.check Previously the .enable option was used to encode the condition as well, which lead to some oddness: - In order to encode an assertion, one had to invert it - To disable a check, one had to mkForce it By introducing a separate .check option this is solved because: - It can be used to encode assertions - Disabling is done separately with .enable option, whose default can be overridden without a mkForce lib/modules: Prefix mkRemovedOptionModule & co. check names To avoid name clashes Co-authored-by: Robert Hensing <robert@roberthensing.nl> bla
1 parent ad267cc commit b013442

16 files changed

Lines changed: 473 additions & 50 deletions

File tree

‎lib/modules.nix‎

Lines changed: 139 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ let
4646
showFiles
4747
showOption
4848
unknownModule
49+
literalExample
4950
;
5051
in
5152

@@ -118,18 +119,24 @@ rec {
118119
};
119120
regularModules = modules ++ legacyModules;
120121

121-
# This internal module declare internal options under the `_module'
122-
# attribute. These options are fragile, as they are used by the
123-
# module system to change the interpretation of modules.
122+
# An internal module that's always added, defining special options which
123+
# change the behavior of the module evaluation itself. This is under a
124+
# `_`-prefixed namespace in order to prevent name clashes with
125+
# user-defined options
124126
#
125127
# When extended with extendModules or moduleType, a fresh instance of
126128
# this module is used, to avoid conflicts and allow chaining of
127129
# extendModules.
128130
internalModule = rec {
129-
_file = ./modules.nix;
131+
# FIXME: Using ./modules.nix directly breaks the doc for some reason
132+
_file = "lib/modules.nix";
130133

131134
key = _file;
132135

136+
# Most of these options are set to be internal only for prefix != [],
137+
# aka it's a submodule evaluation. This way their docs are displayed
138+
# only once as a top-level NixOS option, but will be hidden for all
139+
# submodules, even though they are available there too
133140
options = {
134141
_module.args = mkOption {
135142
# Because things like `mkIf` are entirely useless for
@@ -139,20 +146,25 @@ rec {
139146
# a `_module.args.pkgs = import (fetchTarball { ... }) {}` won't
140147
# start a download when `pkgs` wasn't evaluated.
141148
type = types.lazyAttrsOf types.raw;
142-
internal = true;
149+
internal = prefix != [];
143150
description = "Arguments passed to each module.";
144151
};
145152

146153
_module.check = mkOption {
147154
type = types.bool;
148155
internal = true;
149156
default = true;
150-
description = "Whether to check whether all option definitions have matching declarations.";
157+
description = ''
158+
Whether to check whether all option definitions have matching
159+
declarations.
160+
Note that this has nothing to do with the similarly named
161+
<option>_module.checks</option> option
162+
'';
151163
};
152164

153165
_module.freeformType = mkOption {
154166
type = types.nullOr types.optionType;
155-
internal = true;
167+
internal = prefix != [];
156168
default = null;
157169
description = ''
158170
If set, merge all definitions that don't have an associated option
@@ -165,6 +177,75 @@ rec {
165177
turned off.
166178
'';
167179
};
180+
181+
_module.checks = mkOption {
182+
description = ''
183+
Evaluation checks to trigger during module evaluation. The
184+
attribute name will be displayed when it is triggered, allowing
185+
users to disable/change these checks if necessary. See
186+
the section on Warnings and Assertions in the manual for more
187+
information.
188+
'';
189+
example = literalExample ''
190+
{
191+
gpgSshAgent = {
192+
enable = config.programs.gnupg.agent.enableSSHSupport && config.programs.ssh.startAgent;
193+
message = "You can't use ssh-agent and GnuPG agent with SSH support enabled at the same time!";
194+
};
195+
196+
grafanaPassword = {
197+
enable = config.services.grafana.database.password != "";
198+
message = "Grafana passwords will be stored as plaintext in the Nix store!";
199+
type = "warning";
200+
};
201+
}
202+
'';
203+
default = {};
204+
internal = prefix != [];
205+
type = types.attrsOf (types.submodule {
206+
options.enable = mkOption {
207+
description = ''
208+
Whether to enable this check. Set this to false to not trigger
209+
any errors or warning messages. This is useful for ignoring a
210+
check in case it doesn't make sense in certain scenarios.
211+
'';
212+
default = true;
213+
type = types.bool;
214+
};
215+
216+
options.check = mkOption {
217+
description = ''
218+
The condition that must succeed in order for this check to be
219+
successful and not trigger a warning or error.
220+
'';
221+
readOnly = true;
222+
type = types.bool;
223+
};
224+
225+
options.type = mkOption {
226+
description = ''
227+
The type of the check. The default
228+
<literal>"error"</literal> type will cause evaluation to fail,
229+
while the <literal>"warning"</literal> type will only show a
230+
warning.
231+
'';
232+
type = types.enum [ "error" "warning" ];
233+
default = "error";
234+
example = "warning";
235+
};
236+
237+
options.message = mkOption {
238+
description = ''
239+
The message to display if this check triggers.
240+
To display option names in the message, add
241+
<literal>options</literal> to the module function arguments
242+
and use <literal>''${options.path.to.option}</literal>.
243+
'';
244+
type = types.str;
245+
example = "Enabling both \${options.services.foo.enable} and \${options.services.bar.enable} is not possible.";
246+
};
247+
});
248+
};
168249
};
169250

170251
config = {
@@ -206,6 +287,35 @@ rec {
206287
# paths, meaning recursiveUpdate will never override any value
207288
else recursiveUpdate freeformConfig declaredConfig;
208289

290+
# Triggers all checks defined by _module.checks before returning its argument
291+
triggerChecks = let
292+
293+
handleCheck = errors: name:
294+
let
295+
value = config._module.checks.${name};
296+
show =
297+
# Assertions with a _ prefix aren't meant to be configurable
298+
if lib.hasPrefix "_" name then value.message
299+
else "[${showOption prefix}${optionalString (prefix != []) "/"}${name}] ${value.message}";
300+
in
301+
if value.enable -> value.check then errors
302+
else if value.type == "warning" then lib.warn show errors
303+
else if value.type == "error" then errors ++ [ show ]
304+
else abort "Unknown check type ${value.type}";
305+
306+
errors = lib.foldl' handleCheck [] (lib.attrNames config._module.checks);
307+
308+
errorMessage = ''
309+
Failed checks:
310+
${lib.concatMapStringsSep "\n" (a: "- ${a}") errors}
311+
'';
312+
313+
trigger = if errors == [] then null else throw errorMessage;
314+
315+
in builtins.seq trigger;
316+
317+
finalConfig = triggerChecks (removeAttrs config [ "_module" ]);
318+
209319
checkUnmatched =
210320
if config._module.check && config._module.freeformType == null && merged.unmatchedDefns != [] then
211321
let
@@ -253,7 +363,7 @@ rec {
253363

254364
result = withWarnings {
255365
options = checked options;
256-
config = checked (removeAttrs config [ "_module" ]);
366+
config = checked finalConfig;
257367
_module = checked (config._module);
258368
inherit extendModules type;
259369
};
@@ -874,14 +984,15 @@ rec {
874984
visible = false;
875985
apply = x: throw "The option `${showOption optionName}' can no longer be used since it's been removed. ${replacementInstructions}";
876986
});
877-
config.assertions =
878-
let opt = getAttrFromPath optionName options; in [{
879-
assertion = !opt.isDefined;
987+
config._module.checks =
988+
let opt = getAttrFromPath optionName options; in {
989+
${"removed-" + showOption optionName} = lib.mkIf opt.isDefined {
880990
message = ''
881991
The option definition `${showOption optionName}' in ${showFiles opt.files} no longer has any effect; please remove it.
882992
${replacementInstructions}
883993
'';
884-
}];
994+
};
995+
};
885996
};
886997

887998
/* Return a module that causes a warning to be shown if the
@@ -942,14 +1053,18 @@ rec {
9421053
})) from);
9431054

9441055
config = {
945-
warnings = filter (x: x != "") (map (f:
946-
let val = getAttrFromPath f config;
947-
opt = getAttrFromPath f options;
948-
in
949-
optionalString
950-
(val != "_mkMergedOptionModule")
951-
"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."
952-
) from);
1056+
_module.checks =
1057+
let warningMessages = map (f:
1058+
let val = getAttrFromPath f config;
1059+
opt = getAttrFromPath f options;
1060+
in {
1061+
${"merged" + showOption f} = lib.mkIf (val != "_mkMergedOptionModule") {
1062+
type = "warning";
1063+
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.";
1064+
};
1065+
}
1066+
) from;
1067+
in mkMerge warningMessages;
9531068
} // setAttrByPath to (mkMerge
9541069
(optional
9551070
(any (f: (getAttrFromPath f config) != "_mkMergedOptionModule") from)
@@ -1028,8 +1143,10 @@ rec {
10281143
});
10291144
config = mkMerge [
10301145
{
1031-
warnings = optional (warn && fromOpt.isDefined)
1032-
"The option `${showOption from}' defined in ${showFiles fromOpt.files} has been renamed to `${showOption to}'.";
1146+
_module.checks.${"renamed-" + showOption from} = mkIf (warn && fromOpt.isDefined) {
1147+
type = "warning";
1148+
message = "The option `${showOption from}' defined in ${showFiles fromOpt.files} has been renamed to `${showOption to}'.";
1149+
};
10331150
}
10341151
(if withPriority
10351152
then mkAliasAndWrapDefsWithPriority (setAttrByPath to) fromOpt

‎lib/options.nix‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ rec {
241241
subOptions =
242242
let ss = opt.type.getSubOptions opt.loc;
243243
in if ss != {} then optionAttrSetToDocList' opt.loc ss else [];
244-
subOptionsVisible = docOption.visible && opt.visible or null != "shallow";
244+
subOptionsVisible = docOption.visible && opt.visible or null != "shallow" && ! docOption.internal;
245245
in
246246
[ docOption ] ++ optionals subOptionsVisible subOptions) (collect isOption options);
247247

‎lib/tests/misc.nix‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -689,7 +689,7 @@ runTests {
689689
modules = [ module ];
690690
}).options;
691691

692-
locs = filter (o: ! o.internal) (optionAttrSetToDocList options);
692+
locs = filter (o: ! o.internal) (optionAttrSetToDocList (removeAttrs options [ "_module" ]));
693693
in map (o: o.loc) locs;
694694
expected = [ [ "foo" ] [ "foo" "<name>" "bar" ] [ "foo" "bar" ] ];
695695
};

‎lib/tests/modules.sh‎

Lines changed: 58 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -30,32 +30,51 @@ reportFailure() {
3030
((++fail))
3131
}
3232

33-
checkConfigOutput() {
33+
checkConfigCodeOutErr() {
34+
local expectedExit=$1
35+
shift
3436
local outputContains=$1
3537
shift
36-
if evalConfig "$@" 2>/dev/null | grep --silent "$outputContains" ; then
37-
((++pass))
38-
else
38+
local errorContains=$1
39+
shift
40+
local tmp=$(mktemp -d)
41+
set +o errexit
42+
evalConfig "$@" 1>"$tmp/out" 2>"$tmp/err"
43+
local exitCode=$?
44+
set -o errexit
45+
if [[ "$exitCode" -ne "$expectedExit" ]]; then
46+
echo 2>&1 "error: Expected exit code $expectedExit, while evaluating"
47+
reportFailure "$@"
48+
return
49+
fi
50+
51+
if [[ -n "$outputContains" ]] && ! grep --silent "$outputContains" "$tmp/out"; then
3952
echo 2>&1 "error: Expected result matching '$outputContains', while evaluating"
4053
reportFailure "$@"
54+
return
4155
fi
56+
57+
if [[ -n "$errorContains" ]] && ! grep -zP --silent "$errorContains" "$tmp/err"; then
58+
echo 2>&1 "error: Expected error matching '$errorContains', while evaluating"
59+
reportFailure "$@"
60+
return
61+
fi
62+
63+
((++pass))
64+
65+
rm -rf "$tmp"
66+
}
67+
68+
checkConfigOutput() {
69+
local outputContains=$1
70+
shift
71+
checkConfigCodeOutErr 0 "$outputContains" "" "$@"
4272
}
4373

4474
checkConfigError() {
4575
local errorContains=$1
46-
local err=""
4776
shift
48-
if err="$(evalConfig "$@" 2>&1 >/dev/null)"; then
49-
echo 2>&1 "error: Expected error code, got exit code 0, while evaluating"
50-
reportFailure "$@"
51-
else
52-
if echo "$err" | grep -zP --silent "$errorContains" ; then
53-
((++pass))
54-
else
55-
echo 2>&1 "error: Expected error matching '$errorContains', while evaluating"
56-
reportFailure "$@"
57-
fi
58-
fi
77+
checkConfigCodeOutErr 1 "" "$errorContains" "$@"
5978
}
6079

6180
# Check boolean option.
@@ -311,6 +330,29 @@ checkConfigOutput '^"hello"$' config.theOption.str ./optionTypeMerging.nix
311330
# Test that types.optionType correctly annotates option locations
312331
checkConfigError 'The option .theOption.nested. in .other.nix. is already declared in .optionTypeFile.nix.' config.theOption.nested ./optionTypeFile.nix
313332

333+
## Module assertions
334+
# Check that assertions are triggered by default for just evaluating config
335+
checkConfigError 'Failed checks:\n - \[test\] Assertion failed' config ./assertions/simple.nix
336+
337+
# Assertion is not triggered when enable is false or condition is true
338+
checkConfigOutput '{ }' config ./assertions/condition-true.nix
339+
checkConfigOutput '{ }' config ./assertions/enable-false.nix
340+
341+
# Warnings should be displayed on standard error
342+
checkConfigCodeOutErr 0 '{ }' 'warning: \[test\] Warning message' config ./assertions/warning.nix
343+
344+
# Check that multiple assertions and warnings can be triggered at once
345+
checkConfigError 'Failed checks:\n - \[test1\] Assertion 1 failed\n - \[test2\] Assertion 2 failed' config ./assertions/multi.nix
346+
checkConfigError 'trace: warning: \[test3\] Warning 3 failed\ntrace: warning: \[test4\] Warning 4 failed' config ./assertions/multi.nix
347+
348+
# Submodules should be able to trigger assertions and display the submodule prefix in their error
349+
checkConfigError 'Failed checks:\n - \[foo/test\] Assertion failed' config.foo ./assertions/submodule.nix
350+
checkConfigError 'Failed checks:\n - \[foo.bar/test\] Assertion failed' config.foo.bar ./assertions/submodule-attrsOf.nix
351+
checkConfigError 'Failed checks:\n - \[foo.bar.baz/test\] Assertion failed' config.foo.bar.baz ./assertions/submodule-attrsOf-attrsOf.nix
352+
353+
# Assertions with an attribute starting with _ shouldn't have their name displayed
354+
checkConfigError 'Failed checks:\n - Assertion failed' config ./assertions/underscore-attributes.nix
355+
314356
cat <<EOF
315357
====== module tests ======
316358
$pass Pass

‎lib/tests/modules/assertions/condition-true.nix‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
3+
_module.checks.test = {
4+
check = true;
5+
message = "Assertion failed";
6+
};
7+
8+
}

‎lib/tests/modules/assertions/enable-false.nix‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
{
2+
3+
_module.checks.test = {
4+
enable = false;
5+
check = false;
6+
message = "Assertion failed";
7+
};
8+
9+
}

0 commit comments

Comments
 (0)