JakeHillion/nixpkgs
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Revert "Module-builtin assertions, disabling assertions and submodule assertions"

Browse Source
This commit is contained in:
Silvan Mosberger 2020-12-18 16:42:42 +01:00
parent fd1cc29974
commit 9e6737710c
16 changed files with 96 additions and 442 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

164
lib/modules.nix
View File

@ -46,7 +46,6 @@ let
showFiles
showOption
unknownModule
literalExample
;
in
@ -73,20 +72,14 @@ rec {
check ? true
}:
let
# 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
# 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.
internalModule = rec {
# FIXME: Using ./modules.nix directly breaks the doc for some reason
_file = "lib/modules.nix";
_file = ./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
@ -96,7 +89,7 @@ rec {
# a `_module.args.pkgs = import (fetchTarball { ... }) {}` won't
# start a download when `pkgs` wasn't evaluated.
type = types.lazyAttrsOf types.unspecified;
internal = prefix != [];
internal = true;
description = "Arguments passed to each module.";
};
@ -104,19 +97,13 @@ rec {
type = types.bool;
internal = true;
default = check;
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
'';
description = "Whether to check whether all option definitions have matching declarations.";
};
_module.freeformType = mkOption {
# Disallow merging for now, but could be implemented nicely with a `types.optionType`
type = types.nullOr (types.uniq types.attrs);
internal = prefix != [];
internal = true;
default = null;
description = ''
If set, merge all definitions that don't have an associated option
@ -129,75 +116,6 @@ 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 = {
@ -236,35 +154,6 @@ 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
@ -284,7 +173,7 @@ rec {
result = builtins.seq checkUnmatched {
inherit options;
config = finalConfig;
config = removeAttrs config [ "_module" ];
inherit (config) _module;
};
in result;
@ -625,8 +514,6 @@ 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.
@ -886,15 +773,14 @@ rec {
visible = false;
apply = x: throw "The option `${showOption optionName}' can no longer be used since it's been removed. ${replacementInstructions}";
});
config._module.checks =
let opt = getAttrFromPath optionName options; in {
${"removed-" + showOption optionName} = lib.mkIf opt.isDefined {
config.assertions =
let opt = getAttrFromPath optionName options; in [{
assertion = !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
@ -955,18 +841,14 @@ rec {
})) from);
config = {
_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;
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);
} // setAttrByPath to (mkMerge
(optional
(any (f: (getAttrFromPath f config) != "_mkMergedOptionModule") from)
@ -1025,10 +907,8 @@ rec {
});
config = mkMerge [
{
_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}'.";
};
warnings = optional (warn && fromOpt.isDefined)
"The option `${showOption from}' defined in ${showFiles fromOpt.files} has been renamed to `${showOption to}'.";
}
(if withPriority
then mkAliasAndWrapDefsWithPriority (setAttrByPath to) fromOpt

2
lib/options.nix
View File

@ -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 && ! docOption.internal) subOptions) (collect isOption options);
[ docOption ] ++ optionals docOption.visible subOptions) (collect isOption options);
/* This function recursively removes all derivation attributes from

2
lib/tests/misc.nix
View File

@ -655,7 +655,7 @@ runTests {
modules = [ module ];
}).options;
locs = filter (o: ! o.internal) (optionAttrSetToDocList (removeAttrs options [ "_module" ]));
locs = filter (o: ! o.internal) (optionAttrSetToDocList options);
in map (o: o.loc) locs;
expected = [ [ "foo" ] [ "foo" "<name>" "bar" ] [ "foo" "bar" ] ];
};

82
lib/tests/modules.sh
View File

@ -27,50 +27,37 @@ reportFailure() {
fail=$((fail + 1))
}
checkConfigCodeOutErr() {
local expectedExit=$1
shift;
local outputContains=$1
shift;
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" "" "$@"
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"
reportFailure "$@"
return 1
fi
}
checkConfigError() {
local errorContains=$1
local err=""
shift;
checkConfigCodeOutErr 1 "" "$errorContains" "$@"
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
}
# Check boolean option.
@ -275,29 +262,6 @@ 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

8
lib/tests/modules/assertions/condition-true.nix
View File

@ -1,8 +0,0 @@
{
_module.checks.test = {
check = true;
message = "Assertion failed";
};
}

9
lib/tests/modules/assertions/enable-false.nix
View File

@ -1,9 +0,0 @@
{
_module.checks.test = {
enable = false;
check = false;
message = "Assertion failed";
};
}

23
lib/tests/modules/assertions/multi.nix
View File

@ -1,23 +0,0 @@
{
_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";
};
};
}

6
lib/tests/modules/assertions/simple.nix
View File

@ -1,6 +0,0 @@
{
_module.checks.test = {
check = false;
message = "Assertion failed";
};
}

13
lib/tests/modules/assertions/submodule-attrsOf-attrsOf.nix
View File

@ -1,13 +0,0 @@
{ 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";
};
}));
};
}

13
lib/tests/modules/assertions/submodule-attrsOf.nix
View File

@ -1,13 +0,0 @@
{ lib, ... }: {
options.foo = lib.mkOption {
default = { bar = {}; };
type = lib.types.attrsOf (lib.types.submodule {
_module.checks.test = {
check = false;
message = "Assertion failed";
};
});
};
}

13
lib/tests/modules/assertions/submodule.nix
View File

@ -1,13 +0,0 @@
{ lib, ... }: {
options.foo = lib.mkOption {
default = {};
type = lib.types.submodule {
_module.checks.test = {
check = false;
message = "Assertion failed";
};
};
};
}

8
lib/tests/modules/assertions/underscore-attributes.nix
View File

@ -1,8 +0,0 @@
{
_module.checks._test = {
check = false;
message = "Assertion failed";
};
}

9
lib/tests/modules/assertions/warning.nix
View File

@ -1,9 +0,0 @@
{
_module.checks.test = {
check = false;
type = "warning";
message = "Warning message";
};
}

161
nixos/doc/manual/development/assertions.xml
View File

@ -3,155 +3,72 @@
xmlns:xi="http://www.w3.org/2001/XInclude"
version="5.0"
xml:id="sec-assertions">
<title>Evaluation Checks</title>
<title>Warnings and Assertions</title>
<para>
When configuration problems are detectable in a module, it is a good idea to
write a check for catching it early. Doing so can provide clear feedback to
the user and can prevent errors before the build.
write an assertion or warning. Doing so provides clear feedback to the user
and prevents errors after the build.
</para>
<para>
Although Nix has the <literal>abort</literal> and
<literal>builtins.trace</literal>
<link xlink:href="https://nixos.org/nix/manual/#ssec-builtins">functions</link>
to perform such tasks generally, they are not ideally suited for NixOS
modules. Instead of these functions, you can declare your evaluation checks
using the NixOS module system.
to perform such tasks, they are not ideally suited for NixOS modules. Instead
of these functions, you can declare your warnings and assertions using the
NixOS module system.
</para>
<section xml:id="sec-assertions-define">
<title>Defining Checks</title>
<section xml:id="sec-assertions-warnings">
<title>Warnings</title>
<para>
Checks can be defined using the <xref linkend="opt-_module.checks"/> option.
Each check needs an attribute name, under which you can define a trigger
assertion using <xref linkend="opt-_module.checks._name_.check"/> and a
message using <xref linkend="opt-_module.checks._name_.message"/>.
For the message, you can add
<literal>options</literal> to the module arguments and use
<literal>${options.path.to.option}</literal> to print a context-aware string
representation of an option path. Here is an example showing how this can be
done.
This is an example of using <literal>warnings</literal>.
</para>
<programlisting>
{ config, options, ... }: {
_module.checks.gpgSshAgent = {
check = config.programs.gnupg.agent.enableSSHSupport -> !config.programs.ssh.startAgent;
message = "If you have ${options.programs.gnupg.agent.enableSSHSupport} enabled,"
+ " you can't enable ${options.programs.ssh.startAgent} as well!";
};
_module.checks.grafanaPassword = {
check = config.services.grafana.database.password == "";
message = "The grafana password defined with ${options.services.grafana.database.password}"
+ " will be stored as plaintext in the Nix store!";
# This is a non-fatal warning
type = "warning";
};
<![CDATA[
{ config, lib, ... }:
{
config = lib.mkIf config.services.foo.enable {
warnings =
if config.services.foo.bar
then [ ''You have enabled the bar feature of the foo service.
This is known to cause some specific problems in certain situations.
'' ]
else [];
}
}
]]>
</programlisting>
</section>
<section xml:id="sec-assertions-ignoring">
<title>Ignoring Checks</title>
<section xml:id="sec-assertions-assertions">
<title>Assertions</title>
<para>
Sometimes you can get failing checks that don't apply to your specific case
and you wish to ignore them, or at least make errors non-fatal. You can do so
for all checks defined using <xref linkend="opt-_module.checks"/> by
using the attribute name of the definition, which is conveniently printed
using <literal>[...]</literal> when the check is triggered. For above
example, the evaluation output when the checks are triggered looks as
follows:
</para>
<programlisting>
trace: warning: [grafanaPassword] The grafana password defined with
services.grafana.database.password will be stored as plaintext in the Nix store!
error: Failed checks:
- [gpgSshAgent] If you have programs.gnupg.agent.enableSSHSupport
enabled, you can't enable programs.ssh.startAgent as well!
</programlisting>
<para>
The <literal>[grafanaPassword]</literal> and <literal>[gpgSshAgent]</literal>
strings tell you that these were defined under the <literal>grafanaPassword
</literal> and <literal>gpgSshAgent</literal> attributes of
<xref linkend="opt-_module.checks"/> respectively. With this knowledge
you can adjust them to your liking:
This example, extracted from the
<link xlink:href="https://github.com/NixOS/nixpkgs/blob/release-17.09/nixos/modules/services/logging/syslogd.nix">
<literal>syslogd</literal> module </link> shows how to use
<literal>assertions</literal>. Since there can only be one active syslog
daemon at a time, an assertion is useful to prevent such a broken system
from being built.
</para>
<programlisting>
<![CDATA[
{ config, lib, ... }:
{
# Change the error into a non-fatal warning
_module.checks.gpgSshAgent.type = "warning";
# We don't care about this warning, disable it
_module.checks.grafanaPassword.enable = false;
config = lib.mkIf config.services.syslogd.enable {
assertions =
[ { assertion = !config.services.rsyslogd.enable;
message = "rsyslogd conflicts with syslogd";
}
];
}
}
]]>
</programlisting>
</section>
<section xml:id="sec-assertions-submodules">
<title>Checks in Submodules</title>
<para>
Evaluation checks can be defined within submodules in the same way. Here is an example:
</para>
<programlisting>
{ lib, ... }: {
options.myServices = lib.mkOption {
type = lib.types.attrsOf (lib.types.submodule ({ config, options, ... }: {
options.port = lib.mkOption {};
config._module.checks.portConflict = {
check = config.port != 80;
message = "Port ${toString config.port} defined using"
+ " ${options.port} is usually used for HTTP";
type = "warning";
};
}));
};
}
</programlisting>
<para>
When this check is triggered, it shows both the submodule path along with
the check attribute within that submodule, joined by a
<literal>/</literal>. Note also how <literal>${options.port}</literal>
correctly shows the context of the option.
</para>
<programlisting>
trace: warning: [myServices.foo/portConflict] Port 80 defined using
myServices.foo.port is usually used for HTTP
</programlisting>
<para>
Therefore to disable such a check, you can do so by changing the
<xref linkend="opt-_module.checks"/> option within the
<literal>myServices.foo</literal> submodule:
</para>
<programlisting>
{
myServices.foo._module.checks.portConflict.enable = false;
}
</programlisting>
<note>
<para>
Checks defined in submodules under <literal>types.listOf</literal> can't be
ignored, since there's no way to change previously defined list items.
</para>
</note>
</section>
</section>

15
nixos/modules/misc/assertions.nix
View File

@ -1,4 +1,4 @@
{ lib, config, ... }:
{ lib, ... }:
with lib;
@ -30,18 +30,5 @@ with lib;
};
};
config._module.checks = lib.listToAttrs (lib.imap1 (n: value:
let
name = "_${toString n}";
isWarning = lib.isString value;
result = {
check = if isWarning then false else value.assertion;
type = if isWarning then "warning" else "error";
message = if isWarning then value else value.message;
};
in nameValuePair name result
) (config.assertions ++ config.warnings));
# impl of assertions is in <nixpkgs/nixos/modules/system/activation/top-level.nix>
}

10
nixos/modules/system/activation/top-level.nix
View File

@ -117,10 +117,18 @@ let
perl = "${pkgs.perl}/bin/perl " + (concatMapStringsSep " " (lib: "-I${lib}/${pkgs.perl.libPrefix}") (with pkgs.perlPackages; [ FileSlurp NetDBus XMLParser XMLTwig ]));
};
# Handle assertions and warnings
failedAssertions = map (x: x.message) (filter (x: !x.assertion) config.assertions);
baseSystemAssertWarn = if failedAssertions != []
then throw "\nFailed assertions:\n${concatStringsSep "\n" (map (x: "- ${x}") failedAssertions)}"
else showWarnings config.warnings baseSystem;
# Replace runtime dependencies
system = fold ({ oldDependency, newDependency }: drv:
pkgs.replaceDependency { inherit oldDependency newDependency drv; }
) baseSystem config.system.replaceRuntimeDependencies;
) baseSystemAssertWarn config.system.replaceRuntimeDependencies;
in