hack: remove some requirements for merging types

The type merging code is too restrictive - a bug - since as some types
that specify defaults can't be merged at all even if they should be able
to.

In practice, I'd like the merging system to allow merging if any of the
following apply:
- Unmergeable properties are unspecified on one side
- Unmergeable properties are default on one side (take the non-default -
  we should consider explicitly setting a thing to the default vaule as
  different to actually being the default - maybe mkPrio equivalent can
  be used here?)
- Unmergeable properties are the same on both sides (take the whole
  value - unsure about this one but it would maybe be a little nicer)

I would also like nicer error messages saying why a thing can't be
merged

This commit shouldn't be merged into main
This commit is contained in:
Skyler Grey 2025-06-03 13:11:02 +00:00
parent 7552ab48bb
commit 6e7139ad0a
3 changed files with 42 additions and 12 deletions

View file

@ -94,20 +94,38 @@ lib: {
in
builtins.sort compare normalized;
## Normalize the type of an option. This will set a default type if none
## was provided.
## Normalize an option. This will set a default type if none was
## provided, and remove any property override wrapping
##
## @type List String -> Option -> Option
fixup =
location: option:
let
normalize =
value:
if lib.types.is "override" value then
value.content
else
value;
getNormalized = name: if option ? ${name} then { ${name} = normalize option.${name}; } else {};
in
if option.type.submodules or null == null then
option // { type = option.type or lib.types.unspecified; }
option
// { type = option.type or lib.types.unspecified; }
// getNormalized "default"
// getNormalized "example"
// getNormalized "description"
// getNormalized "apply"
else
option
// {
type = option.type.withSubModules option.options;
options = [ ];
};
}
// getNormalized "default"
// getNormalized "example"
// getNormalized "description"
// getNormalized "apply";
## Invert the structure of `merge`, `when`, and `override` definitions so
## that they apply to each individual attribute in their respective sets.
@ -438,7 +456,7 @@ lib: {
}
else
builtins.throw "The option `${lib.options.getIdentifier location}` in module `${(builtins.head optionDeclarations).__file__}` would be a parent of the following options, but its type `${
(builtins.head optionDeclarations).options.type.description or "<no description>"
(lib.modules.apply.fixup location (builtins.head optionDeclarations)).options.type.description or "<no description>"
}` does not support nested options."
else
process location declarations definitions

View file

@ -116,15 +116,23 @@ lib: {
++ result.options
else
result.options;
getPriority =
value:
if lib.types.is "override" value then
value.priority
else
lib.modules.DEFAULT_PRIORITY;
higherPriority = value1: value2: (getPriority value1) < (getPriority value2);
mergeProperty = name:
if (higherPriority result.${name} option.options.${name}) then result.${name}
else if (higherPriority result.${name} option.options.${name}) then option.options.${name}
else if result.${name} == option.options.${name} then result.${name} # This is mostly so any defaults/etc. get merged cleanly
else builtins.throw "The option `${lib.options.getIdentifier location}` in `${option.__file__}` is already declared in ${serializedFiles}, but their ${name} values could not be merged. Please set one as a higher priority (e.g. using `${name} = lib.modules.orders.before <value>`) or remove a definition";
in
if
shared "default"
|| shared "example"
|| shared "description"
|| shared "apply"
|| (shared "type" && !isTypeMergeable)
shared "type" && !isTypeMergeable
then
builtins.throw "The option `${lib.options.getIdentifier location}` in `${option.__file__}` is already declared in ${serializedFiles}"
builtins.throw "The option `${lib.options.getIdentifier location}` in `${option.__file__}` is already declared in ${serializedFiles} but its type is not mergeable. Please remove a definition"
else
option.options
// result
@ -132,6 +140,10 @@ lib: {
declarations = result.declarations ++ [ option.__file__ ];
options = submodules;
}
// (if shared "default" then { default = mergeProperty "default"; } else {})
// (if shared "example" then { example = mergeProperty "example"; } else {})
// (if shared "description" then { description = mergeProperty "description"; } else {})
// (if shared "apply" then { apply = mergeProperty "apply"; } else {})
// typeSet;
in
builtins.foldl' merge {

View file

@ -1090,7 +1090,7 @@ lib: {
__file__ = definition.__file__;
options = lib.options.create { type = definition.value; };
}) definitions;
merged = lib.modules.fixup location (lib.options.merge.declarations location modules);
merged = lib.modules.apply.fixup location (lib.options.merge.declarations location modules);
in
if builtins.length definitions == 1 then first.value else merged.type;
};