From e14483d6a67399485876ab51c9726dbd2eb0836a Mon Sep 17 00:00:00 2001 From: MithicSpirit Date: Thu, 3 Oct 2024 21:28:24 -0400 Subject: [PATCH] formats.ini: disable merging as list by default Previously, setting listsAsDuplicateKeys or listToValue would make it so merging these treat all values as lists, by coercing non-lists via lib.singleton. Some programs (such as gamemode; see #345121), allow some values to be repeated but not others, which can lead to unexpected behavior when non-list values are merged like this rather than throwing an error. This now makes that behavior opt-in via the mergeAsList option. Setting mergeAsList (to either true or false) without setting either listsAsDuplicateKeys or listToValue is an error, since lists are meaningless in this case. --- .../manual/release-notes/rl-2411.section.md | 5 + pkgs/pkgs-lib/formats.nix | 38 +++-- pkgs/pkgs-lib/tests/formats.nix | 137 ++++++++++++++++++ 3 files changed, 171 insertions(+), 9 deletions(-) diff --git a/nixos/doc/manual/release-notes/rl-2411.section.md b/nixos/doc/manual/release-notes/rl-2411.section.md index e802470e7a04..01b09aee1578 100644 --- a/nixos/doc/manual/release-notes/rl-2411.section.md +++ b/nixos/doc/manual/release-notes/rl-2411.section.md @@ -566,6 +566,11 @@ - The `rustic` package was upgrade to `0.9.0`, which contains [breaking changes to the config file format](https://github.com/rustic-rs/rustic/releases/tag/v0.9.0). +- `pkgs.formats.ini` and `pkgs.formats.iniWithGlobalSection` with + `listsAsDuplicateKeys` or `listToValue` no longer merge non-list values into + lists by default. Backwards-compatible behavior can be enabled with + `atomsCoercedToLists`. + ## Other Notable Changes {#sec-release-24.11-notable-changes} diff --git a/pkgs/pkgs-lib/formats.nix b/pkgs/pkgs-lib/formats.nix index c99144d93519..03663ad96b97 100644 --- a/pkgs/pkgs-lib/formats.nix +++ b/pkgs/pkgs-lib/formats.nix @@ -109,18 +109,21 @@ rec { singleIniAtom = nullOr (oneOf [ bool int float str ]) // { description = "INI atom (null, bool, int, float or string)"; }; - iniAtom = { listsAsDuplicateKeys, listToValue }: + iniAtom = { listsAsDuplicateKeys, listToValue, atomsCoercedToLists }: + let + singleIniAtomOr = if atomsCoercedToLists then coercedTo singleIniAtom lib.singleton else either singleIniAtom; + in if listsAsDuplicateKeys then - coercedTo singleIniAtom lib.singleton (listOf singleIniAtom) // { + singleIniAtomOr (listOf singleIniAtom) // { description = singleIniAtom.description + " or a list of them for duplicate keys"; } else if listToValue != null then - coercedTo singleIniAtom lib.singleton (nonEmptyListOf singleIniAtom) // { + singleIniAtomOr (nonEmptyListOf singleIniAtom) // { description = singleIniAtom.description + " or a non-empty list of them"; } else singleIniAtom; - iniSection = { listsAsDuplicateKeys, listToValue }@args: + iniSection = { listsAsDuplicateKeys, listToValue, atomsCoercedToLists }@args: attrsOf (iniAtom args) // { description = "section of an INI file (attrs of " + (iniAtom args).description + ")"; }; @@ -133,18 +136,26 @@ rec { # Alternative to listsAsDuplicateKeys, converts list to non-list # listToValue :: [IniAtom] -> IniAtom listToValue ? null, + # Merge multiple instances of the same key into a list + atomsCoercedToLists ? null, ... }@args: assert listsAsDuplicateKeys -> listToValue == null; + assert atomsCoercedToLists != null -> (listsAsDuplicateKeys || listToValue != null); + let + atomsCoercedToLists' = if atomsCoercedToLists == null then false else atomsCoercedToLists; + in { - type = lib.types.attrsOf (iniSection { listsAsDuplicateKeys = listsAsDuplicateKeys; listToValue = listToValue; }); + type = lib.types.attrsOf ( + iniSection { inherit listsAsDuplicateKeys listToValue; atomsCoercedToLists = atomsCoercedToLists'; } + ); generate = name: value: lib.pipe value [ (lib.mapAttrs (_: maybeToList listToValue)) - (lib.generators.toINI (removeAttrs args ["listToValue"])) + (lib.generators.toINI (removeAttrs args ["listToValue" "atomsCoercedToLists"])) (pkgs.writeText name) ]; }; @@ -155,26 +166,34 @@ rec { # Alternative to listsAsDuplicateKeys, converts list to non-list # listToValue :: [IniAtom] -> IniAtom listToValue ? null, + # Merge multiple instances of the same key into a list + atomsCoercedToLists ? null, ... }@args: assert listsAsDuplicateKeys -> listToValue == null; + assert atomsCoercedToLists != null -> (listsAsDuplicateKeys || listToValue != null); + let + atomsCoercedToLists' = if atomsCoercedToLists == null then false else atomsCoercedToLists; + in { type = lib.types.submodule { options = { sections = lib.mkOption rec { - type = lib.types.attrsOf (iniSection { listsAsDuplicateKeys = listsAsDuplicateKeys; listToValue = listToValue; }); + type = lib.types.attrsOf ( + iniSection { inherit listsAsDuplicateKeys listToValue; atomsCoercedToLists = atomsCoercedToLists'; } + ); default = {}; description = type.description; }; globalSection = lib.mkOption rec { - type = iniSection { listsAsDuplicateKeys = listsAsDuplicateKeys; listToValue = listToValue; }; + type = iniSection { inherit listsAsDuplicateKeys listToValue; atomsCoercedToLists = atomsCoercedToLists'; }; default = {}; description = "global " + type.description; }; }; }; generate = name: { sections ? {}, globalSection ? {}, ... }: - pkgs.writeText name (lib.generators.toINIWithGlobalSection (removeAttrs args ["listToValue"]) + pkgs.writeText name (lib.generators.toINIWithGlobalSection (removeAttrs args ["listToValue" "atomsCoercedToLists"]) { globalSection = maybeToList listToValue globalSection; sections = lib.mapAttrs (_: maybeToList listToValue) sections; @@ -186,6 +205,7 @@ rec { atom = iniAtom { listsAsDuplicateKeys = listsAsDuplicateKeys; listToValue = null; + atomsCoercedToLists = false; }; in attrsOf (attrsOf (either atom (attrsOf atom))); diff --git a/pkgs/pkgs-lib/tests/formats.nix b/pkgs/pkgs-lib/tests/formats.nix index 86a35c6bf26d..5f6f3b93e280 100644 --- a/pkgs/pkgs-lib/tests/formats.nix +++ b/pkgs/pkgs-lib/tests/formats.nix @@ -222,6 +222,67 @@ in runBuildTests { ''; }; + iniCoercedDuplicateKeys = shouldPass rec { + format = formats.ini { + listsAsDuplicateKeys = true; + atomsCoercedToLists = true; + }; + input = format.type.merge [ ] [ + { + file = "format-test-inner-iniCoercedDuplicateKeys"; + value = { foo = { bar = 1; }; }; + } + { + file = "format-test-inner-iniCoercedDuplicateKeys"; + value = { foo = { bar = 2; }; }; + } + ]; + expected = '' + [foo] + bar=1 + bar=2 + ''; + }; + + iniCoercedListToValue = shouldPass rec { + format = formats.ini { + listToValue = lib.concatMapStringsSep ", " (lib.generators.mkValueStringDefault { }); + atomsCoercedToLists = true; + }; + input = format.type.merge [ ] [ + { + file = "format-test-inner-iniCoercedListToValue"; + value = { foo = { bar = 1; }; }; + } + { + file = "format-test-inner-iniCoercedListToValue"; + value = { foo = { bar = 2; }; }; + } + ]; + expected = '' + [foo] + bar=1, 2 + ''; + }; + + iniCoercedNoLists = shouldFail { + format = formats.ini { atomsCoercedToLists = true; }; + input = { + foo = { + bar = 1; + }; + }; + }; + + iniNoCoercedNoLists = shouldFail { + format = formats.ini { atomsCoercedToLists = false; }; + input = { + foo = { + bar = 1; + }; + }; + }; + iniWithGlobalNoSections = shouldPass { format = formats.iniWithGlobalSection {}; input = {}; @@ -317,6 +378,82 @@ in runBuildTests { ''; }; + iniWithGlobalCoercedDuplicateKeys = shouldPass rec { + format = formats.iniWithGlobalSection { + listsAsDuplicateKeys = true; + atomsCoercedToLists = true; + }; + input = format.type.merge [ ] [ + { + file = "format-test-inner-iniWithGlobalCoercedDuplicateKeys"; + value = { + globalSection = { baz = 4; }; + sections = { foo = { bar = 1; }; }; + }; + } + { + file = "format-test-inner-iniWithGlobalCoercedDuplicateKeys"; + value = { + globalSection = { baz = 3; }; + sections = { foo = { bar = 2; }; }; + }; + } + ]; + expected = '' + baz=3 + baz=4 + + [foo] + bar=2 + bar=1 + ''; + }; + + iniWithGlobalCoercedListToValue = shouldPass rec { + format = formats.iniWithGlobalSection { + listToValue = lib.concatMapStringsSep ", " (lib.generators.mkValueStringDefault { }); + atomsCoercedToLists = true; + }; + input = format.type.merge [ ] [ + { + file = "format-test-inner-iniWithGlobalCoercedListToValue"; + value = { + globalSection = { baz = 4; }; + sections = { foo = { bar = 1; }; }; + }; + } + { + file = "format-test-inner-iniWithGlobalCoercedListToValue"; + value = { + globalSection = { baz = 3; }; + sections = { foo = { bar = 2; }; }; + }; + } + ]; + expected = '' + baz=3, 4 + + [foo] + bar=2, 1 + ''; + }; + + iniWithGlobalCoercedNoLists = shouldFail { + format = formats.iniWithGlobalSection { atomsCoercedToLists = true; }; + input = { + globalSection = { baz = 4; }; + foo = { bar = 1; }; + }; + }; + + iniWithGlobalNoCoercedNoLists = shouldFail { + format = formats.iniWithGlobalSection { atomsCoercedToLists = false; }; + input = { + globalSection = { baz = 4; }; + foo = { bar = 1; }; + }; + }; + keyValueAtoms = shouldPass { format = formats.keyValue {}; input = {