From 68955fe612683f30ef6f8bced39e9db6fb6b9577 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Wed, 30 Sep 2020 01:02:46 +0200 Subject: [PATCH 1/5] lib/types: Introduce mkOptionType occurringTypes argument This will be used to issue deprecation warnings recursively in the next commit In addition, this allows easily getting nested types of other options, which is useful when you want to create an option that aliases a part of another one. --- lib/types.nix | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/lib/types.nix b/lib/types.nix index 31ce440bcb48..d709791d6d0e 100644 --- a/lib/types.nix +++ b/lib/types.nix @@ -147,9 +147,13 @@ rec { , # The deprecation message to display when this type is used by an option # If null, the type isn't deprecated deprecationMessage ? null + , # The types that occur in the definition of this type. This is used to + # issue deprecation warnings recursively. Can also be used to reuse + # nested types + nestedTypes ? {} }: { _type = "option-type"; - inherit name check merge emptyValue getSubOptions getSubModules substSubModules typeMerge functor deprecationMessage; + inherit name check merge emptyValue getSubOptions getSubModules substSubModules typeMerge functor deprecationMessage nestedTypes; description = if description == null then name else description; }; @@ -365,6 +369,7 @@ rec { getSubModules = elemType.getSubModules; substSubModules = m: listOf (elemType.substSubModules m); functor = (defaultFunctor name) // { wrapped = elemType; }; + nestedTypes.elemType = elemType; }; nonEmptyListOf = elemType: @@ -389,6 +394,7 @@ rec { getSubModules = elemType.getSubModules; substSubModules = m: attrsOf (elemType.substSubModules m); functor = (defaultFunctor name) // { wrapped = elemType; }; + nestedTypes.elemType = elemType; }; # A version of attrsOf that's lazy in its values at the expense of @@ -413,6 +419,7 @@ rec { getSubModules = elemType.getSubModules; substSubModules = m: lazyAttrsOf (elemType.substSubModules m); functor = (defaultFunctor name) // { wrapped = elemType; }; + nestedTypes.elemType = elemType; }; # TODO: drop this in the future: @@ -421,6 +428,7 @@ rec { deprecationMessage = "Mixing lists with attribute values is no longer" + " possible; please use `types.attrsOf` instead. See" + " https://github.com/NixOS/nixpkgs/issues/1800 for the motivation."; + nestedTypes.elemType = elemType; }; # Value of given type but with no merging (i.e. `uniq list`s are not concatenated). @@ -433,6 +441,7 @@ rec { getSubModules = elemType.getSubModules; substSubModules = m: uniq (elemType.substSubModules m); functor = (defaultFunctor name) // { wrapped = elemType; }; + nestedTypes.elemType = elemType; }; # Null or value of ... @@ -451,6 +460,7 @@ rec { getSubModules = elemType.getSubModules; substSubModules = m: nullOr (elemType.substSubModules m); functor = (defaultFunctor name) // { wrapped = elemType; }; + nestedTypes.elemType = elemType; }; functionTo = elemType: mkOptionType { @@ -535,6 +545,9 @@ rec { substSubModules = m: submoduleWith (attrs // { modules = m; }); + nestedTypes = lib.optionalAttrs (freeformType != null) { + freeformType = freeformType; + }; functor = defaultFunctor name // { type = types.submoduleWith; payload = { @@ -596,6 +609,8 @@ rec { then functor.type mt1 mt2 else null; functor = (defaultFunctor name) // { wrapped = [ t1 t2 ]; }; + nestedTypes.left = t1; + nestedTypes.right = t2; }; # Any of the types in the given list @@ -627,6 +642,8 @@ rec { substSubModules = m: coercedTo coercedType coerceFunc (finalType.substSubModules m); typeMerge = t1: t2: null; functor = (defaultFunctor name) // { wrapped = finalType; }; + nestedTypes.coercedType = coercedType; + nestedTypes.finalType = finalType; }; # Obsolete alternative to configOf. It takes its option From ce5e3113c353d29edd0601af5c5ba38371f275d6 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Wed, 30 Sep 2020 01:49:58 +0200 Subject: [PATCH 2/5] lib/tests: Make sure the submodule type description can be evaluated In 2d45a62899d47c109a0b8ce4ca9d33265b8a1a37, the submodule type description was amended with the freeformType description. This causes all the modules passed to the submodule to be evaluated once on their own, without any extra definitions from the config section. This means that the specified modules need to be valid on their own, without any undeclared options. This commit adds a test that evaluates a submodules option description, which would trigger the above problem for one of the tests, if it were not fixed by this commit as well. This is done because the next commit makes option evaluation a bit more strict, which would also trigger this test failure, even though it's not related to the change at all. --- lib/tests/modules.sh | 3 +++ lib/tests/modules/declare-submoduleWith-modules.nix | 4 +--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/tests/modules.sh b/lib/tests/modules.sh index 2eddeec07b1a..2e57c2f8e2a1 100755 --- a/lib/tests/modules.sh +++ b/lib/tests/modules.sh @@ -175,6 +175,9 @@ checkConfigOutput "true" config.submodule.config ./declare-submoduleWith-noshort ## submoduleWith should merge all modules in one swoop checkConfigOutput "true" config.submodule.inner ./declare-submoduleWith-modules.nix checkConfigOutput "true" config.submodule.outer ./declare-submoduleWith-modules.nix +# Should also be able to evaluate the type name (which evaluates freeformType, +# which evaluates all the modules defined by the type) +checkConfigOutput "submodule" options.submodule.type.description ./declare-submoduleWith-modules.nix ## Paths should be allowed as values and work as expected checkConfigOutput "true" config.submodule.enable ./declare-submoduleWith-path.nix diff --git a/lib/tests/modules/declare-submoduleWith-modules.nix b/lib/tests/modules/declare-submoduleWith-modules.nix index 4736ab41751b..a8b82d176881 100644 --- a/lib/tests/modules/declare-submoduleWith-modules.nix +++ b/lib/tests/modules/declare-submoduleWith-modules.nix @@ -8,9 +8,6 @@ default = false; }; } - { - outer = true; - } ]; }; default = {}; @@ -25,6 +22,7 @@ }) { inner = true; + outer = true; } ]; } From 4b54aedee5e05aaf2838f6d951508b83e33d2baa Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Wed, 30 Sep 2020 01:12:30 +0200 Subject: [PATCH 3/5] 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(-) diff --git a/lib/modules.nix b/lib/modules.nix index d515ee24d16e..04b65d791b58 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; From 8b957e3b301d748e6fbbed806d82bd9d4b9d4ac4 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Wed, 30 Sep 2020 01:47:10 +0200 Subject: [PATCH 4/5] lib/tests: Add type deprecation tests --- lib/tests/modules.sh | 6 ++++ lib/tests/modules/type-deprecation.nix | 39 ++++++++++++++++++++++++++ 2 files changed, 45 insertions(+) create mode 100644 lib/tests/modules/type-deprecation.nix diff --git a/lib/tests/modules.sh b/lib/tests/modules.sh index 2e57c2f8e2a1..4259c538bb43 100755 --- a/lib/tests/modules.sh +++ b/lib/tests/modules.sh @@ -272,6 +272,12 @@ checkConfigError 'A definition for option .fun.\[function body\]. is not of type checkConfigOutput "b a" config.result ./functionTo/list-order.nix checkConfigOutput "a c" config.result ./functionTo/merging-attrs.nix +## Type deprecation +checkConfigError 'The type `types.simple'\'' of option `simple'\'' defined in .* is deprecated. simple shall not be used' config.simple ./type-deprecation.nix +checkConfigError 'The type `types.infinite'\'' of option `infinite'\'' defined in .* is deprecated. infinite shall not be used' config.infinite ./type-deprecation.nix +checkConfigError 'The type `types.left'\'' of option `nested'\'' defined in .* is deprecated. left shall not be used' config.nested ./type-deprecation.nix +checkConfigError 'The type `types.right'\'' of option `nested'\'' defined in .* is deprecated. right shall not be used' config.nested ./type-deprecation.nix + cat < Date: Wed, 5 May 2021 03:31:41 +0200 Subject: [PATCH 5/5] nixos/treewide: Remove usages of deprecated types.string --- nixos/modules/services/misc/disnix.nix | 2 +- nixos/modules/services/monitoring/prometheus/default.nix | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/nixos/modules/services/misc/disnix.nix b/nixos/modules/services/misc/disnix.nix index 41483d80a2dd..aea49511327d 100644 --- a/nixos/modules/services/misc/disnix.nix +++ b/nixos/modules/services/misc/disnix.nix @@ -37,7 +37,7 @@ in enableProfilePath = mkEnableOption "exposing the Disnix profiles in the system's PATH"; profiles = mkOption { - type = types.listOf types.string; + type = types.listOf types.str; default = [ "default" ]; example = [ "default" ]; description = "Names of the Disnix profiles to expose in the system's PATH"; diff --git a/nixos/modules/services/monitoring/prometheus/default.nix b/nixos/modules/services/monitoring/prometheus/default.nix index bd74e1a9cdb5..1d483627e9e2 100644 --- a/nixos/modules/services/monitoring/prometheus/default.nix +++ b/nixos/modules/services/monitoring/prometheus/default.nix @@ -112,7 +112,7 @@ let http://tools.ietf.org/html/rfc4366#section-3.1 ''; }; - name = mkOpt types.string '' + name = mkOpt types.str '' Name of the remote read config, which if specified must be unique among remote read configs. The name will be used in metrics and logging in place of a generated value to help users distinguish between remote read configs. @@ -174,7 +174,7 @@ let write_relabel_configs = mkOpt (types.listOf promTypes.relabel_config) '' List of remote write relabel configurations. ''; - name = mkOpt types.string '' + name = mkOpt types.str '' Name of the remote write config, which if specified must be unique among remote write configs. The name will be used in metrics and logging in place of a generated value to help users distinguish between remote write configs.