From 20d491a317d9956ddca80913f07d04bd234ea432 Mon Sep 17 00:00:00 2001 From: rnhmjoj Date: Sun, 23 Aug 2020 01:28:45 +0200 Subject: treewide: completely remove types.loaOf --- lib/modules.nix | 1 - 1 file changed, 1 deletion(-) (limited to 'lib/modules.nix') diff --git a/lib/modules.nix b/lib/modules.nix index 2ec34699809..decb96ffe11 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -613,7 +613,6 @@ rec { if tp.name == "option set" || tp.name == "submodule" then throw "The option ${showOption loc} uses submodules without a wrapping type, in ${showFiles opt.declarations}." else if optionSetIn "attrsOf" then types.attrsOf (types.submodule options) - else if optionSetIn "loaOf" then types.loaOf (types.submodule options) else if optionSetIn "listOf" then types.listOf (types.submodule options) else if optionSetIn "nullOr" then types.nullOr (types.submodule options) else tp; -- cgit 1.4.1 From 1d4656225d4f1e93ea9801c72eb0b1b0bffa245d Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Fri, 4 Sep 2020 13:39:27 +0200 Subject: lib/types: Allow types to emit a deprecation warning Previously the only way to deprecate a type was using theType = lib.warn "deprecated" (mkOptionType ...) This caused the warning to be emitted when the type was evaluated, but the error didn't include which option actually used that type. With this commit, types can specify a deprecationMessage, which when non-null, is printed along with the option that uses the type --- lib/modules.nix | 6 +++++- lib/types.nix | 5 ++++- 2 files changed, 9 insertions(+), 2 deletions(-) (limited to 'lib/modules.nix') diff --git a/lib/modules.nix b/lib/modules.nix index decb96ffe11..412c7f1df71 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -457,7 +457,11 @@ rec { # yield a value computed from the definitions value = if opt ? apply then opt.apply res.mergedValue else res.mergedValue; - in opt // + warnDeprecation = + if opt.type.deprecationMessage == null then id + else warn "The type `types.${opt.type.name}' of option `${showOption loc}' defined in ${showFiles opt.declarations} is deprecated. ${opt.type.deprecationMessage}"; + + in warnDeprecation opt // { value = builtins.addErrorContext "while evaluating the option `${showOption loc}':" value; inherit (res.defsFinal') highestPrio; definitions = map (def: def.value) res.defsFinal; diff --git a/lib/types.nix b/lib/types.nix index 17e7a939fe3..f999805e1a9 100644 --- a/lib/types.nix +++ b/lib/types.nix @@ -91,9 +91,12 @@ rec { # combinable with the binOp binary operation. # binOp: binary operation that merge two payloads of the same type. functor ? defaultFunctor name + , # The deprecation message to display when this type is used by an option + # If null, the type isn't deprecated + deprecationMessage ? null }: { _type = "option-type"; - inherit name check merge emptyValue getSubOptions getSubModules substSubModules typeMerge functor; + inherit name check merge emptyValue getSubOptions getSubModules substSubModules typeMerge functor deprecationMessage; description = if description == null then name else description; }; -- cgit 1.4.1 From 035627dff23c4524345c4013e5e01ca95597452b Mon Sep 17 00:00:00 2001 From: zimbatm Date: Sat, 12 Sep 2020 16:33:56 +0200 Subject: lib: allow to import JSON and TOML files The vision here is that configuration tools can generate .json or .toml files, which can be plugged into an existing configuration. Eg: { lib, ... }: { imports = [ (lib.modules.importJSON ./hardware-configuration.json) ]; } --- lib/modules.nix | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) (limited to 'lib/modules.nix') diff --git a/lib/modules.nix b/lib/modules.nix index 412c7f1df71..e7012742a98 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -869,4 +869,21 @@ rec { ]; }; + /* Use this function to import a JSON file as NixOS configuration. + + importJSON -> path -> attrs + */ + importJSON = file: { + _file = file; + config = lib.importJSON file; + }; + + /* Use this function to import a TOML file as NixOS configuration. + + importTOML -> path -> attrs + */ + importTOML = file: { + _file = file; + config = lib.importTOML file; + }; } -- cgit 1.4.1 From bdfcee2590b9eca62cfa5c45b7b774846232ee2f Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Wed, 16 Sep 2020 20:05:53 +0200 Subject: lib/modules: Improve error messages using showDefs --- lib/modules.nix | 8 ++++---- lib/options.nix | 12 ++++++------ 2 files changed, 10 insertions(+), 10 deletions(-) (limited to 'lib/modules.nix') diff --git a/lib/modules.nix b/lib/modules.nix index 412c7f1df71..de6fadbcb91 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -117,7 +117,7 @@ rec { if config._module.check && config._module.freeformType == null && merged.unmatchedDefns != [] then let firstDef = head merged.unmatchedDefns; - baseMsg = "The option `${showOption (prefix ++ firstDef.prefix)}' defined in `${firstDef.file}' does not exist."; + baseMsg = "The option `${showOption (prefix ++ firstDef.prefix)}' does not exist. Definition values:${showDefs [ firstDef ]}"; in if attrNames options == [ "_module" ] then throw '' @@ -449,7 +449,7 @@ rec { # Handle properties, check types, and merge everything together. res = if opt.readOnly or false && length defs' > 1 then - throw "The option `${showOption loc}' is read-only, but it's set multiple times." + throw "The option `${showOption loc}' is read-only, but it's set multiple times. Definition values:${showDefs defs'}" else mergeDefinitions loc opt.type defs'; @@ -497,8 +497,8 @@ rec { mergedValue = if isDefined then if all (def: type.check def.value) defsFinal then type.merge loc defsFinal - else let firstInvalid = findFirst (def: ! type.check def.value) null defsFinal; - in throw "The option value `${showOption loc}' in `${firstInvalid.file}' is not of type `${type.description}'." + else let allInvalid = filter (def: ! type.check def.value) defsFinal; + in throw "A definition for option `${showOption loc}' is not of type `${type.description}'. Definition values:${showDefs allInvalid}" else # (nixos-option detects this specific error message and gives it special # handling. If changed here, please change it there too.) diff --git a/lib/options.nix b/lib/options.nix index 73856573a5d..5b7482c8093 100644 --- a/lib/options.nix +++ b/lib/options.nix @@ -96,12 +96,12 @@ rec { else if all isBool list then foldl' lib.or false list else if all isString list then lib.concatStrings list else if all isInt list && all (x: x == head list) list then head list - else throw "Cannot merge definitions of `${showOption loc}' given in ${showFiles (getFiles defs)}."; + else throw "Cannot merge definitions of `${showOption loc}'. Definition values:${showDefs defs}"; mergeOneOption = loc: defs: if defs == [] then abort "This case should never happen." else if length defs != 1 then - throw "The unique option `${showOption loc}' is defined multiple times, in:\n - ${concatStringsSep "\n - " (getFiles defs)}." + throw "The unique option `${showOption loc}' is defined multiple times. Definition values:${showDefs defs}" else (head defs).value; /* "Merge" option definitions by checking that they all have the same value. */ @@ -111,11 +111,11 @@ rec { # This also makes it work for functions, because the foldl' below would try # to compare the first element with itself, which is false for functions else if length defs == 1 then (elemAt defs 0).value - else foldl' (val: def: - if def.value != val then - throw "The option `${showOption loc}' has conflicting definitions, in ${showFiles (getFiles defs)}." + else (foldl' (first: def: + if def.value != first.value then + throw "The option `${showOption loc}' has conflicting definition values:${showDefs [ first def ]}" else - val) (head defs).value defs; + first) (head defs) defs).value; /* Extracts values of all "value" keys of the given list. -- cgit 1.4.1 From 910dfdc41e134474b605ebd1f380e1b74d1a5e40 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Mon, 21 Sep 2020 18:10:06 +0200 Subject: lib/modules: Evaluate single defs for readOnly error If multiple definitions are passed, this evaluates them all as if they were the only one, for a better error message. In particular this won't show module-internal properties like `_type = "override"` and co. --- lib/modules.nix | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'lib/modules.nix') diff --git a/lib/modules.nix b/lib/modules.nix index de6fadbcb91..02a669df659 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -449,7 +449,13 @@ rec { # Handle properties, check types, and merge everything together. res = if opt.readOnly or false && length defs' > 1 then - throw "The option `${showOption loc}' is read-only, but it's set multiple times. Definition values:${showDefs defs'}" + let + # For a better error message, evaluate all readOnly definitions as + # if they were the only definition. + separateDefs = map (def: def // { + value = (mergeDefinitions loc opt.type [ def ]).mergedValue; + }) defs'; + in throw "The option `${showOption loc}' is read-only, but it's set multiple times. Definition values:${showDefs separateDefs}" else mergeDefinitions loc opt.type defs'; -- cgit 1.4.1 From 769eac074045b86fd4a6e4b492959bb514befd3d Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Tue, 29 Sep 2020 22:34:44 +0200 Subject: lib/modules: Make sure to not import module _file's into the store Previously if `_file` was specified by a module: trace: warning: The type `types.string' of option `foo' defined in `/nix/store/yxhm2il5yrb92fldgriw0wyqh2kk9qyc-bug.nix' is deprecated. See https://github.com/NixOS/nixpkgs/pull/66346 for better alternative types. With this change: trace: warning: The type `types.string' of option `foo' defined in `/home/infinisil/src/nixpkgs/bug.nix' is deprecated. See https://github.com/NixOS/nixpkgs/pull/66346 for better alternative types. --- lib/modules.nix | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib/modules.nix') diff --git a/lib/modules.nix b/lib/modules.nix index 02a669df659..5a3a268cafa 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -224,7 +224,7 @@ rec { if badAttrs != {} then throw "Module `${key}' has an unsupported attribute `${head (attrNames badAttrs)}'. This is caused by introducing a top-level `config' or `options' attribute. Add configuration attributes immediately on the top level instead, or move all of them (namely: ${toString (attrNames badAttrs)}) into the explicit `config' attribute." else - { _file = m._file or file; + { _file = toString m._file or file; key = toString m.key or key; disabledModules = m.disabledModules or []; imports = m.imports or []; @@ -232,7 +232,7 @@ rec { config = addFreeformType (addMeta (m.config or {})); } else - { _file = m._file or file; + { _file = toString m._file or file; key = toString m.key or key; disabledModules = m.disabledModules or []; imports = m.require or [] ++ m.imports or []; -- cgit 1.4.1 From afa6c51f27fb86fda71f91a51b093a5fc3de797d Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Tue, 20 Oct 2020 13:47:24 +0200 Subject: lib: Use Nix's static scope checking, fix error message, optimize Nix can perform static scope checking, but whenever code is inside a `with` expression, the analysis breaks down, because it can't know statically what's in the attribute set whose attributes were brought into scope. In those cases, Nix has to assume that everything works out. Except it doesnt. Removing `with` from lib/ revealed an undefined variable in an error message. If that doesn't convince you that we're better off without `with`, I can tell you that this PR results in a 3% evaluation performance improvement because Nix can look up local variables by index. This adds up with applications like the module system. Furthermore, removing `with` makes the binding site of each variable obvious, which helps with comprehension. --- lib/debug.nix | 32 ++++++++++++++++++------ lib/default.nix | 36 +++++++++++++------------- lib/lists.nix | 2 +- lib/modules.nix | 59 +++++++++++++++++++++++++++++++++++++------ lib/options.nix | 14 +++++++---- lib/sources.nix | 40 ++++++++++++++++++++--------- lib/strings-with-deps.nix | 13 +++++++--- lib/strings.nix | 64 +++++++++++++++++++++++++++++++---------------- lib/types.nix | 60 ++++++++++++++++++++++++++++++++++++++++---- 9 files changed, 238 insertions(+), 82 deletions(-) (limited to 'lib/modules.nix') diff --git a/lib/debug.nix b/lib/debug.nix index 2879f72ed2b..ea6aed60ab4 100644 --- a/lib/debug.nix +++ b/lib/debug.nix @@ -14,9 +14,25 @@ */ { lib }: let - inherit (builtins) trace isAttrs isList isInt - head substring attrNames; - inherit (lib) id elem isFunction; + inherit (lib) + isInt + attrNames + isList + isAttrs + substring + addErrorContext + attrValues + concatLists + concatStringsSep + const + elem + generators + head + id + isDerivation + isFunction + mapAttrs + trace; in rec { @@ -94,7 +110,7 @@ rec { trace: { a = { b = {…}; }; } => null */ - traceSeqN = depth: x: y: with lib; + traceSeqN = depth: x: y: let snip = v: if isList v then noQuotes "[…]" v else if isAttrs v then noQuotes "{…}" v else v; @@ -149,7 +165,7 @@ rec { */ runTests = # Tests to run - tests: lib.concatLists (lib.attrValues (lib.mapAttrs (name: test: + tests: concatLists (attrValues (mapAttrs (name: test: let testsToRun = if tests ? tests then tests.tests else []; in if (substring 0 4 name == "test" || elem name testsToRun) && ((testsToRun == []) || elem name tests.tests) @@ -176,9 +192,9 @@ rec { + "and will be removed in the next release. " + "Please use more specific concatenation " + "for your uses (`lib.concat(Map)StringsSep`)." ) - (lib.concatStringsSep "; " (map (x: "${x}=") (attrNames a))); + (concatStringsSep "; " (map (x: "${x}=") (attrNames a))); - showVal = with lib; + showVal = trace ( "Warning: `showVal` is deprecated " + "and will be removed in the next release, " + "please use `traceSeqN`" ) @@ -226,7 +242,7 @@ rec { trace ( "Warning: `addErrorContextToAttrs` is deprecated " + "and will be removed in the next release. " + "Please use `builtins.addErrorContext` directly." ) - (lib.mapAttrs (a: v: lib.addErrorContext "while evaluating ${a}" v) attrs); + (mapAttrs (a: v: addErrorContext "while evaluating ${a}" v) attrs); # example: (traceCallXml "myfun" id 3) will output something like # calling myfun arg 1: 3 result: 3 diff --git a/lib/default.nix b/lib/default.nix index 44076d29517..78566cceae8 100644 --- a/lib/default.nix +++ b/lib/default.nix @@ -9,7 +9,7 @@ let lib = makeExtensible (self: let callLibs = file: import file { lib = self; }; - in with self; { + in { # often used, or depending on very little trivial = callLibs ./trivial.nix; @@ -54,7 +54,7 @@ let filesystem = callLibs ./filesystem.nix; # back-compat aliases - platforms = systems.doubles; + platforms = self.systems.doubles; # linux kernel configuration kernel = callLibs ./kernel.nix; @@ -64,13 +64,13 @@ let hasAttr head isAttrs isBool isInt isList isString length lessThan listToAttrs pathExists readFile replaceStrings seq stringLength sub substring tail; - inherit (trivial) id const pipe concat or and bitAnd bitOr bitXor + inherit (self.trivial) id const pipe concat or and bitAnd bitOr bitXor bitNot boolToString mergeAttrs flip mapNullable inNixShell min max importJSON importTOML warn info showWarnings nixpkgsVersion version mod compare splitByAndCompare functionArgs setFunctionArgs isFunction toHexString toBaseDigits; - inherit (fixedPoints) fix fix' converge extends composeExtensions + inherit (self.fixedPoints) fix fix' converge extends composeExtensions makeExtensible makeExtensibleWithCustomName; - inherit (attrsets) attrByPath hasAttrByPath setAttrByPath + inherit (self.attrsets) attrByPath hasAttrByPath setAttrByPath getAttrFromPath attrVals attrValues getAttrs catAttrs filterAttrs filterAttrsRecursive foldAttrs collect nameValuePair mapAttrs mapAttrs' mapAttrsToList mapAttrsRecursive mapAttrsRecursiveCond @@ -79,13 +79,13 @@ let recursiveUpdate matchAttrs overrideExisting getOutput getBin getLib getDev getMan chooseDevOutputs zipWithNames zip recurseIntoAttrs dontRecurseIntoAttrs; - inherit (lists) singleton forEach foldr fold foldl foldl' imap0 imap1 + inherit (self.lists) singleton forEach foldr fold foldl foldl' imap0 imap1 concatMap flatten remove findSingle findFirst any all count optional optionals toList range partition zipListsWith zipLists reverseList listDfs toposort sort naturalSort compareLists take drop sublist last init crossLists unique intersectLists subtractLists mutuallyExclusive groupBy groupBy'; - inherit (strings) concatStrings concatMapStrings concatImapStrings + inherit (self.strings) concatStrings concatMapStrings concatImapStrings intersperse concatStringsSep concatMapStringsSep concatImapStringsSep makeSearchPath makeSearchPathOutput makeLibraryPath makeBinPath optionalString @@ -97,19 +97,19 @@ let nameFromURL enableFeature enableFeatureAs withFeature withFeatureAs fixedWidthString fixedWidthNumber isStorePath toInt readPathsFromFile fileContents; - inherit (stringsWithDeps) textClosureList textClosureMap + inherit (self.stringsWithDeps) textClosureList textClosureMap noDepEntry fullDepEntry packEntry stringAfter; - inherit (customisation) overrideDerivation makeOverridable + inherit (self.customisation) overrideDerivation makeOverridable callPackageWith callPackagesWith extendDerivation hydraJob makeScope; - inherit (meta) addMetaAttrs dontDistribute setName updateName + inherit (self.meta) addMetaAttrs dontDistribute setName updateName appendToName mapDerivationAttrset setPrio lowPrio lowPrioSet hiPrio hiPrioSet; - inherit (sources) pathType pathIsDirectory cleanSourceFilter + inherit (self.sources) pathType pathIsDirectory cleanSourceFilter cleanSource sourceByRegex sourceFilesBySuffices commitIdFromGitRepo cleanSourceWith pathHasContext canCleanSource pathIsRegularFile pathIsGitRepo; - inherit (modules) evalModules unifyModuleSyntax + inherit (self.modules) evalModules unifyModuleSyntax applyIfFunction mergeModules mergeModules' mergeOptionDecls evalOptionValue mergeDefinitions pushDownProperties dischargeProperties filterOverrides @@ -119,21 +119,21 @@ let mkAliasAndWrapDefinitions fixMergeModules mkRemovedOptionModule mkRenamedOptionModule mkMergedOptionModule mkChangedOptionModule mkAliasOptionModule doRename; - inherit (options) isOption mkEnableOption mkSinkUndeclaredOptions + inherit (self.options) isOption mkEnableOption mkSinkUndeclaredOptions mergeDefaultOption mergeOneOption mergeEqualOption getValues getFiles optionAttrSetToDocList optionAttrSetToDocList' scrubOptionValue literalExample showOption showFiles unknownModule mkOption; - inherit (types) isType setType defaultTypeMerge defaultFunctor + inherit (self.types) isType setType defaultTypeMerge defaultFunctor isOptionType mkOptionType; - inherit (asserts) + inherit (self.asserts) assertMsg assertOneOf; - inherit (debug) addErrorContextToAttrs traceIf traceVal traceValFn + inherit (self.debug) addErrorContextToAttrs traceIf traceVal traceValFn traceXMLVal traceXMLValMarked traceSeq traceSeqN traceValSeq traceValSeqFn traceValSeqN traceValSeqNFn traceShowVal traceShowValMarked showVal traceCall traceCall2 traceCall3 traceValIfNot runTests testAllTrue traceCallXml attrNamesToStr; - inherit (misc) maybeEnv defaultMergeArg defaultMerge foldArgs + inherit (self.misc) maybeEnv defaultMergeArg defaultMerge foldArgs maybeAttrNullable maybeAttr ifEnable checkFlag getValue checkReqs uniqList uniqListExt condConcat lazyGenericClosure innerModifySumArgs modifySumArgs innerClosePropagation @@ -143,7 +143,7 @@ let mergeAttrsByFuncDefaultsClean mergeAttrBy fakeHash fakeSha256 fakeSha512 nixType imap; - inherit (versions) + inherit (self.versions) splitVersion; }); in lib diff --git a/lib/lists.nix b/lib/lists.nix index f424946c72c..6c97e0686aa 100644 --- a/lib/lists.nix +++ b/lib/lists.nix @@ -1,9 +1,9 @@ # General list operations. { lib }: -with lib.trivial; let inherit (lib.strings) toInt; + inherit (lib.trivial) compare min; in rec { diff --git a/lib/modules.nix b/lib/modules.nix index df3a2ad17e5..37e2e23ec12 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -1,12 +1,55 @@ { lib }: -with lib.lists; -with lib.strings; -with lib.trivial; -with lib.attrsets; -with lib.options; -with lib.debug; -with lib.types; +let + inherit (lib.attrsets) + mapAttrsRecursiveCond + ; + inherit (lib.lists) + any all concatLists concatMap + count filter findFirst foldl foldl' + head imap1 length optional + reverseList sort + ; + inherit (lib.options) + isOption + mkOption + showDefs + showFiles + showOption + unknownModule + ; + inherit (lib.attrsets) + attrByPath + attrNames + catAttrs + getAttrFromPath + mapAttrs + mapAttrsToList + optionalAttrs + recursiveUpdate + setAttrByPath + toList + ; + inherit (lib.types) + types + ; + inherit (lib.trivial) + flip + id + isBool + isFunction + isString + min + warn + ; + inherit (lib.strings) + optionalString + ; + inherit (lib) + elem + isAttrs + ; +in rec { @@ -616,7 +659,7 @@ rec { fixupOptionType = loc: opt: let options = opt.options or - (throw "Option `${showOption loc'}' has type optionSet but has no option attribute, in ${showFiles opt.declarations}."); + (throw "Option `${showOption loc}' has type optionSet but has no option attribute, in ${showFiles opt.declarations}."); f = tp: let optionSetIn = type: (tp.name == type) && (tp.functor.wrapped.name == "optionSet"); in diff --git a/lib/options.nix b/lib/options.nix index 9e0ea010bda..97bb2e77176 100644 --- a/lib/options.nix +++ b/lib/options.nix @@ -1,11 +1,15 @@ # Nixpkgs/NixOS option handling. { lib }: -with lib.trivial; -with lib.lists; -with lib.attrsets; -with lib.strings; - +let + inherit (lib) + isAttrs isBool isDerivation isFunction isInt isList isString + all collect concatMap concatLists elemAt filter foldl' head length mapAttrs optionals optional take + ; + inherit (lib.attrsets) optionalAttrs; + inherit (lib.strings) concatMapStrings concatStringsSep; + inherit (lib.types) mkOptionType; +in rec { /* Returns true when the given argument is an option diff --git a/lib/sources.nix b/lib/sources.nix index 776fcc32052..c7a3a959152 100644 --- a/lib/sources.nix +++ b/lib/sources.nix @@ -1,16 +1,33 @@ # Functions for copying sources to the Nix store. { lib }: +let + inherit (builtins) + hasContext + match + readDir + readFile + storeDir + tryEval + ; + inherit (lib) + filter + getAttr + isString + pathExists + split + ; +in rec { # Returns the type of a path: regular (for file), symlink, or directory - pathType = p: with builtins; getAttr (baseNameOf p) (readDir (dirOf p)); + pathType = p: getAttr (baseNameOf p) (readDir (dirOf p)); # Returns true if the path exists and is a directory, false otherwise - pathIsDirectory = p: if builtins.pathExists p then (pathType p) == "directory" else false; + pathIsDirectory = p: if pathExists p then (pathType p) == "directory" else false; # Returns true if the path exists and is a regular file, false otherwise - pathIsRegularFile = p: if builtins.pathExists p then (pathType p) == "regular" else false; + pathIsRegularFile = p: if pathExists p then (pathType p) == "regular" else false; # Bring in a path as a source, filtering out all Subversion and CVS # directories, as well as backup files (*~). @@ -19,8 +36,8 @@ rec { (baseName == ".git" || type == "directory" && (baseName == ".svn" || baseName == "CVS" || baseName == ".hg")) || # Filter out editor backup / swap files. lib.hasSuffix "~" baseName || - builtins.match "^\\.sw[a-z]$" baseName != null || - builtins.match "^\\..*\\.sw[a-z]$" baseName != null || + match "^\\.sw[a-z]$" baseName != null || + match "^\\..*\\.sw[a-z]$" baseName != null || # Filter out generates files. lib.hasSuffix ".o" baseName || @@ -89,7 +106,7 @@ rec { in lib.cleanSourceWith { filter = (path: type: let relPath = lib.removePrefix (toString origSrc + "/") (toString path); - in lib.any (re: builtins.match re relPath != null) regexes); + in lib.any (re: match re relPath != null) regexes); inherit src; }; @@ -102,13 +119,12 @@ rec { in type == "directory" || lib.any (ext: lib.hasSuffix ext base) exts; in cleanSourceWith { inherit filter; src = path; }; - pathIsGitRepo = path: (builtins.tryEval (commitIdFromGitRepo path)).success; + pathIsGitRepo = path: (tryEval (commitIdFromGitRepo path)).success; # Get the commit id of a git repo # Example: commitIdFromGitRepo commitIdFromGitRepo = let readCommitFromFile = file: path: - with builtins; let fileName = toString path + "/" + file; packedRefsName = toString path + "/packed-refs"; absolutePath = base: path: @@ -145,11 +161,11 @@ rec { # packed-refs file, so we have to grep through it: then let fileContent = readFile packedRefsName; - matchRef = builtins.match "([a-z0-9]+) ${file}"; - isRef = s: builtins.isString s && (matchRef s) != null; + matchRef = match "([a-z0-9]+) ${file}"; + isRef = s: isString s && (matchRef s) != null; # there is a bug in libstdc++ leading to stackoverflow for long strings: # https://github.com/NixOS/nix/issues/2147#issuecomment-659868795 - refs = builtins.filter isRef (builtins.split "\n" fileContent); + refs = filter isRef (split "\n" fileContent); in if refs == [] then throw ("Could not find " + file + " in " + packedRefsName) else lib.head (matchRef (lib.head refs)) @@ -157,7 +173,7 @@ rec { else throw ("Not a .git directory: " + path); in readCommitFromFile "HEAD"; - pathHasContext = builtins.hasContext or (lib.hasPrefix builtins.storeDir); + pathHasContext = builtins.hasContext or (lib.hasPrefix storeDir); canCleanSource = src: src ? _isLibCleanSourceWith || !(pathHasContext (toString src)); } diff --git a/lib/strings-with-deps.nix b/lib/strings-with-deps.nix index e3336983428..7b88b018da5 100644 --- a/lib/strings-with-deps.nix +++ b/lib/strings-with-deps.nix @@ -41,10 +41,15 @@ Usage: [1] maybe this behaviour should be removed to keep things simple (?) */ -with lib.lists; -with lib.attrsets; -with lib.strings; - +let + inherit (lib) + concatStringsSep + head + isAttrs + listToAttrs + tail + ; +in rec { /* !!! The interface of this function is kind of messed up, since diff --git a/lib/strings.nix b/lib/strings.nix index d81e46a1763..f62ff6679ef 100644 --- a/lib/strings.nix +++ b/lib/strings.nix @@ -8,7 +8,29 @@ in rec { - inherit (builtins) stringLength substring head tail isString replaceStrings; + inherit (builtins) + compareVersions + elem + elemAt + filter + fromJSON + head + isInt + isList + isString + match + parseDrvName + readFile + replaceStrings + split + storeDir + stringLength + substring + tail + toJSON + typeOf + unsafeDiscardStringContext + ; /* Concatenate a list of strings. @@ -120,7 +142,7 @@ rec { subDir: # List of base paths paths: - concatStringsSep ":" (map (path: path + "/" + subDir) (builtins.filter (x: x != null) paths)); + concatStringsSep ":" (map (path: path + "/" + subDir) (filter (x: x != null) paths)); /* Construct a Unix-style search path by appending the given `subDir` to the specified `output` of each of the packages. If no @@ -313,7 +335,7 @@ rec { escapeNixString "hello\${}\n" => "\"hello\\\${}\\n\"" */ - escapeNixString = s: escape ["$"] (builtins.toJSON s); + escapeNixString = s: escape ["$"] (toJSON s); /* Turn a string into an exact regular expression @@ -337,7 +359,7 @@ rec { */ escapeNixIdentifier = s: # Regex from https://github.com/NixOS/nix/blob/d048577909e383439c2549e849c5c2f2016c997e/src/libexpr/lexer.l#L91 - if builtins.match "[a-zA-Z_][a-zA-Z0-9_'-]*" s != null + if match "[a-zA-Z_][a-zA-Z0-9_'-]*" s != null then s else escapeNixString s; # Obsolete - use replaceStrings instead. @@ -466,7 +488,7 @@ rec { versionOlder "1.1" "1.1" => false */ - versionOlder = v1: v2: builtins.compareVersions v2 v1 == 1; + versionOlder = v1: v2: compareVersions v2 v1 == 1; /* Return true if string v1 denotes a version equal to or newer than v2. @@ -492,7 +514,7 @@ rec { */ getName = x: let - parse = drv: (builtins.parseDrvName drv).name; + parse = drv: (parseDrvName drv).name; in if isString x then parse x else x.pname or (parse x.name); @@ -509,7 +531,7 @@ rec { */ getVersion = x: let - parse = drv: (builtins.parseDrvName drv).version; + parse = drv: (parseDrvName drv).version; in if isString x then parse x else x.version or (parse x.name); @@ -527,7 +549,7 @@ rec { let components = splitString "/" url; filename = lib.last components; - name = builtins.head (splitString sep filename); + name = head (splitString sep filename); in assert name != filename; name; /* Create an --{enable,disable}- string that can be passed to @@ -617,14 +639,14 @@ rec { */ floatToString = float: let result = toString float; - precise = float == builtins.fromJSON result; + precise = float == fromJSON result; in if precise then result else lib.warn "Imprecise conversion from float to string ${result}" result; /* Check whether a value can be coerced to a string */ isCoercibleToString = x: - builtins.elem (builtins.typeOf x) [ "path" "string" "null" "int" "float" "bool" ] || - (builtins.isList x && lib.all isCoercibleToString x) || + elem (typeOf x) [ "path" "string" "null" "int" "float" "bool" ] || + (isList x && lib.all isCoercibleToString x) || x ? outPath || x ? __toString; @@ -643,8 +665,8 @@ rec { isStorePath = x: if isCoercibleToString x then let str = toString x; in - builtins.substring 0 1 str == "/" - && dirOf str == builtins.storeDir + substring 0 1 str == "/" + && dirOf str == storeDir else false; @@ -662,8 +684,8 @@ rec { */ # Obviously, it is a bit hacky to use fromJSON this way. toInt = str: - let may_be_int = builtins.fromJSON str; in - if builtins.isInt may_be_int + let may_be_int = fromJSON str; in + if isInt may_be_int then may_be_int else throw "Could not convert ${str} to int."; @@ -685,10 +707,10 @@ rec { readPathsFromFile = lib.warn "lib.readPathsFromFile is deprecated, use a list instead" (rootPath: file: let - lines = lib.splitString "\n" (builtins.readFile file); + lines = lib.splitString "\n" (readFile file); removeComments = lib.filter (line: line != "" && !(lib.hasPrefix "#" line)); relativePaths = removeComments lines; - absolutePaths = builtins.map (path: rootPath + "/${path}") relativePaths; + absolutePaths = map (path: rootPath + "/${path}") relativePaths; in absolutePaths); @@ -702,7 +724,7 @@ rec { fileContents ./version => "1.0" */ - fileContents = file: removeSuffix "\n" (builtins.readFile file); + fileContents = file: removeSuffix "\n" (readFile file); /* Creates a valid derivation name from a potentially invalid one. @@ -720,13 +742,13 @@ rec { sanitizeDerivationName = string: lib.pipe string [ # Get rid of string context. This is safe under the assumption that the # resulting string is only used as a derivation name - builtins.unsafeDiscardStringContext + unsafeDiscardStringContext # Strip all leading "." - (x: builtins.elemAt (builtins.match "\\.*(.*)" x) 0) + (x: elemAt (match "\\.*(.*)" x) 0) # Split out all invalid characters # https://github.com/NixOS/nix/blob/2.3.2/src/libstore/store-api.cc#L85-L112 # https://github.com/NixOS/nix/blob/2242be83c61788b9c0736a92bb0b5c7bbfc40803/nix-rust/src/store/path.rs#L100-L125 - (builtins.split "[^[:alnum:]+._?=-]+") + (split "[^[:alnum:]+._?=-]+") # Replace invalid character ranges with a "-" (concatMapStrings (s: if lib.isList s then "-" else s)) # Limit to 211 characters (minus 4 chars for ".drv") diff --git a/lib/types.nix b/lib/types.nix index dd287734388..1144c018b26 100644 --- a/lib/types.nix +++ b/lib/types.nix @@ -1,12 +1,62 @@ # Definitions related to run-time type checking. Used in particular # to type-check NixOS configurations. { lib }: -with lib.lists; -with lib.attrsets; -with lib.options; -with lib.trivial; -with lib.strings; + let + inherit (lib) + elem + flip + functionArgs + isAttrs + isBool + isDerivation + isFloat + isFunction + isInt + isList + isString + isStorePath + setFunctionArgs + toDerivation + toList + ; + inherit (lib.lists) + all + concatLists + count + elemAt + filter + foldl' + head + imap1 + last + length + tail + unique + ; + inherit (lib.attrsets) + attrNames + filterAttrs + hasAttr + mapAttrs + optionalAttrs + zipAttrsWith + ; + inherit (lib.options) + getFiles + getValues + mergeDefaultOption + mergeEqualOption + mergeOneOption + showFiles + showOption + ; + inherit (lib.strings) + concatMapStringsSep + concatStringsSep + escapeNixString + isCoercibleToString + ; inherit (lib.modules) mergeDefinitions; outer_types = -- cgit 1.4.1 From f8ab5fcd8d15dd551dfc5c6e34f27a1e92d7b1f6 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Tue, 20 Oct 2020 16:10:21 +0200 Subject: lib/modules: Simplify inherits --- lib/modules.nix | 66 ++++++++++++++++++++++++++++----------------------------- 1 file changed, 32 insertions(+), 34 deletions(-) (limited to 'lib/modules.nix') diff --git a/lib/modules.nix b/lib/modules.nix index 37e2e23ec12..6c4a8f7b324 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -1,53 +1,51 @@ { lib }: let - inherit (lib.attrsets) - mapAttrsRecursiveCond - ; - inherit (lib.lists) - any all concatLists concatMap - count filter findFirst foldl foldl' - head imap1 length optional - reverseList sort - ; - inherit (lib.options) - isOption - mkOption - showDefs - showFiles - showOption - unknownModule - ; - inherit (lib.attrsets) + inherit (lib) + all + any attrByPath attrNames catAttrs + concatLists + concatMap + count + elem + filter + findFirst + flip + foldl + foldl' getAttrFromPath + head + id + imap1 + isAttrs + isBool + isFunction + isString + length mapAttrs mapAttrsToList + mapAttrsRecursiveCond + min + optional optionalAttrs + optionalString recursiveUpdate + reverseList sort setAttrByPath toList - ; - inherit (lib.types) types - ; - inherit (lib.trivial) - flip - id - isBool - isFunction - isString - min warn ; - inherit (lib.strings) - optionalString - ; - inherit (lib) - elem - isAttrs + inherit (lib.options) + isOption + mkOption + showDefs + showFiles + showOption + unknownModule ; in -- cgit 1.4.1 From e0fa72d04d75f7d7e5a0b3d45a69825f209589f9 Mon Sep 17 00:00:00 2001 From: Robert Helgesson Date: Thu, 22 Oct 2020 20:28:22 +0200 Subject: docs: update documentation of `mkRemovedOptionModule` Since b08b0bcbbec77046e5a7082177cedc12fbf1dc6c, the function actually causes an assertion error, not a warning. --- lib/modules.nix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/modules.nix') diff --git a/lib/modules.nix b/lib/modules.nix index df3a2ad17e5..103a22ed131 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -719,7 +719,7 @@ rec { mkRemovedOptionModule [ "boot" "loader" "grub" "bootDevice" ] "" - causes a warning if the user defines boot.loader.grub.bootDevice. + causes a assertion if the user defines boot.loader.grub.bootDevice. replacementInstructions is a string that provides instructions on how to achieve the same functionality without the removed option, -- cgit 1.4.1 From df5ba82f74df75e96390995472f3e1e5179da21c Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Tue, 1 Sep 2020 23:32:57 +0200 Subject: 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. --- lib/modules.nix | 153 +++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 152 insertions(+), 1 deletion(-) (limited to 'lib/modules.nix') diff --git a/lib/modules.nix b/lib/modules.nix index 3f2bfd478b0..0d761c632d0 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -46,6 +46,7 @@ let showFiles showOption unknownModule + literalExample ; in @@ -116,6 +117,98 @@ rec { turned off. ''; }; + + _module.assertions = mkOption { + description = '' + Assertions and warnings to trigger during module evaluation. The + attribute name will be displayed when it is triggered, allowing + users to disable/change these assertions again 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 = true; + type = types.attrsOf (types.submodule { + # TODO: Rename to assertion? Or allow also setting assertion? + options.enable = mkOption { + description = '' + Whether to enable this assertion. + + This is the inverse of asserting a condition: If a certain + condition should be true, then this + option should be set to false when that + case occurs + + ''; + type = types.bool; + }; + + options.type = mkOption { + description = '' + The type of the assertion. The default + "error" type will cause evaluation to fail, + while the "warning" type will only show a + warning. + ''; + type = types.enum [ "error" "warning" ]; + default = "error"; + example = "warning"; + }; + + options.message = mkOption { + description = '' + The assertion message to display if this assertion triggers. + To display option names in the message, add + options to the module function arguments + and use ''${options.path.to.option}. + ''; + type = types.str; + example = literalExample '' + Enabling both ''${options.services.foo.enable} and ''${options.services.bar.enable} is not possible. + ''; + }; + + options.triggerPath = mkOption { + description = '' + The config path which when evaluated should + trigger this assertion. By default this is + [], meaning evaluating + config at all will trigger the assertion. + On NixOS this default is changed to + [ "system" "build" "toplevel" such that + only a system evaluation triggers the assertions. + + Evaluating config 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 + assertions if this option doesn't have an externally-accessed + value. + + ''; + # Mark as internal as it's easy to misuse it + internal = true; + type = types.uniq (types.listOf types.str); + # Default to [], causing assertions to be triggered when + # anything is evaluated. This is a safe and convenient default. + default = []; + example = [ "system" "build" "vm" ]; + }; + }); + }; }; config = { @@ -154,6 +247,64 @@ rec { # paths, meaning recursiveUpdate will never override any value else recursiveUpdate freeformConfig declaredConfig; + /* + Inject a list of assertions into a config value, corresponding to their + triggerPath (meaning when that path is accessed from the result of this + function, the assertion triggers). + */ + injectAssertions = assertions: config: let + # Partition into assertions that are triggered on this level and ones that aren't + parted = partition (a: length a.triggerPath == 0) assertions; + + # From the ones that are triggered, filter out ones that aren't enabled + # and group into warnings/errors + byType = groupBy (a: a.type) (filter (a: a.enable) parted.right); + + # 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 assertions: + ${concatMapStringsSep "\n" (a: "- ${a.show}") byType.error} + ''; + # Trigger for both warnings and errors + trigger = value: warningTrigger (errorTrigger value); + + # From the non-triggered assertions, split off the first element of triggerPath + # to get a mapping from nested attributes to a list of assertions for that attribute + nested = zipAttrs (map (a: { + ${head a.triggerPath} = a // { + triggerPath = tail a.triggerPath; + }; + }) parted.wrong); + + # Recursively inject assertions if config is an attribute set and we + # have assertions under its attributes + result = + if isAttrs config + then + mapAttrs (name: value: + if nested ? ${name} + then injectAssertions nested.${name} value + else value + ) config + else config; + in trigger result; + + # List of assertions for this module evaluation, where each assertion also + # has a `show` attribute for how to show it if triggered + assertions = mapAttrsToList (name: value: + let id = + if hasPrefix "_" name then "" + else "[${showOption prefix}${optionalString (prefix != []) "/"}${name}] "; + in value // { + show = "${id}${value.message}"; + } + ) config._module.assertions; + + finalConfig = injectAssertions assertions (removeAttrs config [ "_module" ]); + checkUnmatched = if config._module.check && config._module.freeformType == null && merged.unmatchedDefns != [] then let @@ -173,7 +324,7 @@ rec { result = builtins.seq checkUnmatched { inherit options; - config = removeAttrs config [ "_module" ]; + config = finalConfig; inherit (config) _module; }; in result; -- cgit 1.4.1 From 9523df7eb600e7fc2a88bc5227d9dfe12055a9bd Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Wed, 2 Sep 2020 00:17:26 +0200 Subject: nixos/assertions: Use module-builtin assertion implementation --- lib/modules.nix | 12 ++++++------ nixos/modules/misc/assertions.nix | 21 ++++++++++++++++++++- nixos/modules/system/activation/top-level.nix | 10 +--------- 3 files changed, 27 insertions(+), 16 deletions(-) (limited to 'lib/modules.nix') diff --git a/lib/modules.nix b/lib/modules.nix index 0d761c632d0..31200ae0b03 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -254,11 +254,11 @@ rec { */ injectAssertions = assertions: config: let # Partition into assertions that are triggered on this level and ones that aren't - parted = partition (a: length a.triggerPath == 0) assertions; + parted = lib.partition (a: length a.triggerPath == 0) assertions; # From the ones that are triggered, filter out ones that aren't enabled # and group into warnings/errors - byType = groupBy (a: a.type) (filter (a: a.enable) parted.right); + byType = lib.groupBy (a: a.type) (filter (a: a.enable) parted.right); # 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 []); @@ -266,16 +266,16 @@ rec { if byType.error or [] == [] then value else throw '' Failed assertions: - ${concatMapStringsSep "\n" (a: "- ${a.show}") byType.error} + ${lib.concatMapStringsSep "\n" (a: "- ${a.show}") byType.error} ''; # Trigger for both warnings and errors trigger = value: warningTrigger (errorTrigger value); # From the non-triggered assertions, split off the first element of triggerPath # to get a mapping from nested attributes to a list of assertions for that attribute - nested = zipAttrs (map (a: { + nested = lib.zipAttrs (map (a: { ${head a.triggerPath} = a // { - triggerPath = tail a.triggerPath; + triggerPath = lib.tail a.triggerPath; }; }) parted.wrong); @@ -296,7 +296,7 @@ rec { # has a `show` attribute for how to show it if triggered assertions = mapAttrsToList (name: value: let id = - if hasPrefix "_" name then "" + if lib.hasPrefix "_" name then "" else "[${showOption prefix}${optionalString (prefix != []) "/"}${name}] "; in value // { show = "${id}${value.message}"; diff --git a/nixos/modules/misc/assertions.nix b/nixos/modules/misc/assertions.nix index 550b3ac97f6..e931611247f 100644 --- a/nixos/modules/misc/assertions.nix +++ b/nixos/modules/misc/assertions.nix @@ -1,4 +1,4 @@ -{ lib, ... }: +{ lib, config, ... }: with lib; @@ -29,6 +29,25 @@ with lib; ''; }; + _module.assertions = mkOption { + type = types.attrsOf (types.submodule { + triggerPath = mkDefault [ "system" "build" "toplevel" ]; + }); + }; + }; + + config._module.assertions = lib.listToAttrs (lib.imap1 (n: value: + let + name = "_${toString n}"; + isWarning = lib.isString value; + result = { + enable = if isWarning then true 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 } diff --git a/nixos/modules/system/activation/top-level.nix b/nixos/modules/system/activation/top-level.nix index 03d7e749323..17b62ad9569 100644 --- a/nixos/modules/system/activation/top-level.nix +++ b/nixos/modules/system/activation/top-level.nix @@ -117,18 +117,10 @@ 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; } - ) baseSystemAssertWarn config.system.replaceRuntimeDependencies; + ) baseSystem config.system.replaceRuntimeDependencies; in -- cgit 1.4.1 From 20131348db3b51f7fe41f2d4aa3cd8875a6b48f2 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Wed, 2 Sep 2020 15:43:16 +0200 Subject: lib/modules: Use module-builtin assertions for mkRemovedOptionModule and co. --- lib/modules.nix | 38 ++++++++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 14 deletions(-) (limited to 'lib/modules.nix') diff --git a/lib/modules.nix b/lib/modules.nix index 31200ae0b03..1902db5c616 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -924,14 +924,16 @@ 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.assertions = + let opt = getAttrFromPath optionName options; in { + ${showOption optionName} = { + enable = mkDefault 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 @@ -992,14 +994,19 @@ 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.assertions = + let warningMessages = map (f: + let val = getAttrFromPath f config; + opt = getAttrFromPath f options; + in { + ${showOption f} = { + enable = mkDefault (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) @@ -1058,8 +1065,11 @@ rec { }); config = mkMerge [ { - warnings = optional (warn && fromOpt.isDefined) - "The option `${showOption from}' defined in ${showFiles fromOpt.files} has been renamed to `${showOption to}'."; + _module.assertions.${showOption from} = { + enable = mkDefault (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 -- cgit 1.4.1 From 1e6a84b7af71f579f2467619f504d1cfe43bb3e9 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Wed, 2 Sep 2020 16:10:17 +0200 Subject: nixos/modules: Allow options to be coerced to a string for convenience --- lib/modules.nix | 2 ++ 1 file changed, 2 insertions(+) (limited to 'lib/modules.nix') diff --git a/lib/modules.nix b/lib/modules.nix index 1902db5c616..9ff8f4701bb 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -665,6 +665,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. -- cgit 1.4.1 From 3759a77fcda2e33f89023b8c6b1476e8fa413a8e Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Wed, 2 Sep 2020 18:01:10 +0200 Subject: nixos/modules: Expose the internal module in the top-level documentation --- lib/modules.nix | 15 ++++++++++----- lib/tests/misc.nix | 2 +- 2 files changed, 11 insertions(+), 6 deletions(-) (limited to 'lib/modules.nix') diff --git a/lib/modules.nix b/lib/modules.nix index 9ff8f4701bb..e3f7ca3581c 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -77,10 +77,15 @@ rec { # attribute. These options are fragile, as they are used by the # module system to change the interpretation of modules. internalModule = rec { - _file = ./modules.nix; + # FIXME: Using ./modules.nix directly breaks the doc for some reason + _file = "lib/modules.nix"; key = _file; + # 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 @@ -90,13 +95,13 @@ 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."; }; _module.check = mkOption { type = types.bool; - internal = true; + internal = prefix != []; default = check; description = "Whether to check whether all option definitions have matching declarations."; }; @@ -104,7 +109,7 @@ rec { _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 @@ -141,7 +146,7 @@ rec { } ''; default = {}; - internal = true; + internal = prefix != []; type = types.attrsOf (types.submodule { # TODO: Rename to assertion? Or allow also setting assertion? options.enable = mkOption { 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" "" "bar" ] [ "foo" "bar" ] ]; }; -- cgit 1.4.1 From c9cc8969b4d4adeb389837d1cc85cc73a8272f55 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Mon, 30 Nov 2020 20:04:03 +0100 Subject: lib/modules: Rename _module.assertions to _module.checks --- lib/modules.nix | 66 ++++++++++--------- lib/tests/modules.sh | 16 ++--- lib/tests/modules/assertions/enable-false.nix | 2 +- lib/tests/modules/assertions/enable-lazy.nix | 2 +- lib/tests/modules/assertions/multi.nix | 2 +- lib/tests/modules/assertions/non-cascading.nix | 2 +- lib/tests/modules/assertions/simple.nix | 2 +- .../assertions/submodule-attrsOf-attrsOf.nix | 2 +- lib/tests/modules/assertions/submodule-attrsOf.nix | 2 +- lib/tests/modules/assertions/submodule.nix | 2 +- lib/tests/modules/assertions/trigger-lazy.nix | 2 +- lib/tests/modules/assertions/trigger-submodule.nix | 2 +- .../modules/assertions/underscore-attributes.nix | 2 +- lib/tests/modules/assertions/warning.nix | 2 +- nixos/doc/manual/development/assertions.xml | 74 +++++++++++++++------- nixos/modules/misc/assertions.nix | 4 +- 16 files changed, 108 insertions(+), 76 deletions(-) (limited to 'lib/modules.nix') diff --git a/lib/modules.nix b/lib/modules.nix index e3f7ca3581c..9aa638231bf 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -103,7 +103,13 @@ rec { type = types.bool; internal = prefix != []; 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.freeformType = mkOption { @@ -123,11 +129,11 @@ rec { ''; }; - _module.assertions = mkOption { + _module.checks = mkOption { description = '' - Assertions and warnings to trigger during module evaluation. The + Evaluation checks to trigger during module evaluation. The attribute name will be displayed when it is triggered, allowing - users to disable/change these assertions again if necessary. See + users to disable/change these checks if necessary. See the section on Warnings and Assertions in the manual for more information. ''; @@ -151,7 +157,7 @@ rec { # TODO: Rename to assertion? Or allow also setting assertion? options.enable = mkOption { description = '' - Whether to enable this assertion. + Whether to enable this check. This is the inverse of asserting a condition: If a certain condition should be true, then this @@ -164,7 +170,7 @@ rec { options.type = mkOption { description = '' - The type of the assertion. The default + The type of the check. The default "error" type will cause evaluation to fail, while the "warning" type will only show a warning. @@ -176,7 +182,7 @@ rec { options.message = mkOption { description = '' - The assertion message to display if this assertion triggers. + The message to display if this check triggers. To display option names in the message, add options to the module function arguments and use ''${options.path.to.option}. @@ -190,24 +196,24 @@ rec { options.triggerPath = mkOption { description = '' The config path which when evaluated should - trigger this assertion. By default this is + trigger this check. By default this is [], meaning evaluating - config at all will trigger the assertion. + config at all will trigger the check. On NixOS this default is changed to [ "system" "build" "toplevel" such that - only a system evaluation triggers the assertions. + only a system evaluation triggers the checks. Evaluating config 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 - assertions if this option doesn't have an externally-accessed + failing checks if this option doesn't have an externally-accessed value. ''; # Mark as internal as it's easy to misuse it internal = true; type = types.uniq (types.listOf types.str); - # Default to [], causing assertions to be triggered when + # Default to [], causing checks to be triggered when # anything is evaluated. This is a safe and convenient default. default = []; example = [ "system" "build" "vm" ]; @@ -253,13 +259,13 @@ rec { else recursiveUpdate freeformConfig declaredConfig; /* - Inject a list of assertions into a config value, corresponding to their + 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 assertion triggers). + function, the check triggers). */ - injectAssertions = assertions: config: let - # Partition into assertions that are triggered on this level and ones that aren't - parted = lib.partition (a: length a.triggerPath == 0) assertions; + 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; # From the ones that are triggered, filter out ones that aren't enabled # and group into warnings/errors @@ -270,45 +276,45 @@ rec { errorTrigger = value: if byType.error or [] == [] then value else throw '' - Failed assertions: + Failed checks: ${lib.concatMapStringsSep "\n" (a: "- ${a.show}") byType.error} ''; # Trigger for both warnings and errors trigger = value: warningTrigger (errorTrigger value); - # From the non-triggered assertions, split off the first element of triggerPath - # to get a mapping from nested attributes to a list of assertions for that attribute + # 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); - # Recursively inject assertions if config is an attribute set and we - # have assertions under its attributes + # 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 injectAssertions nested.${name} value + then injectChecks nested.${name} value else value ) config else config; in trigger result; - # List of assertions for this module evaluation, where each assertion also + # List of checks for this module evaluation, where each check also # has a `show` attribute for how to show it if triggered - assertions = mapAttrsToList (name: value: + 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.assertions; + ) config._module.checks; - finalConfig = injectAssertions assertions (removeAttrs config [ "_module" ]); + finalConfig = injectChecks checks (removeAttrs config [ "_module" ]); checkUnmatched = if config._module.check && config._module.freeformType == null && merged.unmatchedDefns != [] then @@ -931,7 +937,7 @@ rec { visible = false; apply = x: throw "The option `${showOption optionName}' can no longer be used since it's been removed. ${replacementInstructions}"; }); - config._module.assertions = + config._module.checks = let opt = getAttrFromPath optionName options; in { ${showOption optionName} = { enable = mkDefault opt.isDefined; @@ -1001,7 +1007,7 @@ rec { })) from); config = { - _module.assertions = + _module.checks = let warningMessages = map (f: let val = getAttrFromPath f config; opt = getAttrFromPath f options; @@ -1072,7 +1078,7 @@ rec { }); config = mkMerge [ { - _module.assertions.${showOption from} = { + _module.checks.${showOption from} = { enable = mkDefault (warn && fromOpt.isDefined); type = "warning"; message = "The option `${showOption from}' defined in ${showFiles fromOpt.files} has been renamed to `${showOption to}'."; diff --git a/lib/tests/modules.sh b/lib/tests/modules.sh index 65eb91c9927..43bcabdf816 100755 --- a/lib/tests/modules.sh +++ b/lib/tests/modules.sh @@ -277,9 +277,9 @@ 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 assertions:\n- \[test\] Assertion failed' config ./assertions/simple.nix +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 assertions:\n- \[test\] Assertion failed' config.foo ./assertions/trigger-lazy.nix +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 @@ -294,23 +294,23 @@ checkConfigCodeOutErr 0 '{ }' 'warning: \[test\] Warning message' config ./asser # A triggerPath can be set to a submodule path checkConfigOutput '{ baz = ; }' config.foo.bar ./assertions/trigger-submodule.nix -checkConfigError 'Failed assertions:\n- \[test\] Assertion failed' config.foo.bar.baz ./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 assertions:\n- \[test1\] Assertion 1 failed\n- \[test2\] Assertion 2 failed' config ./assertions/multi.nix +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 assertions:\n- \[foo/test\] Assertion failed' config.foo ./assertions/submodule.nix -checkConfigError 'Failed assertions:\n- \[foo.bar/test\] Assertion failed' config.foo.bar ./assertions/submodule-attrsOf.nix -checkConfigError 'Failed assertions:\n- \[foo.bar.baz/test\] Assertion failed' config.foo.bar.baz ./assertions/submodule-attrsOf-attrsOf.nix +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 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 assertions:\n- Assertion failed' config ./assertions/underscore-attributes.nix +checkConfigError 'Failed checks:\n- Assertion failed' config ./assertions/underscore-attributes.nix cat < - Warnings and Assertions + Evaluation Checks When configuration problems are detectable in a module, it is a good idea to - write an assertion or warning. Doing so provides clear feedback to the user - and can prevent errors before the build. + write a check for catching it early. Doing so can provide clear feedback to + the user and can prevent errors before the build. Although Nix has the abort and builtins.trace functions - 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. + 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.
- Defining Warnings and Assertions + Defining Checks - Both warnings and assertions can be defined using the option. Each assertion needs an attribute name, under which you have to define an enable condition using and a message using . Note that the enable condition is inverse of what an assertion would be: To assert a value being true, the enable condition should be false in that case, so that it isn't triggered. For the assertion message, you can add options to the module arguments and use ${options.path.to.option} to print a context-aware string representation of the option path. Here is an example showing how this can be done. + Checks can be defined using the option. + Each check needs an attribute name, under which you have to define an enable + condition using and a + message using . Note that + the enable condition is inverse of what an assertion + would be: To assert a value being true, the enable condition should be false + in that case, so that it isn't triggered. For the check message, you can add + options to the module arguments and use + ${options.path.to.option} to print a context-aware string + representation of the option path. Here is an example showing how this can be + done. { config, options, ... }: { - _module.assertions.gpgSshAgent = { + _module.checks.gpgSshAgent = { enable = config.programs.gnupg.agent.enableSSHSupport && config.programs.ssh.startAgent; message = "You can't enable both ${options.programs.ssh.startAgent}" + " and ${options.programs.gnupg.agent.enableSSHSupport}!"; }; - _module.assertions.grafanaPassword = { + _module.checks.grafanaPassword = { enable = 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!"; @@ -48,41 +58,51 @@
- Ignoring Warnings and Assertions + Ignoring Checks - Sometimes you can get warnings or assertions that don't apply to your specific case and you wish to ignore them, or at least make assertions non-fatal. You can do so for all assertions defined using by using the attribute name of the definition, which is conveniently printed using [...] when the assertion is triggered. For above example, the evaluation output when the assertions are triggered looks as follows: + 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 by + using the attribute name of the definition, which is conveniently printed + using [...] when the check is triggered. For above + example, the evaluation output when the checks are triggered looks as + follows: trace: warning: [grafanaPassword] The grafana password defined with services.grafana.database.password will be stored as plaintext in the Nix store! -error: Failed assertions: +error: Failed checks: - [gpgSshAgent] You can't enable both programs.ssh.startAgent and programs.gnupg.agent.enableSSHSupport! - The [grafanaPassword] and [gpgSshAgent] strings tell you that these were defined under the grafanaPassword and gpgSshAgent attributes of respectively. With this knowledge you can adjust them to your liking: + The [grafanaPassword] and [gpgSshAgent] + strings tell you that these were defined under the grafanaPassword + and gpgSshAgent attributes of + respectively. With this knowledge + you can adjust them to your liking: { lib, ... }: { - # Change the assertion into a non-fatal warning - _module.assertions.gpgSshAgent.type = "warning"; + # Change the error into a non-fatal warning + _module.checks.gpgSshAgent.type = "warning"; # We don't care about this warning, disable it - _module.assertions.grafanaPassword.enable = lib.mkForce false; + _module.checks.grafanaPassword.enable = lib.mkForce false; }
- Warnings and Assertions in Submodules + Checks in Submodules - Warnings and assertions can be defined within submodules in the same way. Here is an example: + Evaluation checks can be defined within submodules in the same way. Here is an example: @@ -92,7 +112,7 @@ error: Failed assertions: type = lib.types.attrsOf (lib.types.submodule ({ config, options, ... }: { options.port = lib.mkOption {}; - config._module.assertions.portConflict = { + config._module.checks.portConflict = { enable = config.port == 80; message = "Port ${toString config.port} defined using" + " ${options.port} is usually used for HTTP"; @@ -105,7 +125,10 @@ error: Failed assertions: - When this assertion is triggered, it shows both the submodule path along with the assertion attribute within that submodule, joined by a /. Note also how ${options.port} correctly shows the context of the option. + When this check is triggered, it shows both the submodule path along with + the check attribute within that submodule, joined by a + /. Note also how ${options.port} + correctly shows the context of the option. @@ -114,18 +137,21 @@ trace: warning: [myServices.foo/portConflict] Port 80 defined using - Therefore to disable such an assertion, you can do so by changing the option within the myServices.foo submodule: + Therefore to disable such a check, you can do so by changing the + option within the + myServices.foo submodule: { lib, ... }: { - myServices.foo._module.assertions.portConflict.enable = lib.mkForce false; + myServices.foo._module.checks.portConflict.enable = lib.mkForce false; } - Assertions defined in submodules under types.listOf can't be ignored, since there's no way to change previously defined list items. + Checks defined in submodules under types.listOf can't be + ignored, since there's no way to change previously defined list items. diff --git a/nixos/modules/misc/assertions.nix b/nixos/modules/misc/assertions.nix index e931611247f..e8b1f5afca3 100644 --- a/nixos/modules/misc/assertions.nix +++ b/nixos/modules/misc/assertions.nix @@ -29,7 +29,7 @@ with lib; ''; }; - _module.assertions = mkOption { + _module.checks = mkOption { type = types.attrsOf (types.submodule { triggerPath = mkDefault [ "system" "build" "toplevel" ]; }); @@ -37,7 +37,7 @@ with lib; }; - config._module.assertions = lib.listToAttrs (lib.imap1 (n: value: + config._module.checks = lib.listToAttrs (lib.imap1 (n: value: let name = "_${toString n}"; isWarning = lib.isString value; -- cgit 1.4.1 From 8dea4df90323c43f9cc86a629f1581b91866e11d Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Mon, 30 Nov 2020 21:42:52 +0100 Subject: 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.nix | 109 +++++---------------- lib/tests/modules.sh | 15 --- lib/tests/modules/assertions/enable-lazy.nix | 17 ---- lib/tests/modules/assertions/non-cascading.nix | 17 ---- lib/tests/modules/assertions/trigger-lazy.nix | 15 --- lib/tests/modules/assertions/trigger-submodule.nix | 18 ---- nixos/modules/misc/assertions.nix | 6 -- 7 files changed, 27 insertions(+), 170 deletions(-) delete mode 100644 lib/tests/modules/assertions/enable-lazy.nix delete mode 100644 lib/tests/modules/assertions/non-cascading.nix delete mode 100644 lib/tests/modules/assertions/trigger-lazy.nix delete mode 100644 lib/tests/modules/assertions/trigger-submodule.nix (limited to 'lib/modules.nix') diff --git a/lib/modules.nix b/lib/modules.nix index 9aa638231bf..2c827a01e01 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -192,32 +192,6 @@ rec { Enabling both ''${options.services.foo.enable} and ''${options.services.bar.enable} is not possible. ''; }; - - options.triggerPath = mkOption { - description = '' - The config path which when evaluated should - trigger this check. By default this is - [], meaning evaluating - config at all will trigger the check. - On NixOS this default is changed to - [ "system" "build" "toplevel" such that - only a system evaluation triggers the checks. - - Evaluating config 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. - - ''; - # 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; - - # 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); - - # 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); - - # 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); - - # 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; - - # 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; + # 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 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 = injectChecks checks (removeAttrs config [ "_module" ]); + finalConfig = triggerChecks (removeAttrs config [ "_module" ]); checkUnmatched = if config._module.check && config._module.freeformType == null && merged.unmatchedDefns != [] then diff --git a/lib/tests/modules.sh b/lib/tests/modules.sh index 43bcabdf816..9e85c90d15c 100755 --- a/lib/tests/modules.sh +++ b/lib/tests/modules.sh @@ -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 = ; }' 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 diff --git a/lib/tests/modules/assertions/enable-lazy.nix b/lib/tests/modules/assertions/enable-lazy.nix deleted file mode 100644 index e2519022864..00000000000 --- a/lib/tests/modules/assertions/enable-lazy.nix +++ /dev/null @@ -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" ]; - }; - -} diff --git a/lib/tests/modules/assertions/non-cascading.nix b/lib/tests/modules/assertions/non-cascading.nix deleted file mode 100644 index 7b9e333a11a..00000000000 --- a/lib/tests/modules/assertions/non-cascading.nix +++ /dev/null @@ -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" ]; - }; - -} diff --git a/lib/tests/modules/assertions/trigger-lazy.nix b/lib/tests/modules/assertions/trigger-lazy.nix deleted file mode 100644 index 9e9e3683b0c..00000000000 --- a/lib/tests/modules/assertions/trigger-lazy.nix +++ /dev/null @@ -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" ]; - }; -} diff --git a/lib/tests/modules/assertions/trigger-submodule.nix b/lib/tests/modules/assertions/trigger-submodule.nix deleted file mode 100644 index 27deb48e4a9..00000000000 --- a/lib/tests/modules/assertions/trigger-submodule.nix +++ /dev/null @@ -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" ]; - }; - -} diff --git a/nixos/modules/misc/assertions.nix b/nixos/modules/misc/assertions.nix index e8b1f5afca3..6a26a2332f2 100644 --- a/nixos/modules/misc/assertions.nix +++ b/nixos/modules/misc/assertions.nix @@ -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: -- cgit 1.4.1 From 991dfccbd1935aabb76a20245ca0108aadd38f3c Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Mon, 30 Nov 2020 22:10:48 +0100 Subject: lib/modules: _module.check should always be internal Honestly this option should probably just be removed --- lib/modules.nix | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) (limited to 'lib/modules.nix') diff --git a/lib/modules.nix b/lib/modules.nix index 2c827a01e01..23dbe962491 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -73,19 +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 { # FIXME: Using ./modules.nix directly breaks the doc for some reason _file = "lib/modules.nix"; key = _file; - # 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 + # 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 @@ -101,7 +102,7 @@ rec { _module.check = mkOption { type = types.bool; - internal = prefix != []; + internal = true; default = check; description = '' Whether to check whether all option definitions have matching -- cgit 1.4.1 From 767d80099cd8418b3cc7338eb24f9217fedb6449 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Mon, 30 Nov 2020 22:38:56 +0100 Subject: 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.nix | 36 +++++++++++----------- lib/tests/modules.sh | 3 +- lib/tests/modules/assertions/condition-true.nix | 8 +++++ lib/tests/modules/assertions/enable-false.nix | 1 + lib/tests/modules/assertions/multi.nix | 8 ++--- lib/tests/modules/assertions/simple.nix | 2 +- .../assertions/submodule-attrsOf-attrsOf.nix | 2 +- lib/tests/modules/assertions/submodule-attrsOf.nix | 2 +- lib/tests/modules/assertions/submodule.nix | 2 +- .../modules/assertions/underscore-attributes.nix | 2 +- lib/tests/modules/assertions/warning.nix | 2 +- nixos/doc/manual/development/assertions.xml | 34 ++++++++++---------- nixos/modules/misc/assertions.nix | 2 +- 13 files changed, 56 insertions(+), 48 deletions(-) create mode 100644 lib/tests/modules/assertions/condition-true.nix (limited to 'lib/modules.nix') diff --git a/lib/modules.nix b/lib/modules.nix index 23dbe962491..468c373d6aa 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -155,17 +155,22 @@ rec { default = {}; internal = prefix != []; type = types.attrsOf (types.submodule { - # TODO: Rename to assertion? Or allow also setting assertion? options.enable = mkOption { description = '' - Whether to enable this check. - - This is the inverse of asserting a condition: If a certain - condition should be true, then this - option should be set to false when that - case occurs - + 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; }; @@ -189,9 +194,7 @@ rec { and use ''${options.path.to.option}. ''; type = types.str; - example = literalExample '' - Enabling both ''${options.services.foo.enable} and ''${options.services.bar.enable} is not possible. - ''; + example = "Enabling both \${options.services.foo.enable} and \${options.services.bar.enable} is not possible."; }; }); }; @@ -244,7 +247,7 @@ rec { if lib.hasPrefix "_" name then value.message else "[${showOption prefix}${optionalString (prefix != []) "/"}${name}] ${value.message}"; in - if ! value.enable then errors + 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}"; @@ -885,8 +888,7 @@ rec { }); config._module.checks = let opt = getAttrFromPath optionName options; in { - ${showOption optionName} = { - enable = mkDefault opt.isDefined; + ${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} @@ -958,8 +960,7 @@ rec { let val = getAttrFromPath f config; opt = getAttrFromPath f options; in { - ${showOption f} = { - enable = mkDefault (val != "_mkMergedOptionModule"); + ${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."; }; @@ -1024,8 +1025,7 @@ rec { }); config = mkMerge [ { - _module.checks.${showOption from} = { - enable = mkDefault (warn && fromOpt.isDefined); + _module.checks.${showOption from} = mkIf (warn && fromOpt.isDefined) { type = "warning"; message = "The option `${showOption from}' defined in ${showFiles fromOpt.files} has been renamed to `${showOption to}'."; }; diff --git a/lib/tests/modules.sh b/lib/tests/modules.sh index 9e85c90d15c..775be9f7209 100755 --- a/lib/tests/modules.sh +++ b/lib/tests/modules.sh @@ -279,7 +279,8 @@ checkConfigOutput baz config.value.nested.bar.baz ./types-anything/mk-mods.nix # 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 +# 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 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 index c326c086f03..11f753bb32e 100644 --- a/lib/tests/modules/assertions/enable-false.nix +++ b/lib/tests/modules/assertions/enable-false.nix @@ -2,6 +2,7 @@ _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 index ebbe17f3a55..1e2e14b8643 100644 --- a/lib/tests/modules/assertions/multi.nix +++ b/lib/tests/modules/assertions/multi.nix @@ -2,20 +2,20 @@ _module.checks = { test1 = { - enable = true; + check = false; message = "Assertion 1 failed"; }; test2 = { - enable = true; + check = false; message = "Assertion 2 failed"; }; test3 = { - enable = true; + check = false; message = "Warning 3 failed"; type = "warning"; }; test4 = { - enable = true; + check = false; message = "Warning 4 failed"; type = "warning"; }; diff --git a/lib/tests/modules/assertions/simple.nix b/lib/tests/modules/assertions/simple.nix index a63b8090f91..115d89a3036 100644 --- a/lib/tests/modules/assertions/simple.nix +++ b/lib/tests/modules/assertions/simple.nix @@ -1,6 +1,6 @@ { _module.checks.test = { - enable = true; + 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 index a5f92aa93c7..27a63d1e432 100644 --- a/lib/tests/modules/assertions/submodule-attrsOf-attrsOf.nix +++ b/lib/tests/modules/assertions/submodule-attrsOf-attrsOf.nix @@ -4,7 +4,7 @@ default = { bar.baz = {}; }; type = lib.types.attrsOf (lib.types.attrsOf (lib.types.submodule { _module.checks.test = { - enable = true; + check = false; message = "Assertion failed"; }; })); diff --git a/lib/tests/modules/assertions/submodule-attrsOf.nix b/lib/tests/modules/assertions/submodule-attrsOf.nix index 450cad0804d..aac5937cf7e 100644 --- a/lib/tests/modules/assertions/submodule-attrsOf.nix +++ b/lib/tests/modules/assertions/submodule-attrsOf.nix @@ -4,7 +4,7 @@ default = { bar = {}; }; type = lib.types.attrsOf (lib.types.submodule { _module.checks.test = { - enable = true; + check = false; message = "Assertion failed"; }; }); diff --git a/lib/tests/modules/assertions/submodule.nix b/lib/tests/modules/assertions/submodule.nix index a46734a326b..4e7e0b1bd61 100644 --- a/lib/tests/modules/assertions/submodule.nix +++ b/lib/tests/modules/assertions/submodule.nix @@ -4,7 +4,7 @@ default = {}; type = lib.types.submodule { _module.checks.test = { - enable = true; + check = false; message = "Assertion failed"; }; }; diff --git a/lib/tests/modules/assertions/underscore-attributes.nix b/lib/tests/modules/assertions/underscore-attributes.nix index c28c9dcd918..f9ee5c5787b 100644 --- a/lib/tests/modules/assertions/underscore-attributes.nix +++ b/lib/tests/modules/assertions/underscore-attributes.nix @@ -1,7 +1,7 @@ { _module.checks._test = { - enable = true; + check = false; message = "Assertion failed"; }; diff --git a/lib/tests/modules/assertions/warning.nix b/lib/tests/modules/assertions/warning.nix index 8fed9871aa2..72598ba3fdd 100644 --- a/lib/tests/modules/assertions/warning.nix +++ b/lib/tests/modules/assertions/warning.nix @@ -1,7 +1,7 @@ { _module.checks.test = { - enable = true; + check = false; type = "warning"; message = "Warning message"; }; diff --git a/nixos/doc/manual/development/assertions.xml b/nixos/doc/manual/development/assertions.xml index a873345ef43..31d09f958af 100644 --- a/nixos/doc/manual/development/assertions.xml +++ b/nixos/doc/manual/development/assertions.xml @@ -25,28 +25,26 @@ Checks can be defined using the option. - Each check needs an attribute name, under which you have to define an enable - condition using and a - message using . Note that - the enable condition is inverse of what an assertion - would be: To assert a value being true, the enable condition should be false - in that case, so that it isn't triggered. For the check message, you can add + Each check needs an attribute name, under which you can define a trigger + assertion using and a + message using . + For the message, you can add options to the module arguments and use ${options.path.to.option} to print a context-aware string - representation of the option path. Here is an example showing how this can be + representation of an option path. Here is an example showing how this can be done. { config, options, ... }: { _module.checks.gpgSshAgent = { - enable = config.programs.gnupg.agent.enableSSHSupport && config.programs.ssh.startAgent; - message = "You can't enable both ${options.programs.ssh.startAgent}" - + " and ${options.programs.gnupg.agent.enableSSHSupport}!"; + 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 = { - enable = config.services.grafana.database.password != ""; + 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 @@ -74,8 +72,8 @@ 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] You can't enable both programs.ssh.startAgent and - programs.gnupg.agent.enableSSHSupport! +- [gpgSshAgent] If you have programs.gnupg.agent.enableSSHSupport + enabled, you can't enable programs.ssh.startAgent as well! @@ -87,12 +85,12 @@ error: Failed checks: -{ 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 = lib.mkForce false; + _module.checks.grafanaPassword.enable = false; } @@ -113,7 +111,7 @@ error: Failed checks: options.port = lib.mkOption {}; config._module.checks.portConflict = { - enable = config.port == 80; + check = config.port != 80; message = "Port ${toString config.port} defined using" + " ${options.port} is usually used for HTTP"; type = "warning"; @@ -143,8 +141,8 @@ trace: warning: [myServices.foo/portConflict] Port 80 defined using -{ lib, ... }: { - myServices.foo._module.checks.portConflict.enable = lib.mkForce false; +{ + myServices.foo._module.checks.portConflict.enable = false; } diff --git a/nixos/modules/misc/assertions.nix b/nixos/modules/misc/assertions.nix index 6a26a2332f2..d7cdb32491d 100644 --- a/nixos/modules/misc/assertions.nix +++ b/nixos/modules/misc/assertions.nix @@ -36,7 +36,7 @@ with lib; name = "_${toString n}"; isWarning = lib.isString value; result = { - enable = if isWarning then true else ! value.assertion; + check = if isWarning then false else value.assertion; type = if isWarning then "warning" else "error"; message = if isWarning then value else value.message; }; -- cgit 1.4.1 From a6a70d14a9f7b885e65a51c5e6bd02145884ee50 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 17 Dec 2020 21:50:51 +0100 Subject: lib/modules: Prefix mkRemovedOptionModule & co. check names To avoid name clashes Co-authored-by: Robert Hensing --- lib/modules.nix | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'lib/modules.nix') diff --git a/lib/modules.nix b/lib/modules.nix index 468c373d6aa..5548c5f7049 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -888,7 +888,7 @@ rec { }); config._module.checks = let opt = getAttrFromPath optionName options; in { - ${showOption optionName} = lib.mkIf opt.isDefined { + ${"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} @@ -960,7 +960,7 @@ rec { let val = getAttrFromPath f config; opt = getAttrFromPath f options; in { - ${showOption f} = lib.mkIf (val != "_mkMergedOptionModule") { + ${"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."; }; @@ -1025,7 +1025,7 @@ rec { }); config = mkMerge [ { - _module.checks.${showOption from} = mkIf (warn && fromOpt.isDefined) { + _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}'."; }; -- cgit 1.4.1 From 9e6737710c4fb2613850e699178b23d54f1a3261 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Fri, 18 Dec 2020 16:42:42 +0100 Subject: Revert "Module-builtin assertions, disabling assertions and submodule assertions" --- lib/modules.nix | 164 +++------------------ lib/options.nix | 2 +- lib/tests/misc.nix | 2 +- lib/tests/modules.sh | 78 +++------- lib/tests/modules/assertions/condition-true.nix | 8 - lib/tests/modules/assertions/enable-false.nix | 9 -- lib/tests/modules/assertions/multi.nix | 23 --- lib/tests/modules/assertions/simple.nix | 6 - .../assertions/submodule-attrsOf-attrsOf.nix | 13 -- lib/tests/modules/assertions/submodule-attrsOf.nix | 13 -- lib/tests/modules/assertions/submodule.nix | 13 -- .../modules/assertions/underscore-attributes.nix | 8 - lib/tests/modules/assertions/warning.nix | 9 -- nixos/doc/manual/development/assertions.xml | 159 +++++--------------- nixos/modules/misc/assertions.nix | 15 +- nixos/modules/system/activation/top-level.nix | 10 +- 16 files changed, 93 insertions(+), 439 deletions(-) delete mode 100644 lib/tests/modules/assertions/condition-true.nix delete mode 100644 lib/tests/modules/assertions/enable-false.nix delete mode 100644 lib/tests/modules/assertions/multi.nix delete mode 100644 lib/tests/modules/assertions/simple.nix delete mode 100644 lib/tests/modules/assertions/submodule-attrsOf-attrsOf.nix delete mode 100644 lib/tests/modules/assertions/submodule-attrsOf.nix delete mode 100644 lib/tests/modules/assertions/submodule.nix delete mode 100644 lib/tests/modules/assertions/underscore-attributes.nix delete mode 100644 lib/tests/modules/assertions/warning.nix (limited to 'lib/modules.nix') diff --git a/lib/modules.nix b/lib/modules.nix index 5548c5f7049..3f2bfd478b0 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -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 - ''; + 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 - "error" type will cause evaluation to fail, - while the "warning" 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 - options to the module function arguments - and use ''${options.path.to.option}. - ''; - 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 diff --git a/lib/options.nix b/lib/options.nix index 5c042a6c6f2..87cd8b79796 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 && ! docOption.internal) subOptions) (collect isOption options); + [ docOption ] ++ optionals docOption.visible 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 2d53ed81176..35a5801c724 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 (removeAttrs options [ "_module" ])); + locs = filter (o: ! o.internal) (optionAttrSetToDocList options); in map (o: o.loc) locs; expected = [ [ "foo" ] [ "foo" "" "bar" ] [ "foo" "bar" ] ]; }; diff --git a/lib/tests/modules.sh b/lib/tests/modules.sh index 775be9f7209..309c5311361 100755 --- a/lib/tests/modules.sh +++ b/lib/tests/modules.sh @@ -27,50 +27,37 @@ reportFailure() { fail=$((fail + 1)) } -checkConfigCodeOutErr() { - local expectedExit=$1 - shift; +checkConfigOutput() { 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" + 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 - - 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; - 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 < - Evaluation Checks + Warnings and Assertions 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. Although Nix has the abort and builtins.trace functions - 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. -
- Defining Checks +
+ Warnings - Checks can be defined using the option. - Each check needs an attribute name, under which you can define a trigger - assertion using and a - message using . - For the message, you can add - options to the module arguments and use - ${options.path.to.option} to print a context-aware string - representation of an option path. Here is an example showing how this can be - done. - - - -{ 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"; - }; -} - - -
- -
- Ignoring Checks - - - 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 by - using the attribute name of the definition, which is conveniently printed - using [...] when the check is triggered. For above - example, the evaluation output when the checks are triggered looks as - follows: - - - -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! - - - - The [grafanaPassword] and [gpgSshAgent] - strings tell you that these were defined under the grafanaPassword - and gpgSshAgent attributes of - respectively. With this knowledge - you can adjust them to your liking: + This is an example of using warnings. + - -
-
- Checks in Submodules - - Evaluation checks can be defined within submodules in the same way. Here is an example: - - - -{ 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"; - }; - })); - }; - -} - +
+ Assertions - When this check is triggered, it shows both the submodule path along with - the check attribute within that submodule, joined by a - /. Note also how ${options.port} - correctly shows the context of the option. - - - -trace: warning: [myServices.foo/portConflict] Port 80 defined using - myServices.foo.port is usually used for HTTP - - - - Therefore to disable such a check, you can do so by changing the - option within the - myServices.foo submodule: + This example, extracted from the + + syslogd module shows how to use + assertions. 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. + - - - - Checks defined in submodules under types.listOf can't be - ignored, since there's no way to change previously defined list items. - - -
diff --git a/nixos/modules/misc/assertions.nix b/nixos/modules/misc/assertions.nix index d7cdb32491d..550b3ac97f6 100644 --- a/nixos/modules/misc/assertions.nix +++ b/nixos/modules/misc/assertions.nix @@ -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 } diff --git a/nixos/modules/system/activation/top-level.nix b/nixos/modules/system/activation/top-level.nix index 17b62ad9569..03d7e749323 100644 --- a/nixos/modules/system/activation/top-level.nix +++ b/nixos/modules/system/activation/top-level.nix @@ -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 -- cgit 1.4.1 From 7f2fcc45f734c1e73cef2d7063562b9e411c2614 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 21 Jan 2021 21:57:48 +0100 Subject: lib/modules: Set submodule type for renamed option sets For renames like mkAliasOptionModule [ "services" "compton" ] [ "services" "picom" ] where the target is an option set (like services.picom) instead of a single option (like services.picom.enable), previously the renamed option type was unset, leading to it being `types.unspecified`. This changes it to be `types.submodule {}` instead, which makes more sense. --- lib/modules.nix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/modules.nix') diff --git a/lib/modules.nix b/lib/modules.nix index 3f2bfd478b0..33a0d84a6d7 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -895,7 +895,7 @@ rec { fromOpt = getAttrFromPath from options; toOf = attrByPath to (abort "Renaming error: option `${showOption to}' does not exist."); - toType = let opt = attrByPath to {} options; in opt.type or null; + toType = let opt = attrByPath to {} options; in opt.type or (types.submodule {}); in { options = setAttrByPath from (mkOption { -- cgit 1.4.1 From e878fc4aac3a9cecf5f6509fd887824c52a20af3 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Thu, 11 Mar 2021 11:57:38 +0100 Subject: lib/modules: better error message if an attr-set of options is expected I recently wrote some Nix code where I wrongly set a value to an option which wasn't an actual option, but an attr-set of options. The mistake I made can be demonstrated with an expression like this: { foo = { lib, pkgs, config, ... }: with lib; { options.foo.bar.baz = mkOption { type = types.str; }; config.foo.bar = 23; }; } While it wasn't too hard to find the cause of the mistake for me, it was necessary to have some practice in reading stack traces from the module system since the eval-error I got was not very helpful: error: --- TypeError --------------------------------------------------------- nix-build at: (323:25) in file: /nix/store/3nm31brdz95pj8gch5gms6xwqh0xx55c-source/lib/modules.nix 322| foldl' (acc: module: 323| acc // (mapAttrs (n: v: | ^ 324| (acc.${n} or []) ++ f module v value is an integer while a set was expected (use '--show-trace' to show detailed location information) I figured that such an error can be fairly confusing for someone who's new to NixOS, so I decided to catch this case in th `byName` function in `lib/modules.nix` by checking if the value to map through is an actual attr-set. If not, a different error will be thrown. --- lib/modules.nix | 11 +++++++++++ lib/tests/modules.sh | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-) (limited to 'lib/modules.nix') diff --git a/lib/modules.nix b/lib/modules.nix index 33a0d84a6d7..d3f10944e70 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -361,6 +361,17 @@ rec { */ byName = attr: f: modules: foldl' (acc: module: + if !(builtins.isAttrs module.${attr}) then + throw '' + You're trying to declare a value of type `${builtins.typeOf module.${attr}}' + rather than an attribute-set for the option + `${builtins.concatStringsSep "." prefix}'! + + This usually happens if `${builtins.concatStringsSep "." prefix}' has option + definitions inside that are not matched. Please check how to properly define + this option by e.g. referring to `man 5 configuration.nix'! + '' + else acc // (mapAttrs (n: v: (acc.${n} or []) ++ f module v ) module.${attr} diff --git a/lib/tests/modules.sh b/lib/tests/modules.sh index f843d303e44..2eddeec07b1 100755 --- a/lib/tests/modules.sh +++ b/lib/tests/modules.sh @@ -169,7 +169,7 @@ checkConfigOutput "foo" config.submodule.foo ./declare-submoduleWith-special.nix ## shorthandOnlyDefines config behaves as expected checkConfigOutput "true" config.submodule.config ./declare-submoduleWith-shorthand.nix ./define-submoduleWith-shorthand.nix checkConfigError 'is not of type `boolean' config.submodule.config ./declare-submoduleWith-shorthand.nix ./define-submoduleWith-noshorthand.nix -checkConfigError 'value is a boolean while a set was expected' config.submodule.config ./declare-submoduleWith-noshorthand.nix ./define-submoduleWith-shorthand.nix +checkConfigError "You're trying to declare a value of type \`bool'\nrather than an attribute-set for the option" config.submodule.config ./declare-submoduleWith-noshorthand.nix ./define-submoduleWith-shorthand.nix checkConfigOutput "true" config.submodule.config ./declare-submoduleWith-noshorthand.nix ./define-submoduleWith-noshorthand.nix ## submoduleWith should merge all modules in one swoop -- cgit 1.4.1 From a8afbb45c12be27b9b93a802e9e276595899ed5a Mon Sep 17 00:00:00 2001 From: Alyssa Ross Date: Tue, 27 Apr 2021 13:52:15 +0000 Subject: treewide: use lib.warnIf where appropriate --- lib/modules.nix | 6 +++--- lib/strings.nix | 4 ++-- nixos/lib/testing-python.nix | 4 +--- pkgs/applications/networking/browsers/chromium/common.nix | 6 ++---- pkgs/development/perl-modules/generic/default.nix | 6 ++---- pkgs/servers/jellyfin/default.nix | 5 ++--- 6 files changed, 12 insertions(+), 19 deletions(-) (limited to 'lib/modules.nix') diff --git a/lib/modules.nix b/lib/modules.nix index d3f10944e70..d515ee24d16 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -37,7 +37,7 @@ let setAttrByPath toList types - warn + warnIf ; inherit (lib.options) isOption @@ -516,8 +516,8 @@ rec { value = if opt ? apply then opt.apply res.mergedValue else res.mergedValue; warnDeprecation = - if opt.type.deprecationMessage == null then id - else warn "The type `types.${opt.type.name}' of option `${showOption loc}' defined in ${showFiles opt.declarations} is deprecated. ${opt.type.deprecationMessage}"; + warnIf (opt.type.deprecationMessage != null) + "The type `types.${opt.type.name}' of option `${showOption loc}' defined in ${showFiles opt.declarations} is deprecated. ${opt.type.deprecationMessage}"; in warnDeprecation opt // { value = builtins.addErrorContext "while evaluating the option `${showOption loc}':" value; diff --git a/lib/strings.nix b/lib/strings.nix index 5010d9159cb..0f23b6b9d41 100644 --- a/lib/strings.nix +++ b/lib/strings.nix @@ -644,8 +644,8 @@ rec { floatToString = float: let result = toString float; precise = float == fromJSON result; - in if precise then result - else lib.warn "Imprecise conversion from float to string ${result}" result; + in lib.warnIf (!precise) "Imprecise conversion from float to string ${result}" + result; /* Check whether a value can be coerced to a string */ isCoercibleToString = x: diff --git a/nixos/lib/testing-python.nix b/nixos/lib/testing-python.nix index 6192be1cd05..c7e45f55ce1 100644 --- a/nixos/lib/testing-python.nix +++ b/nixos/lib/testing-python.nix @@ -131,10 +131,8 @@ rec { "it's currently ${toString testNameLen} characters long.") else "nixos-test-driver-${name}"; - - warn = if skipLint then lib.warn "Linting is disabled!" else lib.id; in - warn (runCommand testDriverName + lib.warnIf skipLint "Linting is disabled" (runCommand testDriverName { buildInputs = [ makeWrapper ]; testScript = testScript'; diff --git a/pkgs/applications/networking/browsers/chromium/common.nix b/pkgs/applications/networking/browsers/chromium/common.nix index b08ff1ac7c1..73ce022915c 100644 --- a/pkgs/applications/networking/browsers/chromium/common.nix +++ b/pkgs/applications/networking/browsers/chromium/common.nix @@ -112,10 +112,8 @@ let warnObsoleteVersionConditional = min-version: result: let ungoogled-version = (importJSON ./upstream-info.json).ungoogled-chromium.version; - in if versionAtLeast ungoogled-version min-version - then warn "chromium: ungoogled version ${ungoogled-version} is newer than a conditional bounded at ${min-version}. You can safely delete it." - result - else result; + in warnIf (versionAtLeast ungoogled-version min-version) "chromium: ungoogled version ${ungoogled-version} is newer than a conditional bounded at ${min-version}. You can safely delete it." + result; chromiumVersionAtLeast = min-version: let result = versionAtLeast upstream-info.version min-version; in warnObsoleteVersionConditional min-version result; diff --git a/pkgs/development/perl-modules/generic/default.nix b/pkgs/development/perl-modules/generic/default.nix index c7b57eae906..9beacd65a64 100644 --- a/pkgs/development/perl-modules/generic/default.nix +++ b/pkgs/development/perl-modules/generic/default.nix @@ -5,10 +5,8 @@ assert attrs?pname -> attrs?version; assert attrs?pname -> !(attrs?name); -(if attrs ? name then - lib.trivial.warn "builtPerlPackage: `name' (\"${attrs.name}\") is deprecated, use `pname' and `version' instead" - else - (x: x)) +lib.warnIf (attrs ? name) "builtPerlPackage: `name' (\"${attrs.name}\") is deprecated, use `pname' and `version' instead" + toPerlModule(stdenv.mkDerivation ( ( lib.recursiveUpdate diff --git a/pkgs/servers/jellyfin/default.nix b/pkgs/servers/jellyfin/default.nix index 2b00cb50073..77406c46415 100644 --- a/pkgs/servers/jellyfin/default.nix +++ b/pkgs/servers/jellyfin/default.nix @@ -11,9 +11,8 @@ let else if isAarch64 then "arm64" else lib.warn "Unsupported architecture, some image processing features might be unavailable" "unknown"; musl = lib.optionalString stdenv.hostPlatform.isMusl - (if (arch != "x64") - then lib.warn "Some image processing features might be unavailable for non x86-64 with Musl" "musl-" - else "musl-"); + (lib.warnIf (arch != "x64") "Some image processing features might be unavailable for non x86-64 with Musl" + "musl-"); runtimeDir = "${os}-${musl}${arch}"; in stdenv.mkDerivation rec { -- cgit 1.4.1 From 4b54aedee5e05aaf2838f6d951508b83e33d2baa Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Wed, 30 Sep 2020 01:12:30 +0200 Subject: lib/modules: Issue type deprecation warnings recursively Previously, an option of type attrsOf string wouldn't throw a deprecation warning, even though the string type is deprecated. This was because the deprecation warning trigger only looked at the type of the option itself, not any of its subtypes. This commit fixes this, causing each of the types deprecationMessages to trigger for the option. This relies on the subtypes mkOptionType attribute introduced in 26607a5a2e06653fec453c83d063cdfc4b59185f --- lib/modules.nix | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) (limited to 'lib/modules.nix') diff --git a/lib/modules.nix b/lib/modules.nix index d515ee24d16..04b65d791b5 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -515,11 +515,20 @@ rec { # yield a value computed from the definitions value = if opt ? apply then opt.apply res.mergedValue else res.mergedValue; - warnDeprecation = - warnIf (opt.type.deprecationMessage != null) - "The type `types.${opt.type.name}' of option `${showOption loc}' defined in ${showFiles opt.declarations} is deprecated. ${opt.type.deprecationMessage}"; + # Issue deprecation warnings recursively over all nested types of the + # given type. But don't recurse if a type with the same name was already + # visited before in order to prevent infinite recursion. So this only + # warns once per type name. + # Returns the new set of visited type names + recursiveWarn = visited: type: + let + maybeWarn = warnIf (type.deprecationMessage != null) + "The type `types.${type.name}' of option `${showOption loc}' defined in ${showFiles opt.declarations} is deprecated. ${type.deprecationMessage}"; + in + if visited ? ${type.name} then visited + else lib.foldl' recursiveWarn (maybeWarn visited // { ${type.name} = null; }) (lib.attrValues type.nestedTypes); - in warnDeprecation opt // + in builtins.seq (recursiveWarn {} opt.type) opt // { value = builtins.addErrorContext "while evaluating the option `${showOption loc}':" value; inherit (res.defsFinal') highestPrio; definitions = map (def: def.value) res.defsFinal; -- cgit 1.4.1 From a36e6760e9be0ec260b637a06d751d39e2a78e4e Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Wed, 5 May 2021 18:53:34 +0200 Subject: Revert "lib/modules: Issue type deprecation warnings recursively" This reverts commit 4b54aedee5e05aaf2838f6d951508b83e33d2baa. --- lib/modules.nix | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) (limited to 'lib/modules.nix') diff --git a/lib/modules.nix b/lib/modules.nix index 04b65d791b5..d515ee24d16 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -515,20 +515,11 @@ rec { # yield a value computed from the definitions value = if opt ? apply then opt.apply res.mergedValue else res.mergedValue; - # Issue deprecation warnings recursively over all nested types of the - # given type. But don't recurse if a type with the same name was already - # visited before in order to prevent infinite recursion. So this only - # warns once per type name. - # Returns the new set of visited type names - recursiveWarn = visited: type: - let - maybeWarn = warnIf (type.deprecationMessage != null) - "The type `types.${type.name}' of option `${showOption loc}' defined in ${showFiles opt.declarations} is deprecated. ${type.deprecationMessage}"; - in - if visited ? ${type.name} then visited - else lib.foldl' recursiveWarn (maybeWarn visited // { ${type.name} = null; }) (lib.attrValues type.nestedTypes); + warnDeprecation = + warnIf (opt.type.deprecationMessage != null) + "The type `types.${opt.type.name}' of option `${showOption loc}' defined in ${showFiles opt.declarations} is deprecated. ${opt.type.deprecationMessage}"; - in builtins.seq (recursiveWarn {} opt.type) opt // + in warnDeprecation opt // { value = builtins.addErrorContext "while evaluating the option `${showOption loc}':" value; inherit (res.defsFinal') highestPrio; definitions = map (def: def.value) res.defsFinal; -- cgit 1.4.1 From 810c9c6a0ea9a021426acf8ae3f2bcfbde545ef7 Mon Sep 17 00:00:00 2001 From: Nicolas Berbiche Date: Wed, 3 Mar 2021 00:59:52 -0500 Subject: lib/modules: provide error message when imports contains a list --- lib/modules.nix | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'lib/modules.nix') diff --git a/lib/modules.nix b/lib/modules.nix index 04b65d791b5..fb6fe48395d 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -23,6 +23,7 @@ let isAttrs isBool isFunction + isList isString length mapAttrs @@ -188,6 +189,9 @@ rec { loadModule = args: fallbackFile: fallbackKey: m: if isFunction m || isAttrs m then unifyModuleSyntax fallbackFile fallbackKey (applyIfFunction fallbackKey m args) + else if isList m then + let defs = [{ file = fallbackFile; value = m; }]; in + throw "Module imports can't be nested lists. Perhaps you meant to remove one level of lists? Definitions: ${showDefs defs}" else unifyModuleSyntax (toString m) (toString m) (applyIfFunction (toString m) (import m) args); /* -- cgit 1.4.1 From 98c77a0b2d7c10fc3ef811e9346d1e3cb99f1b32 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 6 May 2021 04:59:27 +0200 Subject: lib/modules: Small optimization --- lib/modules.nix | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) (limited to 'lib/modules.nix') diff --git a/lib/modules.nix b/lib/modules.nix index 6b3faa447be..4b02d6aee2f 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -299,13 +299,11 @@ rec { # a module will resolve strictly the attributes used as argument but # not their values. The values are forwarding the result of the # evaluation of the option. - requiredArgs = builtins.attrNames (lib.functionArgs f); context = name: ''while evaluating the module argument `${name}' in "${key}":''; - extraArgs = builtins.listToAttrs (map (name: { - inherit name; - value = builtins.addErrorContext (context name) - (args.${name} or config._module.args.${name}); - }) requiredArgs); + extraArgs = builtins.mapAttrs (name: _: + builtins.addErrorContext (context name) + (args.${name} or config._module.args.${name}) + ) (lib.functionArgs f); # Note: we append in the opposite order such that we can add an error # context on the explicited arguments of "args" too. This update -- cgit 1.4.1 From c949e6022078188e2fbcf0d9306c4d9cf018876e Mon Sep 17 00:00:00 2001 From: Pacman99 Date: Wed, 5 May 2021 18:42:52 -0700 Subject: lib/modules: pass specialArgs as a module argument --- lib/modules.nix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/modules.nix') diff --git a/lib/modules.nix b/lib/modules.nix index d515ee24d16..cc38e4cba87 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -127,7 +127,7 @@ rec { let collected = collectModules (specialArgs.modulesPath or "") (modules ++ [ internalModule ]) - ({ inherit lib options config; } // specialArgs); + ({ inherit lib options config specialArgs; } // specialArgs); in mergeModules prefix (reverseList collected); options = merged.matchedOptions; -- cgit 1.4.1 From 8fb9984690c878fcd768e967190957441de05d11 Mon Sep 17 00:00:00 2001 From: Janne Heß Date: Sun, 6 Jun 2021 20:41:37 +0200 Subject: lib/modules: Drop mkStrict and mkFixStrictness This was deprecated in 2014 and is not used anywhere in the tree. --- lib/default.nix | 4 ++-- lib/modules.nix | 4 ---- 2 files changed, 2 insertions(+), 6 deletions(-) (limited to 'lib/modules.nix') diff --git a/lib/default.nix b/lib/default.nix index ccae0bbc3ab..ccfee2ebe30 100644 --- a/lib/default.nix +++ b/lib/default.nix @@ -115,8 +115,8 @@ let mergeModules' mergeOptionDecls evalOptionValue mergeDefinitions pushDownProperties dischargeProperties filterOverrides sortProperties fixupOptionType mkIf mkAssert mkMerge mkOverride - mkOptionDefault mkDefault mkForce mkVMOverride mkStrict - mkFixStrictness mkOrder mkBefore mkAfter mkAliasDefinitions + mkOptionDefault mkDefault mkForce mkVMOverride + mkOrder mkBefore mkAfter mkAliasDefinitions mkAliasAndWrapDefinitions fixMergeModules mkRemovedOptionModule mkRenamedOptionModule mkMergedOptionModule mkChangedOptionModule mkAliasOptionModule doRename; diff --git a/lib/modules.nix b/lib/modules.nix index 99b9a8a31ea..58c6cda58e4 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -713,10 +713,6 @@ rec { mkForce = mkOverride 50; mkVMOverride = mkOverride 10; # used by ‘nixos-rebuild build-vm’ - mkStrict = builtins.trace "`mkStrict' is obsolete; use `mkOverride 0' instead." (mkOverride 0); - - mkFixStrictness = id; # obsolete, no-op - mkOrder = priority: content: { _type = "order"; inherit priority content; -- cgit 1.4.1 From 99bc203025a0ff1265eedc6ff3d6c7aa1f320c09 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Mon, 12 Jul 2021 07:23:45 +0200 Subject: Partially revert "lib/modules: Drop mkStrict and mkFixStrictness" mkFixStrictness was never properly deprecated and should only be removed after warning for some time. This partially reverts commit 8fb9984690c878fcd768e967190957441de05d11. --- lib/default.nix | 4 ++-- lib/modules.nix | 4 ++++ 2 files changed, 6 insertions(+), 2 deletions(-) (limited to 'lib/modules.nix') diff --git a/lib/default.nix b/lib/default.nix index ccfee2ebe30..ccae0bbc3ab 100644 --- a/lib/default.nix +++ b/lib/default.nix @@ -115,8 +115,8 @@ let mergeModules' mergeOptionDecls evalOptionValue mergeDefinitions pushDownProperties dischargeProperties filterOverrides sortProperties fixupOptionType mkIf mkAssert mkMerge mkOverride - mkOptionDefault mkDefault mkForce mkVMOverride - mkOrder mkBefore mkAfter mkAliasDefinitions + mkOptionDefault mkDefault mkForce mkVMOverride mkStrict + mkFixStrictness mkOrder mkBefore mkAfter mkAliasDefinitions mkAliasAndWrapDefinitions fixMergeModules mkRemovedOptionModule mkRenamedOptionModule mkMergedOptionModule mkChangedOptionModule mkAliasOptionModule doRename; diff --git a/lib/modules.nix b/lib/modules.nix index 58c6cda58e4..99b9a8a31ea 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -713,6 +713,10 @@ rec { mkForce = mkOverride 50; mkVMOverride = mkOverride 10; # used by ‘nixos-rebuild build-vm’ + mkStrict = builtins.trace "`mkStrict' is obsolete; use `mkOverride 0' instead." (mkOverride 0); + + mkFixStrictness = id; # obsolete, no-op + mkOrder = priority: content: { _type = "order"; inherit priority content; -- cgit 1.4.1 From cad20d8983a547c19e914ec5a2a6093b0c8e5a16 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Mon, 12 Jul 2021 07:31:20 +0200 Subject: lib.mkFixStrictness: Deprecate --- lib/default.nix | 2 +- lib/modules.nix | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) (limited to 'lib/modules.nix') diff --git a/lib/default.nix b/lib/default.nix index ccae0bbc3ab..8e29ef5c420 100644 --- a/lib/default.nix +++ b/lib/default.nix @@ -115,7 +115,7 @@ let mergeModules' mergeOptionDecls evalOptionValue mergeDefinitions pushDownProperties dischargeProperties filterOverrides sortProperties fixupOptionType mkIf mkAssert mkMerge mkOverride - mkOptionDefault mkDefault mkForce mkVMOverride mkStrict + mkOptionDefault mkDefault mkForce mkVMOverride mkFixStrictness mkOrder mkBefore mkAfter mkAliasDefinitions mkAliasAndWrapDefinitions fixMergeModules mkRemovedOptionModule mkRenamedOptionModule mkMergedOptionModule mkChangedOptionModule diff --git a/lib/modules.nix b/lib/modules.nix index 99b9a8a31ea..ab2bc4f7f8e 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -713,9 +713,7 @@ rec { mkForce = mkOverride 50; mkVMOverride = mkOverride 10; # used by ‘nixos-rebuild build-vm’ - mkStrict = builtins.trace "`mkStrict' is obsolete; use `mkOverride 0' instead." (mkOverride 0); - - mkFixStrictness = id; # obsolete, no-op + mkFixStrictness = lib.warn "lib.mkFixStrictness has no effect and will be removed. It returns its argument unmodified, so you can just remove any calls." id; mkOrder = priority: content: { _type = "order"; -- cgit 1.4.1