Merge pull request #286399 from benaryorg/global_ini_alt2

pkgs.formats: pkgs.formats version of lib.generators.toINIWithGlobalSection

3 files changed, 317 insertions, 83 deletions

diff --git a/nixos/doc/manual/development/settings-options.section.md b/nixos/doc/manual/development/settings-options.section.md
index 3a4800742b04..71ec9bbc8892 100644
--- a/nixos/doc/manual/development/settings-options.section.md
+++ b/nixos/doc/manual/development/settings-options.section.md
@@ -73,6 +73,34 @@ have a predefined type and string generator already declared under
 
     It returns a set with INI-specific attributes `type` and `generate`
     as specified [below](#pkgs-formats-result).
+    The type of the input is an *attrset* of sections; key-value pairs where
+    the key is the section name and the value is the corresponding content
+    which is also an *attrset* of key-value pairs for the actual key-value
+    mappings of the INI format.
+    The values of the INI atoms are subject to the above parameters (e.g. lists
+    may be transformed into multiple key-value pairs depending on
+    `listToValue`).
+
+`pkgs.formats.iniWithGlobalSection` { *`listsAsDuplicateKeys`* ? false, *`listToValue`* ? null, \.\.\. }
+
+:   A function taking an attribute set with values
+
+    `listsAsDuplicateKeys`
+
+    :   A boolean for controlling whether list values can be used to
+        represent duplicate INI keys
+
+    `listToValue`
+
+    :   A function for turning a list of values into a single value.
+
+    It returns a set with INI-specific attributes `type` and `generate`
+    as specified [below](#pkgs-formats-result).
+    The type of the input is an *attrset* of the structure
+    `{ sections = {}; globalSection = {}; }` where *sections* are several
+    sections as with *pkgs.formats.ini* and *globalSection* being just a single
+    attrset of key-value pairs for a single section, the global section which
+    preceedes the section definitions.
 
 `pkgs.formats.toml` { }
 
diff --git a/pkgs/pkgs-lib/formats.nix b/pkgs/pkgs-lib/formats.nix
index c78bd82e01ef..1b72270b43ca 100644
--- a/pkgs/pkgs-lib/formats.nix
+++ b/pkgs/pkgs-lib/formats.nix
@@ -95,29 +95,13 @@ rec {
 
   };
 
-  ini = {
-    # Represents lists as duplicate keys
-    listsAsDuplicateKeys ? false,
-    # Alternative to listsAsDuplicateKeys, converts list to non-list
-    # listToValue :: [IniAtom] -> IniAtom
-    listToValue ? null,
-    ...
-    }@args:
-    assert !listsAsDuplicateKeys || listToValue == null;
-    {
-
-    type = with lib.types; let
-
-      singleIniAtom = nullOr (oneOf [
-        bool
-        int
-        float
-        str
-      ]) // {
+  # the ini formats share a lot of code
+  inherit (
+    let
+      singleIniAtom = with lib.types; nullOr (oneOf [ bool int float str ]) // {
         description = "INI atom (null, bool, int, float or string)";
       };
-
-      iniAtom =
+      iniAtom = with lib.types; { listsAsDuplicateKeys, listToValue }:
         if listsAsDuplicateKeys then
           coercedTo singleIniAtom lib.singleton (listOf singleIniAtom) // {
             description = singleIniAtom.description + " or a list of them for duplicate keys";
@@ -128,21 +112,79 @@ rec {
           }
         else
           singleIniAtom;
+      iniSection = with lib.types; { listsAsDuplicateKeys, listToValue }@args:
+        attrsOf (iniAtom args) // {
+          description = "section of an INI file (attrs of " + (iniAtom args).description + ")";
+        };
 
-    in attrsOf (attrsOf iniAtom);
+      maybeToList = listToValue: if listToValue != null then lib.mapAttrs (key: val: if lib.isList val then listToValue val else val) else lib.id;
+    in {
+      ini = {
+        # Represents lists as duplicate keys
+        listsAsDuplicateKeys ? false,
+        # Alternative to listsAsDuplicateKeys, converts list to non-list
+        # listToValue :: [IniAtom] -> IniAtom
+        listToValue ? null,
+        ...
+        }@args:
+        assert listsAsDuplicateKeys -> listToValue == null;
+        {
 
-    generate = name: value:
-      let
-        transformedValue =
-          if listToValue != null
-          then
-            lib.mapAttrs (section: lib.mapAttrs (key: val:
-              if lib.isList val then listToValue val else val
-            )) value
-          else value;
-      in pkgs.writeText name (lib.generators.toINI (removeAttrs args ["listToValue"]) transformedValue);
+        type = lib.types.attrsOf (iniSection { listsAsDuplicateKeys = listsAsDuplicateKeys; listToValue = listToValue; });
 
-  };
+        generate = name: value:
+          lib.pipe value
+          [
+            (lib.mapAttrs (_: maybeToList listToValue))
+            (lib.generators.toINI (removeAttrs args ["listToValue"]))
+            (pkgs.writeText name)
+          ];
+      };
+
+      iniWithGlobalSection = {
+        # Represents lists as duplicate keys
+        listsAsDuplicateKeys ? false,
+        # Alternative to listsAsDuplicateKeys, converts list to non-list
+        # listToValue :: [IniAtom] -> IniAtom
+        listToValue ? null,
+        ...
+        }@args:
+        assert listsAsDuplicateKeys -> listToValue == null;
+        {
+          type = lib.types.submodule {
+            options = {
+              sections = lib.mkOption rec {
+                type = lib.types.attrsOf (iniSection { listsAsDuplicateKeys = listsAsDuplicateKeys; listToValue = listToValue; });
+                default = {};
+                description = type.description;
+              };
+              globalSection = lib.mkOption rec {
+                type = iniSection { listsAsDuplicateKeys = listsAsDuplicateKeys; listToValue = listToValue; };
+                default = {};
+                description = "global " + type.description;
+              };
+            };
+          };
+          generate = name: { sections ? {}, globalSection ? {}, ... }:
+            pkgs.writeText name (lib.generators.toINIWithGlobalSection (removeAttrs args ["listToValue"])
+            {
+              globalSection = maybeToList listToValue globalSection;
+              sections = lib.mapAttrs (_: maybeToList listToValue) sections;
+            });
+        };
+
+      gitIni = { listsAsDuplicateKeys ? false, ... }@args: {
+        type = let
+          atom = iniAtom {
+            listsAsDuplicateKeys = listsAsDuplicateKeys;
+            listToValue = null;
+          };
+        in with lib.types; attrsOf (attrsOf (either atom (attrsOf atom)));
+
+        generate = name: value: pkgs.writeText name (lib.generators.toGitINI value);
+      };
+
+    }) ini iniWithGlobalSection gitIni;
 
   # As defined by systemd.syntax(7)
   #
@@ -166,7 +208,7 @@ rec {
     listToValue ? null,
     ...
     }@args:
-    assert !listsAsDuplicateKeys || listToValue == null;
+    assert listsAsDuplicateKeys -> listToValue == null;
     {
 
     type = with lib.types; let
@@ -207,17 +249,6 @@ rec {
 
   };
 
-  gitIni = { listsAsDuplicateKeys ? false, ... }@args: {
-
-    type = with lib.types; let
-
-      iniAtom = (ini args).type/*attrsOf*/.functor.wrapped/*attrsOf*/.functor.wrapped;
-
-    in attrsOf (attrsOf (either iniAtom (attrsOf iniAtom)));
-
-    generate = name: value: pkgs.writeText name (lib.generators.toGitINI value);
-  };
-
   toml = {}: json {} // {
     type = with lib.types; let
       valueType = oneOf [
diff --git a/pkgs/pkgs-lib/tests/formats.nix b/pkgs/pkgs-lib/tests/formats.nix
index b7e100dd73bc..3243f4058e68 100644
--- a/pkgs/pkgs-lib/tests/formats.nix
+++ b/pkgs/pkgs-lib/tests/formats.nix
@@ -1,24 +1,21 @@
 { pkgs }:
 let
   inherit (pkgs) lib formats;
-in
-with lib;
-let
 
-  evalFormat = format: args: def:
-    let
-      formatSet = format args;
-      config = formatSet.type.merge [] (imap1 (n: def: {
-        # We check the input values, so that
-        #  - we don't write nonsensical tests that will impede progress
-        #  - the test author has a slightly more realistic view of the
-        #    final format during development.
-        value = lib.throwIfNot (formatSet.type.check def) (builtins.trace def "definition does not pass the type's check function") def;
-        file = "def${toString n}";
-      }) [ def ]);
-    in formatSet.generate "test-format-file" config;
-
-  runBuildTest = name: { drv, expected }: pkgs.runCommand name {
+  # merging allows us to add metadata to the input
+  # this makes error messages more readable during development
+  mergeInput = name: format: input:
+    format.type.merge [] [
+      {
+        # explicitly throw here to trigger the code path that prints the error message for users
+        value = lib.throwIfNot (format.type.check input) (builtins.trace input "definition does not pass the type's check function") input;
+        # inject the name
+        file = "format-test-${name}";
+      }
+    ];
+
+  # run a diff between expected and real output
+  runDiff = name: drv: expected: pkgs.runCommand name {
     passAsFile = ["expected"];
     inherit expected drv;
   } ''
@@ -31,12 +28,66 @@ let
     fi
   '';
 
-  runBuildTests = tests: pkgs.linkFarm "nixpkgs-pkgs-lib-format-tests" (mapAttrsToList (name: value: { inherit name; path = runBuildTest name value; }) (filterAttrs (name: value: value != null) tests));
+  # use this to check for proper serialization
+  # in practice you do not have to supply the name parameter as this one will be added by runBuildTests
+  shouldPass = { format, input, expected }: name: {
+    name = "pass-${name}";
+    path = runDiff "test-format-${name}" (format.generate "test-format-${name}" (mergeInput name format input)) expected;
+  };
+
+  # use this function to assert that a type check must fail
+  # in practice you do not have to supply the name parameter as this one will be added by runBuildTests
+  # note that as per 352e7d330a26 and 352e7d330a26 the type checking of attrsets and lists are not strict
+  # this means that the code below needs to properly merge the module type definition and also evaluate the (lazy) return value
+  shouldFail = { format, input }: name:
+    let
+      # trigger a deep type check using the module system
+      typeCheck = lib.modules.mergeDefinitions
+        [ "tests" name ]
+        format.type
+        [
+          {
+            file = "format-test-${name}";
+            value = input;
+          }
+        ];
+      # actually use the return value to trigger the evaluation
+      eval = builtins.tryEval (typeCheck.mergedValue == input);
+      # the check failing is what we want, so don't do anything here
+      typeFails = pkgs.runCommand "test-format-${name}" {} "touch $out";
+      # bail with some verbose information in case the type check passes
+      typeSucceeds = pkgs.runCommand "test-format-${name}" {
+          passAsFile = [ "inputText" ];
+          testName = name;
+          # this will fail if the input contains functions as values
+          # however that should get caught by the type check already
+          inputText = builtins.toJSON input;
+        }
+        ''
+          echo "Type check $testName passed when it shouldn't."
+          echo "The following data was used as input:"
+          echo
+          cat "$inputTextPath"
+          exit 1
+        '';
+    in {
+      name = "fail-${name}";
+      path = if eval.success then typeSucceeds else typeFails;
+    };
+
+  # this function creates a linkFarm for all the tests below such that the results are easily visible in the filesystem after a build
+  # the parameters are an attrset of name: test pairs where the name is automatically passed to the test
+  # the test therefore is an invocation of ShouldPass or shouldFail with the attrset parameters but *not* the name (which this adds for convenience)
+  runBuildTests = (lib.flip lib.pipe) [
+    (lib.mapAttrsToList (name: value: value name))
+    (pkgs.linkFarm "nixpkgs-pkgs-lib-format-tests")
+  ];
 
 in runBuildTests {
 
-  testJsonAtoms = {
-    drv = evalFormat formats.json {} {
+  jsonAtoms = shouldPass {
+    format = formats.json {};
+    input = {
       null = null;
       false = false;
       true = true;
@@ -67,8 +118,9 @@ in runBuildTests {
     '';
   };
 
-  testYamlAtoms = {
-    drv = evalFormat formats.yaml {} {
+  yamlAtoms = shouldPass {
+    format = formats.yaml {};
+    input = {
       null = null;
       false = false;
       true = true;
@@ -93,8 +145,9 @@ in runBuildTests {
     '';
   };
 
-  testIniAtoms = {
-    drv = evalFormat formats.ini {} {
+  iniAtoms = shouldPass {
+    format = formats.ini {};
+    input = {
       foo = {
         bool = true;
         int = 10;
@@ -111,8 +164,29 @@ in runBuildTests {
     '';
   };
 
-  testIniDuplicateKeys = {
-    drv = evalFormat formats.ini { listsAsDuplicateKeys = true; } {
+  iniInvalidAtom = shouldFail {
+    format = formats.ini {};
+    input = {
+      foo = {
+        function = _: 1;
+      };
+    };
+  };
+
+  iniDuplicateKeysWithoutList = shouldFail {
+    format = formats.ini {};
+    input = {
+      foo = {
+        bar = [ null true "test" 1.2 10 ];
+        baz = false;
+        qux = "qux";
+      };
+    };
+  };
+
+  iniDuplicateKeys = shouldPass {
+    format = formats.ini { listsAsDuplicateKeys = true; };
+    input = {
       foo = {
         bar = [ null true "test" 1.2 10 ];
         baz = false;
@@ -131,8 +205,9 @@ in runBuildTests {
     '';
   };
 
-  testIniListToValue = {
-    drv = evalFormat formats.ini { listToValue = concatMapStringsSep ", " (generators.mkValueStringDefault {}); } {
+  iniListToValue = shouldPass {
+    format = formats.ini { listToValue = lib.concatMapStringsSep ", " (lib.generators.mkValueStringDefault {}); };
+    input = {
       foo = {
         bar = [ null true "test" 1.2 10 ];
         baz = false;
@@ -147,8 +222,104 @@ in runBuildTests {
     '';
   };
 
-  testKeyValueAtoms = {
-    drv = evalFormat formats.keyValue {} {
+  iniWithGlobalNoSections = shouldPass {
+    format = formats.iniWithGlobalSection {};
+    input = {};
+    expected = "";
+  };
+
+  iniWithGlobalOnlySections = shouldPass {
+    format = formats.iniWithGlobalSection {};
+    input = {
+      sections = {
+        foo = {
+          bar = "baz";
+        };
+      };
+    };
+    expected = ''
+      [foo]
+      bar=baz
+    '';
+  };
+
+  iniWithGlobalOnlyGlobal = shouldPass {
+    format = formats.iniWithGlobalSection {};
+    input = {
+      globalSection = {
+        bar = "baz";
+      };
+    };
+    expected = ''
+      bar=baz
+
+    '';
+  };
+
+  iniWithGlobalWrongSections = shouldFail {
+    format = formats.iniWithGlobalSection {};
+    input = {
+      foo = {};
+    };
+  };
+
+  iniWithGlobalEverything = shouldPass {
+    format = formats.iniWithGlobalSection {};
+    input = {
+      globalSection = {
+        bar = true;
+      };
+      sections = {
+        foo = {
+          bool = true;
+          int = 10;
+          float = 3.141;
+          str = "string";
+        };
+      };
+    };
+    expected = ''
+      bar=true
+
+      [foo]
+      bool=true
+      float=3.141000
+      int=10
+      str=string
+    '';
+  };
+
+  iniWithGlobalListToValue = shouldPass {
+    format = formats.iniWithGlobalSection { listToValue = lib.concatMapStringsSep ", " (lib.generators.mkValueStringDefault {}); };
+    input = {
+      globalSection = {
+        bar = [ null true "test" 1.2 10 ];
+        baz = false;
+        qux = "qux";
+      };
+      sections = {
+        foo = {
+          bar = [ null true "test" 1.2 10 ];
+          baz = false;
+          qux = "qux";
+        };
+      };
+    };
+    expected = ''
+      bar=null, true, test, 1.200000, 10
+      baz=false
+      qux=qux
+
+      [foo]
+      bar=null, true, test, 1.200000, 10
+      baz=false
+      qux=qux
+    '';
+  };
+
+  keyValueAtoms = shouldPass {
+    format = formats.keyValue {};
+    input = {
       bool = true;
       int = 10;
       float = 3.141;
@@ -162,8 +333,9 @@ in runBuildTests {
     '';
   };
 
-  testKeyValueDuplicateKeys = {
-    drv = evalFormat formats.keyValue { listsAsDuplicateKeys = true; } {
+  keyValueDuplicateKeys = shouldPass {
+    format = formats.keyValue { listsAsDuplicateKeys = true; };
+    input = {
       bar = [ null true "test" 1.2 10 ];
       baz = false;
       qux = "qux";
@@ -179,8 +351,9 @@ in runBuildTests {
     '';
   };
 
-  testKeyValueListToValue = {
-    drv = evalFormat formats.keyValue { listToValue = concatMapStringsSep ", " (generators.mkValueStringDefault {}); } {
+  keyValueListToValue = shouldPass {
+    format = formats.keyValue { listToValue = lib.concatMapStringsSep ", " (lib.generators.mkValueStringDefault {}); };
+    input = {
       bar = [ null true "test" 1.2 10 ];
       baz = false;
       qux = "qux";
@@ -192,8 +365,9 @@ in runBuildTests {
     '';
   };
 
-  testTomlAtoms = {
-    drv = evalFormat formats.toml {} {
+  tomlAtoms = shouldPass {
+    format = formats.toml {};
+    input = {
       false = false;
       true = true;
       int = 10;
@@ -222,8 +396,9 @@ in runBuildTests {
   #   1. testing type coercions
   #   2. providing a more readable example test
   # Whereas java-properties/default.nix tests the low level escaping, etc.
-  testJavaProperties = {
-    drv = evalFormat formats.javaProperties {} {
+  javaProperties = shouldPass {
+    format = formats.javaProperties {};
+    input = {
       floaty = 3.1415;
       tautologies = true;
       contradictions = false;