Merge pull request #324268 from hercules-ci/modules-lib-hygiene
lib/modules: lib fix and tidying
This commit is contained in:
+1
-1
@@ -66,7 +66,7 @@ let
|
||||
|
||||
# TODO: For consistency, all builtins should also be available from a sub-library;
|
||||
# these are the only ones that are currently not
|
||||
inherit (builtins) addErrorContext isPath trace;
|
||||
inherit (builtins) addErrorContext isPath trace typeOf unsafeGetAttrPos;
|
||||
inherit (self.trivial) id const pipe concat or and xor bitAnd bitOr bitXor
|
||||
bitNot boolToString mergeAttrs flip mapNullable inNixShell isFloat min max
|
||||
importJSON importTOML warn warnIf warnIfNot throwIf throwIfNot checkListOfEnum
|
||||
|
||||
+53
-41
@@ -2,6 +2,7 @@
|
||||
|
||||
let
|
||||
inherit (lib)
|
||||
addErrorContext
|
||||
all
|
||||
any
|
||||
attrByPath
|
||||
@@ -13,13 +14,16 @@ let
|
||||
elem
|
||||
filter
|
||||
foldl'
|
||||
functionArgs
|
||||
getAttrFromPath
|
||||
genericClosure
|
||||
head
|
||||
id
|
||||
imap1
|
||||
isAttrs
|
||||
isBool
|
||||
isFunction
|
||||
isInOldestRelease
|
||||
isList
|
||||
isString
|
||||
length
|
||||
@@ -32,9 +36,17 @@ let
|
||||
optionalString
|
||||
recursiveUpdate
|
||||
reverseList sort
|
||||
seq
|
||||
setAttrByPath
|
||||
substring
|
||||
throwIfNot
|
||||
trace
|
||||
typeOf
|
||||
types
|
||||
unsafeGetAttrPos
|
||||
warn
|
||||
warnIf
|
||||
zipAttrs
|
||||
zipAttrsWith
|
||||
;
|
||||
inherit (lib.options)
|
||||
@@ -89,8 +101,8 @@ let
|
||||
}:
|
||||
let
|
||||
withWarnings = x:
|
||||
lib.warnIf (evalModulesArgs?args) "The args argument to evalModules is deprecated. Please set config._module.args instead."
|
||||
lib.warnIf (evalModulesArgs?check) "The check argument to evalModules is deprecated. Please set config._module.check instead."
|
||||
warnIf (evalModulesArgs?args) "The args argument to evalModules is deprecated. Please set config._module.args instead."
|
||||
warnIf (evalModulesArgs?check) "The check argument to evalModules is deprecated. Please set config._module.check instead."
|
||||
x;
|
||||
|
||||
legacyModules =
|
||||
@@ -265,9 +277,9 @@ let
|
||||
let
|
||||
optText = showOption (prefix ++ firstDef.prefix);
|
||||
defText =
|
||||
builtins.addErrorContext
|
||||
addErrorContext
|
||||
"while evaluating the error message for definitions for `${optText}', which is an option that does not exist"
|
||||
(builtins.addErrorContext
|
||||
(addErrorContext
|
||||
"while evaluating a definition from `${firstDef.file}'"
|
||||
( showDefs [ firstDef ])
|
||||
);
|
||||
@@ -298,7 +310,7 @@ let
|
||||
else throw baseMsg
|
||||
else null;
|
||||
|
||||
checked = builtins.seq checkUnmatched;
|
||||
checked = seq checkUnmatched;
|
||||
|
||||
extendModules = extendArgs@{
|
||||
modules ? [],
|
||||
@@ -312,7 +324,7 @@ let
|
||||
prefix = extendArgs.prefix or evalModulesArgs.prefix or [];
|
||||
});
|
||||
|
||||
type = lib.types.submoduleWith {
|
||||
type = types.submoduleWith {
|
||||
inherit modules specialArgs class;
|
||||
};
|
||||
|
||||
@@ -344,8 +356,8 @@ let
|
||||
else
|
||||
throw (
|
||||
"Could not load a value as a module, because it is of type ${lib.strings.escapeNixString m._type}"
|
||||
+ lib.optionalString (fallbackFile != unknownModule) ", in file ${toString fallbackFile}."
|
||||
+ lib.optionalString (m._type == "configuration") " If you do intend to import this configuration, please only import the modules that make up the configuration. You may have to create a `let` binding, file or attribute to give yourself access to the relevant modules.\nWhile loading a configuration into the module system is a very sensible idea, it can not be done cleanly in practice."
|
||||
+ optionalString (fallbackFile != unknownModule) ", in file ${toString fallbackFile}."
|
||||
+ optionalString (m._type == "configuration") " If you do intend to import this configuration, please only import the modules that make up the configuration. You may have to create a `let` binding, file or attribute to give yourself access to the relevant modules.\nWhile loading a configuration into the module system is a very sensible idea, it can not be done cleanly in practice."
|
||||
# Extended explanation: That's because a finalized configuration is more than just a set of modules. For instance, it has its own `specialArgs` that, by the nature of `specialArgs` can't be loaded through `imports` or the the `modules` argument. So instead, we have to ask you to extract the relevant modules and use those instead. This way, we keep the module system comparatively simple, and hopefully avoid a bad surprise down the line.
|
||||
)
|
||||
else if isList m then
|
||||
@@ -415,7 +427,7 @@ let
|
||||
moduleKey = file: m:
|
||||
if isString m
|
||||
then
|
||||
if builtins.substring 0 1 m == "/"
|
||||
if substring 0 1 m == "/"
|
||||
then m
|
||||
else toString modulesPath + "/" + m
|
||||
|
||||
@@ -433,11 +445,11 @@ let
|
||||
|
||||
else if isAttrs m
|
||||
then throw "Module `${file}` contains a disabledModules item that is an attribute set, presumably a module, that does not have a `key` attribute. This means that the module system doesn't have any means to identify the module that should be disabled. Make sure that you've put the correct value in disabledModules: a string path relative to modulesPath, a path value, or an attribute set with a `key` attribute."
|
||||
else throw "Each disabledModules item must be a path, string, or a attribute set with a key attribute, or a value supported by toString. However, one of the disabledModules items in `${toString file}` is none of that, but is of type ${builtins.typeOf m}.";
|
||||
else throw "Each disabledModules item must be a path, string, or a attribute set with a key attribute, or a value supported by toString. However, one of the disabledModules items in `${toString file}` is none of that, but is of type ${typeOf m}.";
|
||||
|
||||
disabledKeys = concatMap ({ file, disabled }: map (moduleKey file) disabled) disabled;
|
||||
keyFilter = filter (attrs: ! elem attrs.key disabledKeys);
|
||||
in map (attrs: attrs.module) (builtins.genericClosure {
|
||||
in map (attrs: attrs.module) (genericClosure {
|
||||
startSet = keyFilter modules;
|
||||
operator = attrs: keyFilter attrs.modules;
|
||||
});
|
||||
@@ -475,7 +487,7 @@ let
|
||||
}
|
||||
else
|
||||
# shorthand syntax
|
||||
lib.throwIfNot (isAttrs m) "module ${file} (${key}) does not look like a module."
|
||||
throwIfNot (isAttrs m) "module ${file} (${key}) does not look like a module."
|
||||
{ _file = toString m._file or file;
|
||||
_class = m._class or null;
|
||||
key = toString m.key or key;
|
||||
@@ -485,10 +497,10 @@ let
|
||||
config = addFreeformType (removeAttrs m ["_class" "_file" "key" "disabledModules" "require" "imports" "freeformType"]);
|
||||
};
|
||||
|
||||
applyModuleArgsIfFunction = key: f: args@{ config, options, lib, ... }:
|
||||
applyModuleArgsIfFunction = key: f: args@{ config, ... }:
|
||||
if isFunction f then applyModuleArgs key f args else f;
|
||||
|
||||
applyModuleArgs = key: f: args@{ config, options, lib, ... }:
|
||||
applyModuleArgs = key: f: args@{ config, ... }:
|
||||
let
|
||||
# Module arguments are resolved in a strict manner when attribute set
|
||||
# deconstruction is used. As the arguments are now defined with the
|
||||
@@ -503,10 +515,10 @@ let
|
||||
# not their values. The values are forwarding the result of the
|
||||
# evaluation of the option.
|
||||
context = name: ''while evaluating the module argument `${name}' in "${key}":'';
|
||||
extraArgs = builtins.mapAttrs (name: _:
|
||||
builtins.addErrorContext (context name)
|
||||
extraArgs = mapAttrs (name: _:
|
||||
addErrorContext (context name)
|
||||
(args.${name} or config._module.args.${name})
|
||||
) (lib.functionArgs f);
|
||||
) (functionArgs f);
|
||||
|
||||
# Note: we append in the opposite order such that we can add an error
|
||||
# context on the explicit arguments of "args" too. This update
|
||||
@@ -547,16 +559,16 @@ let
|
||||
(n: concatLists)
|
||||
(map
|
||||
(module: let subtree = module.options; in
|
||||
if !(builtins.isAttrs subtree) then
|
||||
if !(isAttrs subtree) then
|
||||
throw ''
|
||||
An option declaration for `${builtins.concatStringsSep "." prefix}' has type
|
||||
`${builtins.typeOf subtree}' rather than an attribute set.
|
||||
An option declaration for `${concatStringsSep "." prefix}' has type
|
||||
`${typeOf subtree}' rather than an attribute set.
|
||||
Did you mean to define this outside of `options'?
|
||||
''
|
||||
else
|
||||
mapAttrs
|
||||
(n: option:
|
||||
[{ inherit (module) _file; pos = builtins.unsafeGetAttrPos n subtree; options = option; }]
|
||||
[{ inherit (module) _file; pos = unsafeGetAttrPos n subtree; options = option; }]
|
||||
)
|
||||
subtree
|
||||
)
|
||||
@@ -565,17 +577,17 @@ let
|
||||
# The root of any module definition must be an attrset.
|
||||
checkedConfigs =
|
||||
assert
|
||||
lib.all
|
||||
all
|
||||
(c:
|
||||
# TODO: I have my doubts that this error would occur when option definitions are not matched.
|
||||
# The implementation of this check used to be tied to a superficially similar check for
|
||||
# options, so maybe that's why this is here.
|
||||
isAttrs c.config || throw ''
|
||||
In module `${c.file}', you're trying to define a value of type `${builtins.typeOf c.config}'
|
||||
In module `${c.file}', you're trying to define a value of type `${typeOf c.config}'
|
||||
rather than an attribute set for the option
|
||||
`${builtins.concatStringsSep "." prefix}'!
|
||||
`${concatStringsSep "." prefix}'!
|
||||
|
||||
This usually happens if `${builtins.concatStringsSep "." prefix}' has option
|
||||
This usually happens if `${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'!
|
||||
''
|
||||
@@ -667,7 +679,7 @@ let
|
||||
let
|
||||
nonOptions = filter (m: !isOption m.options) decls;
|
||||
in
|
||||
throw "The option `${showOption loc}' in module `${(lib.head optionDecls)._file}' would be a parent of the following options, but its type `${(lib.head optionDecls).options.type.description or "<no description>"}' does not support nested options.\n${
|
||||
throw "The option `${showOption loc}' in module `${(head optionDecls)._file}' would be a parent of the following options, but its type `${(head optionDecls).options.type.description or "<no description>"}' does not support nested options.\n${
|
||||
showRawDecls loc nonOptions
|
||||
}"
|
||||
else
|
||||
@@ -806,7 +818,7 @@ let
|
||||
"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;
|
||||
{ value = addErrorContext "while evaluating the option `${showOption loc}':" value;
|
||||
inherit (res.defsFinal') highestPrio;
|
||||
definitions = map (def: def.value) res.defsFinal;
|
||||
files = map (def: def.file) res.defsFinal;
|
||||
@@ -822,7 +834,7 @@ let
|
||||
let
|
||||
# Process mkMerge and mkIf properties.
|
||||
defs' = concatMap (m:
|
||||
map (value: { inherit (m) file; inherit value; }) (builtins.addErrorContext "while evaluating definitions from `${m.file}':" (dischargeProperties m.value))
|
||||
map (value: { inherit (m) file; inherit value; }) (addErrorContext "while evaluating definitions from `${m.file}':" (dischargeProperties m.value))
|
||||
) defs;
|
||||
|
||||
# Process mkOverride properties.
|
||||
@@ -972,12 +984,12 @@ let
|
||||
mergeAttrDefinitionsWithPrio = opt:
|
||||
let
|
||||
defsByAttr =
|
||||
lib.zipAttrs (
|
||||
lib.concatLists (
|
||||
lib.concatMap
|
||||
zipAttrs (
|
||||
concatLists (
|
||||
concatMap
|
||||
({ value, ... }@def:
|
||||
map
|
||||
(lib.mapAttrsToList (k: value: { ${k} = def // { inherit value; }; }))
|
||||
(mapAttrsToList (k: value: { ${k} = def // { inherit value; }; }))
|
||||
(pushDownProperties value)
|
||||
)
|
||||
opt.definitionsWithLocations
|
||||
@@ -985,9 +997,9 @@ let
|
||||
);
|
||||
in
|
||||
assert opt.type.name == "attrsOf" || opt.type.name == "lazyAttrsOf";
|
||||
lib.mapAttrs
|
||||
mapAttrs
|
||||
(k: v:
|
||||
let merging = lib.mergeDefinitions (opt.loc ++ [k]) opt.type.nestedTypes.elemType v;
|
||||
let merging = mergeDefinitions (opt.loc ++ [k]) opt.type.nestedTypes.elemType v;
|
||||
in {
|
||||
value = merging.mergedValue;
|
||||
inherit (merging.defsFinal') highestPrio;
|
||||
@@ -1023,9 +1035,9 @@ let
|
||||
mkForce = mkOverride 50;
|
||||
mkVMOverride = mkOverride 10; # used by ‘nixos-rebuild build-vm’
|
||||
|
||||
defaultPriority = lib.warnIf (lib.isInOldestRelease 2305) "lib.modules.defaultPriority is deprecated, please use lib.modules.defaultOverridePriority instead." defaultOverridePriority;
|
||||
defaultPriority = warnIf (isInOldestRelease 2305) "lib.modules.defaultPriority is deprecated, please use lib.modules.defaultOverridePriority instead." defaultOverridePriority;
|
||||
|
||||
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;
|
||||
mkFixStrictness = 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";
|
||||
@@ -1121,7 +1133,7 @@ let
|
||||
inherit from to;
|
||||
visible = false;
|
||||
warn = true;
|
||||
use = builtins.trace "Obsolete option `${showOption from}' is used. It was renamed to `${showOption to}'.";
|
||||
use = trace "Obsolete option `${showOption from}' is used. It was renamed to `${showOption to}'.";
|
||||
};
|
||||
|
||||
mkRenamedOptionModuleWith = {
|
||||
@@ -1139,8 +1151,8 @@ let
|
||||
}: doRename {
|
||||
inherit from to;
|
||||
visible = false;
|
||||
warn = lib.isInOldestRelease sinceRelease;
|
||||
use = lib.warnIf (lib.isInOldestRelease sinceRelease)
|
||||
warn = isInOldestRelease sinceRelease;
|
||||
use = warnIf (isInOldestRelease sinceRelease)
|
||||
"Obsolete option `${showOption from}' is used. It was renamed to `${showOption to}'.";
|
||||
};
|
||||
|
||||
@@ -1372,8 +1384,8 @@ let
|
||||
config = lib.importTOML file;
|
||||
};
|
||||
|
||||
private = lib.mapAttrs
|
||||
(k: lib.warn "External use of `lib.modules.${k}` is deprecated. If your use case isn't covered by non-deprecated functions, we'd like to know more and perhaps support your use case well, instead of providing access to these low level functions. In this case please open an issue in https://github.com/nixos/nixpkgs/issues/.")
|
||||
private = mapAttrs
|
||||
(k: warn "External use of `lib.modules.${k}` is deprecated. If your use case isn't covered by non-deprecated functions, we'd like to know more and perhaps support your use case well, instead of providing access to these low level functions. In this case please open an issue in https://github.com/nixos/nixpkgs/issues/.")
|
||||
{
|
||||
inherit
|
||||
applyModuleArgsIfFunction
|
||||
|
||||
@@ -94,6 +94,8 @@ checkConfigOutput '^true$' config.result ./module-argument-default.nix
|
||||
# gvariant
|
||||
checkConfigOutput '^true$' config.assertion ./gvariant.nix
|
||||
|
||||
checkConfigOutput '"ok"' config.result ./specialArgs-lib.nix
|
||||
|
||||
# https://github.com/NixOS/nixpkgs/pull/131205
|
||||
# We currently throw this error already in `config`, but throwing in `config.wrong1` would be acceptable.
|
||||
checkConfigError 'It seems as if you.re trying to declare an option by placing it into .config. rather than .options.' config.wrong1 ./error-mkOption-in-config.nix
|
||||
|
||||
@@ -0,0 +1,28 @@
|
||||
{ config, lib, ... }:
|
||||
|
||||
{
|
||||
options = {
|
||||
result = lib.mkOption { };
|
||||
weird = lib.mkOption {
|
||||
type = lib.types.submoduleWith {
|
||||
# I generally recommend against overriding lib, because that leads to
|
||||
# slightly incompatible dialects of the module system.
|
||||
# Nonetheless, it's worth guarding the property that the module system
|
||||
# evaluates with a completely custom lib, as a matter of separation of
|
||||
# concerns.
|
||||
specialArgs.lib = { };
|
||||
modules = [ ];
|
||||
};
|
||||
};
|
||||
};
|
||||
config.weird = args@{ ... /* note the lack of a `lib` argument */ }:
|
||||
assert args.lib == { };
|
||||
assert args.specialArgs == { lib = { }; };
|
||||
{
|
||||
options.foo = lib.mkOption { };
|
||||
config.foo = lib.mkIf true "alright";
|
||||
};
|
||||
config.result =
|
||||
assert config.weird.foo == "alright";
|
||||
"ok";
|
||||
}
|
||||
Reference in New Issue
Block a user