From c4a9529071d5464d0c778690d8219f3548151b0b Mon Sep 17 00:00:00 2001 From: Johannes Kirschbauer Date: Sat, 9 Nov 2024 11:22:25 +0100 Subject: [PATCH 1/6] lib/types: init {types.attrsWith} --- lib/tests/modules.sh | 3 + lib/tests/modules/lazy-attrsWith.nix | 45 ++++++++++++++ lib/types.nix | 89 ++++++++++++++++++---------- 3 files changed, 106 insertions(+), 31 deletions(-) create mode 100644 lib/tests/modules/lazy-attrsWith.nix diff --git a/lib/tests/modules.sh b/lib/tests/modules.sh index 2884c00d0768c..2443c080180bd 100755 --- a/lib/tests/modules.sh +++ b/lib/tests/modules.sh @@ -575,6 +575,9 @@ checkConfigOutput '^38|27$' options.submoduleLine38.declarationPositions.1.line # nested options work checkConfigOutput '^34$' options.nested.nestedLine34.declarationPositions.0.line ./declaration-positions.nix +# AttrsWith tests +checkConfigOutput '^11$' config.result ./lazy-attrsWith.nix + cat <"]); - getSubModules = elemType.getSubModules; - substSubModules = m: attrsOf (elemType.substSubModules m); - functor = (defaultFunctor name) // { wrapped = elemType; }; - nestedTypes.elemType = elemType; - }; + attrsOf = elemType: attrsWith { inherit elemType; }; # A version of attrsOf that's lazy in its values at the expense of # conditional definitions not working properly. E.g. defining a value with # `foo.attr = mkIf false 10`, then `foo ? attr == true`, whereas with # attrsOf it would correctly be `false`. Accessing `foo.attr` would throw an # error that it's not defined. Use only if conditional definitions don't make sense. - lazyAttrsOf = elemType: mkOptionType rec { - name = "lazyAttrsOf"; - description = "lazy attribute set of ${optionDescriptionPhrase (class: class == "noun" || class == "composite") elemType}"; + lazyAttrsOf = elemType: attrsWith { inherit elemType; lazy = true; }; + + # base type for lazyAttrsOf and attrsOf + attrsWith = { + elemType, + lazy ? false, + }: + let + typeName = if lazy then "lazyAttrsOf" else "attrsOf"; + # Push down position info. + pushPositions = map (def: mapAttrs (n: v: { inherit (def) file; value = v; }) def.value); + in + mkOptionType { + name = typeName; + description = (if lazy then "lazy attribute set" else "attribute set") + " of ${optionDescriptionPhrase (class: class == "noun" || class == "composite") elemType}"; descriptionClass = "composite"; check = isAttrs; - merge = loc: defs: - zipAttrsWith (name: defs: - let merged = mergeDefinitions (loc ++ [name]) elemType defs; - # mergedValue will trigger an appropriate error when accessed - in merged.optionalValue.value or elemType.emptyValue.value or merged.mergedValue - ) - # Push down position info. - (map (def: mapAttrs (n: v: { inherit (def) file; value = v; }) def.value) defs); + merge = if lazy then ( + # Lazy merge Function + loc: defs: + zipAttrsWith (name: defs: + let merged = mergeDefinitions (loc ++ [name]) elemType defs; + # mergedValue will trigger an appropriate error when accessed + in merged.optionalValue.value or elemType.emptyValue.value or merged.mergedValue + ) + # Push down position info. + (pushPositions defs) + ) else ( + # Non-lazy merge Function + loc: defs: + mapAttrs (n: v: v.value) (filterAttrs (n: v: v ? value) (zipAttrsWith (name: defs: + (mergeDefinitions (loc ++ [name]) elemType (defs)).optionalValue + ) + # Push down position info. + (pushPositions defs))) + ); emptyValue = { value = {}; }; getSubOptions = prefix: elemType.getSubOptions (prefix ++ [""]); getSubModules = elemType.getSubModules; - substSubModules = m: lazyAttrsOf (elemType.substSubModules m); - functor = (defaultFunctor name) // { wrapped = elemType; }; + substSubModules = m: attrsWith { elemType = elemType.substSubModules m; inherit lazy; }; + functor = defaultFunctor "attrsWith" // { + wrapped = elemType; + payload = { + # Important!: Add new function attributes here in case of future changes + inherit elemType lazy; + }; + binOp = lhs: rhs: + let + elemType = lhs.elemType.typeMerge rhs.elemType.functor; + lazy = + if lhs.lazy == rhs.lazy then + lhs.lazy + else + null; + in + if elemType == null || lazy == null then + null + else + { + inherit elemType lazy; + }; + }; nestedTypes.elemType = elemType; }; From 415d1932eae07d9a64e1e21199a2b9c904ce3d5f Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Mon, 25 Nov 2024 16:06:23 +0100 Subject: [PATCH 2/6] lib/types: Test attrsWith type merging Co-Authored-By: @hsjobeki --- lib/tests/modules.sh | 6 ++-- lib/tests/modules/lazy-attrsWith.nix | 46 ++++++++++++++++++---------- lib/types.nix | 3 +- 3 files changed, 35 insertions(+), 20 deletions(-) diff --git a/lib/tests/modules.sh b/lib/tests/modules.sh index 2443c080180bd..4c00ecaab605b 100755 --- a/lib/tests/modules.sh +++ b/lib/tests/modules.sh @@ -386,6 +386,10 @@ checkConfigOutput '^true$' config.conditionalWorks ./declare-attrsOf.nix ./attrs checkConfigOutput '^false$' config.conditionalWorks ./declare-lazyAttrsOf.nix ./attrsOf-conditional-check.nix checkConfigOutput '^"empty"$' config.value.foo ./declare-lazyAttrsOf.nix ./attrsOf-conditional-check.nix +# Check attrsWith type merging +checkConfigError 'The option `mergedLazyNonLazy'\'' in `.*'\'' is already declared in `.*'\''\.' options.mergedLazyNonLazy ./lazy-attrsWith.nix +checkConfigOutput '^11$' config.lazyResult ./lazy-attrsWith.nix +checkConfigError 'infinite recursion encountered' config.nonLazyResult ./lazy-attrsWith.nix # Even with multiple assignments, a type error should be thrown if any of them aren't valid checkConfigError 'A definition for option .* is not of type .*' \ @@ -575,8 +579,6 @@ checkConfigOutput '^38|27$' options.submoduleLine38.declarationPositions.1.line # nested options work checkConfigOutput '^34$' options.nested.nestedLine34.declarationPositions.0.line ./declaration-positions.nix -# AttrsWith tests -checkConfigOutput '^11$' config.result ./lazy-attrsWith.nix cat < Date: Mon, 25 Nov 2024 16:58:58 +0100 Subject: [PATCH 3/6] lib/types: Add deprecation to attrsWith Co-Authored-By: @infinisil --- lib/modules.nix | 29 ++++++++++++++++++++++++++--- lib/types.nix | 38 ++++++++++++++++++++++++-------------- 2 files changed, 50 insertions(+), 17 deletions(-) diff --git a/lib/modules.nix b/lib/modules.nix index 855ffaf25ed81..2294585723ba2 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -751,9 +751,32 @@ let t' = opt.options.type; mergedType = t.typeMerge t'.functor; typesMergeable = mergedType != null; - typeSet = if (bothHave "type") && typesMergeable - then { type = mergedType; } - else {}; + + # TODO: Remove this when all downstream reliances of internals: 'functor.wrapped' are sufficiently migrated. + # A function that adds the deprecated wrapped message to a type. + addDeprecatedWrapped = t: + t // { + functor = t.functor // { + wrapped = t.functor.wrappedDeprecationMessage { + inherit loc; + }; + }; + }; + + mergedType' = + if mergedType ? functor.wrappedDeprecationMessage then + addDeprecatedWrapped mergedType + else + mergedType; + + typeSet = + if (bothHave "type") && typesMergeable then + { type = mergedType'; } + else if opt.options ? type && opt.options.type ? functor.wrappedDeprecationMessage then + { type = addDeprecatedWrapped opt.options.type; } + else + {}; + bothHave = k: opt.options ? ${k} && res ? ${k}; in if bothHave "default" || diff --git a/lib/types.nix b/lib/types.nix index a286f16227a0f..41c53ec89ba65 100644 --- a/lib/types.nix +++ b/lib/types.nix @@ -83,11 +83,15 @@ rec { # Default type merging function # takes two type functors and return the merged type defaultTypeMerge = f: f': - let mergedWrapped = f.wrapped.typeMerge f'.wrapped.functor; - mergedPayload = f.binOp f.payload f'.payload; + let + mergedWrapped = f.wrapped.typeMerge f'.wrapped.functor; + mergedPayload = f.binOp f.payload f'.payload; + + hasPayload = assert (f'.payload != null) == (f.payload != null); f.payload != null; + hasWrapped = assert (f'.wrapped != null) == (f.wrapped != null); f.wrapped != null; - hasPayload = assert (f'.payload != null) == (f.payload != null); f.payload != null; - hasWrapped = assert (f'.wrapped != null) == (f.wrapped != null); f.wrapped != null; + typeFromPayload = if mergedPayload == null then null else f.type mergedPayload; + typeFromWrapped = if mergedWrapped == null then null else f.type mergedWrapped; in # Abort early: cannot merge different types if f.name != f'.name @@ -95,23 +99,23 @@ rec { else if hasPayload then - if hasWrapped then + # Just return the payload if returning wrapped is deprecated + if f ? wrappedDeprecationMessage then + typeFromPayload + else if hasWrapped then # Has both wrapped and payload throw '' Type ${f.name} defines both `functor.payload` and `functor.wrapped` at the same time, which is not supported. Use either `functor.payload` or `functor.wrapped` but not both. - If your code worked before remove `functor.payload` from the type definition. + If your code worked before remove either `functor.wrapped` or `functor.payload` from the type definition. '' else - # Has payload - if mergedPayload == null then null else f.type mergedPayload + typeFromPayload else if hasWrapped then - # Has wrapped - # TODO(@hsjobeki): This could also be a warning and removed in the future - if mergedWrapped == null then null else f.type mergedWrapped + typeFromWrapped else f.type; @@ -592,7 +596,8 @@ rec { lazyAttrsOf = elemType: attrsWith { inherit elemType; lazy = true; }; # base type for lazyAttrsOf and attrsOf - attrsWith = { + attrsWith = + { elemType, lazy ? false, }: @@ -630,8 +635,13 @@ rec { getSubModules = elemType.getSubModules; substSubModules = m: attrsWith { elemType = elemType.substSubModules m; inherit lazy; }; functor = defaultFunctor "attrsWith" // { - # TODO: This breaks stuff - # wrapped = elemType; + wrappedDeprecationMessage = { loc }: lib.warn '' + Using 'functor.wrapped' on option types will be deprecated. + + Use 'nestedTypes.elemType' instead. + + option: '${showOption loc}' + '' elemType; payload = { # Important!: Add new function attributes here in case of future changes inherit elemType lazy; From 14f4431d123be35ed23e49c23b3e3332b2dd4996 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Mon, 2 Dec 2024 15:54:20 +0100 Subject: [PATCH 4/6] lib/modules: Minor performance optimisation Co-Authored-By: Johannes Kirschbauer --- lib/modules.nix | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/lib/modules.nix b/lib/modules.nix index 2294585723ba2..f5b8af0e9858b 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -763,17 +763,24 @@ let }; }; - mergedType' = - if mergedType ? functor.wrappedDeprecationMessage then - addDeprecatedWrapped mergedType - else - mergedType; - typeSet = - if (bothHave "type") && typesMergeable then - { type = mergedType'; } - else if opt.options ? type && opt.options.type ? functor.wrappedDeprecationMessage then - { type = addDeprecatedWrapped opt.options.type; } + if opt.options ? type then + if res ? type then + if typesMergeable then + { + type = + if mergedType ? functor.wrappedDeprecationMessage then + addDeprecatedWrapped mergedType + else + mergedType; + } + else + # Keep in sync with the same error below! + throw "The option `${showOption loc}' in `${opt._file}' is already declared in ${showFiles res.declarations}." + else if opt.options.type ? functor.wrappedDeprecationMessage then + { type = addDeprecatedWrapped opt.options.type; } + else + {} else {}; @@ -782,9 +789,9 @@ let if bothHave "default" || bothHave "example" || bothHave "description" || - bothHave "apply" || - (bothHave "type" && (! typesMergeable)) + bothHave "apply" then + # Keep in sync with the same error above! throw "The option `${showOption loc}' in `${opt._file}' is already declared in ${showFiles res.declarations}." else let From d5eccbbbae7b781552d0e18ff54a3231b4e295ec Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Mon, 2 Dec 2024 16:08:59 +0100 Subject: [PATCH 5/6] lib/types: standardise attrsOf functor.wrapped warning and add a test --- lib/tests/modules.sh | 4 ++++ lib/types.nix | 8 ++------ 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/tests/modules.sh b/lib/tests/modules.sh index 4c00ecaab605b..6085d4b7984ab 100755 --- a/lib/tests/modules.sh +++ b/lib/tests/modules.sh @@ -391,6 +391,10 @@ checkConfigError 'The option `mergedLazyNonLazy'\'' in `.*'\'' is already declar checkConfigOutput '^11$' config.lazyResult ./lazy-attrsWith.nix checkConfigError 'infinite recursion encountered' config.nonLazyResult ./lazy-attrsWith.nix +# Test the attrsOf functor.wrapped warning +# shellcheck disable=2016 +NIX_ABORT_ON_WARN=1 checkConfigError 'The deprecated `type.functor.wrapped` attribute of the option `mergedLazyLazy` is accessed, use `nestedTypes.elemType` instead.' options.mergedLazyLazy.type.functor.wrapped ./lazy-attrsWith.nix + # Even with multiple assignments, a type error should be thrown if any of them aren't valid checkConfigError 'A definition for option .* is not of type .*' \ config.value ./declare-int-unsigned-value.nix ./define-value-list.nix ./define-value-int-positive.nix diff --git a/lib/types.nix b/lib/types.nix index 41c53ec89ba65..caa7b6e7b5f2f 100644 --- a/lib/types.nix +++ b/lib/types.nix @@ -636,12 +636,8 @@ rec { substSubModules = m: attrsWith { elemType = elemType.substSubModules m; inherit lazy; }; functor = defaultFunctor "attrsWith" // { wrappedDeprecationMessage = { loc }: lib.warn '' - Using 'functor.wrapped' on option types will be deprecated. - - Use 'nestedTypes.elemType' instead. - - option: '${showOption loc}' - '' elemType; + The deprecated `type.functor.wrapped` attribute of the option `${showOption loc}` is accessed, use `type.nestedTypes.elemType` instead. + '' elemType; payload = { # Important!: Add new function attributes here in case of future changes inherit elemType lazy; From 399e582e18d54768edf9404dce6c4b03d3ac2001 Mon Sep 17 00:00:00 2001 From: Johannes Kirschbauer Date: Mon, 2 Dec 2024 16:39:29 +0100 Subject: [PATCH 6/6] lib.types: improve performance on attrsWith --- lib/types.nix | 42 +++++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/lib/types.nix b/lib/types.nix index caa7b6e7b5f2f..dd1e8ab9d0165 100644 --- a/lib/types.nix +++ b/lib/types.nix @@ -597,17 +597,31 @@ rec { # base type for lazyAttrsOf and attrsOf attrsWith = - { - elemType, - lazy ? false, - }: let - typeName = if lazy then "lazyAttrsOf" else "attrsOf"; # Push down position info. pushPositions = map (def: mapAttrs (n: v: { inherit (def) file; value = v; }) def.value); + binOp = lhs: rhs: + let + elemType = lhs.elemType.typeMerge rhs.elemType.functor; + lazy = + if lhs.lazy == rhs.lazy then + lhs.lazy + else + null; + in + if elemType == null || lazy == null then + null + else + { + inherit elemType lazy; + }; in + { + elemType, + lazy ? false, + }: mkOptionType { - name = typeName; + name = if lazy then "lazyAttrsOf" else "attrsOf"; description = (if lazy then "lazy attribute set" else "attribute set") + " of ${optionDescriptionPhrase (class: class == "noun" || class == "composite") elemType}"; descriptionClass = "composite"; check = isAttrs; @@ -642,21 +656,7 @@ rec { # Important!: Add new function attributes here in case of future changes inherit elemType lazy; }; - binOp = lhs: rhs: - let - elemType = lhs.elemType.typeMerge rhs.elemType.functor; - lazy = - if lhs.lazy == rhs.lazy then - lhs.lazy - else - null; - in - if elemType == null || lazy == null then - null - else - { - inherit elemType lazy; - }; + inherit binOp; }; nestedTypes.elemType = elemType; };