From cec6d7f51a32d6085caa8636ba3003aa1f4c3be6 Mon Sep 17 00:00:00 2001
From: Yarny0 <41838844+Yarny0@users.noreply.github.com>
Date: Tue, 15 Aug 2023 11:12:07 +0200
Subject: [PATCH 1/9] nixos/tsm*: update product name and URLs

With the tsm-client 8.1.19.0 release,
IBM renamed the product brand from
"IBM Spectrum Protect" to "IBM Storage Protect":
https://www.ibm.com/support/pages/node/6964770 .

The package already got updated in commits
5ff5b2ae4cea8fa54fe14ad38e3bad1c7a226d63 and
a4b7a6253286337e212da47835fd3785ea861abb .

The commit at hand updates the modules accordingly.
---
 nixos/modules/programs/tsm-client.nix | 6 +++---
 nixos/modules/services/backup/tsm.nix | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/nixos/modules/programs/tsm-client.nix b/nixos/modules/programs/tsm-client.nix
index 6cb225d102de..67b9f6d7a362 100644
--- a/nixos/modules/programs/tsm-client.nix
+++ b/nixos/modules/programs/tsm-client.nix
@@ -144,7 +144,7 @@ let
     };
     config.name = mkDefault name;
     # Client system-options file directives are explained here:
-    # https://www.ibm.com/docs/en/spectrum-protect/8.1.13?topic=commands-processing-options
+    # https://www.ibm.com/docs/en/storage-protect/8.1.20?topic=commands-processing-options
     config.extraConfig =
       mapAttrs (lib.trivial.const mkDefault) (
         {
@@ -173,7 +173,7 @@ let
 
   options.programs.tsmClient = {
     enable = mkEnableOption (lib.mdDoc ''
-      IBM Spectrum Protect (Tivoli Storage Manager, TSM)
+      IBM Storage Protect (Tivoli Storage Manager, TSM)
       client command line applications with a
       client system-options file "dsm.sys"
     '');
@@ -251,7 +251,7 @@ let
   ];
 
   dsmSysText = ''
-    ****  IBM Spectrum Protect (Tivoli Storage Manager)
+    ****  IBM Storage Protect (Tivoli Storage Manager)
     ****  client system-options file "dsm.sys".
     ****  Do not edit!
     ****  This file is generated by NixOS configuration.
diff --git a/nixos/modules/services/backup/tsm.nix b/nixos/modules/services/backup/tsm.nix
index c4de0b16d47d..8968321f3cee 100644
--- a/nixos/modules/services/backup/tsm.nix
+++ b/nixos/modules/services/backup/tsm.nix
@@ -10,7 +10,7 @@ let
   options.services.tsmBackup = {
     enable = mkEnableOption (lib.mdDoc ''
       automatic backups with the
-      IBM Spectrum Protect (Tivoli Storage Manager, TSM) client.
+      IBM Storage Protect (Tivoli Storage Manager, TSM) client.
       This also enables
       {option}`programs.tsmClient.enable`
     '');
@@ -81,7 +81,7 @@ in
     programs.tsmClient.servers.${cfg.servername}.passwdDir =
       mkDefault "/var/lib/tsm-backup/password";
     systemd.services.tsm-backup = {
-      description = "IBM Spectrum Protect (Tivoli Storage Manager) Backup";
+      description = "IBM Storage Protect (Tivoli Storage Manager) Backup";
       # DSM_LOG needs a trailing slash to have it treated as a directory.
       # `/var/log` would be littered with TSM log files otherwise.
       environment.DSM_LOG = "/var/log/tsm-backup/";
@@ -89,7 +89,7 @@ in
       environment.HOME = "/var/lib/tsm-backup";
       serviceConfig = {
         # for exit status description see
-        # https://www.ibm.com/docs/en/spectrum-protect/8.1.13?topic=clients-client-return-codes
+        # https://www.ibm.com/docs/en/storage-protect/8.1.20?topic=clients-client-return-codes
         SuccessExitStatus = "4 8";
         # The `-se` option must come after the command.
         # The `-optfile` option suppresses a `dsm.opt`-not-found warning.

From d5f337809e92aad743c683d3be47051052f096eb Mon Sep 17 00:00:00 2001
From: Yarny0 <41838844+Yarny0@users.noreply.github.com>
Date: Sat, 18 Nov 2023 13:09:20 +0100
Subject: [PATCH 2/9] nixos/backup/tsm: use `lib.getExe'` for service command
 line

---
 nixos/modules/services/backup/tsm.nix | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/nixos/modules/services/backup/tsm.nix b/nixos/modules/services/backup/tsm.nix
index 8968321f3cee..583334cba298 100644
--- a/nixos/modules/services/backup/tsm.nix
+++ b/nixos/modules/services/backup/tsm.nix
@@ -3,6 +3,7 @@
 let
 
   inherit (lib.attrsets) hasAttr;
+  inherit (lib.meta) getExe';
   inherit (lib.modules) mkDefault mkIf;
   inherit (lib.options) mkEnableOption mkOption;
   inherit (lib.types) nonEmptyStr nullOr;
@@ -94,7 +95,7 @@ in
         # The `-se` option must come after the command.
         # The `-optfile` option suppresses a `dsm.opt`-not-found warning.
         ExecStart =
-          "${cfgPrg.wrappedPackage}/bin/dsmc ${cfg.command} -se='${cfg.servername}' -optfile=/dev/null";
+          "${getExe' cfgPrg.wrappedPackage "dsmc"} ${cfg.command} -se='${cfg.servername}' -optfile=/dev/null";
         LogsDirectory = "tsm-backup";
         StateDirectory = "tsm-backup";
         StateDirectoryMode = "0750";

From 363cf1e363cdf948f11bdc46596df5b2a4bec30b Mon Sep 17 00:00:00 2001
From: Yarny0 <41838844+Yarny0@users.noreply.github.com>
Date: Sat, 18 Nov 2023 13:55:36 +0100
Subject: [PATCH 3/9] nixos/tsm-client: use `mkPackageOption` for
 `wrappedPackage`

---
 nixos/modules/programs/tsm-client.nix | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/nixos/modules/programs/tsm-client.nix b/nixos/modules/programs/tsm-client.nix
index 67b9f6d7a362..e9e8d14c0e4c 100644
--- a/nixos/modules/programs/tsm-client.nix
+++ b/nixos/modules/programs/tsm-client.nix
@@ -7,7 +7,7 @@ let
   inherit (lib.modules) mkDefault mkIf;
   inherit (lib.options) literalExpression mkEnableOption mkOption mkPackageOption;
   inherit (lib.strings) concatLines optionalString toLower;
-  inherit (lib.types) addCheck attrsOf lines nonEmptyStr nullOr package path port str strMatching submodule;
+  inherit (lib.types) addCheck attrsOf lines nonEmptyStr nullOr path port str strMatching submodule;
 
   # Checks if given list of strings contains unique
   # elements when compared without considering case.
@@ -222,16 +222,16 @@ let
         to add paths to the client system-options file.
       '';
     };
-    wrappedPackage = mkOption {
-      type = package;
-      readOnly = true;
-      description = lib.mdDoc ''
-        The TSM client derivation, wrapped with the path
-        to the client system-options file "dsm.sys".
-        This option is to provide the effective derivation
+    wrappedPackage = mkPackageOption pkgs "tsm-client" {
+      default = null;
+      extraDescription = ''
+        This option is to provide the effective derivation,
+        wrapped with the path to the
+        client system-options file "dsm.sys".
+        It should not be changed, but exists
         for other modules that want to call TSM executables.
       '';
-    };
+    } // { readOnly = true; };
   };
 
   cfg = config.programs.tsmClient;

From fe96d79adf3ff2ca15d21163972639fd4964f589 Mon Sep 17 00:00:00 2001
From: Yarny0 <41838844+Yarny0@users.noreply.github.com>
Date: Sat, 18 Nov 2023 12:22:46 +0100
Subject: [PATCH 4/9] nixos/tsm-client: drop own `checkIUnique` for `allUnique`

---
 nixos/modules/programs/tsm-client.nix | 16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/nixos/modules/programs/tsm-client.nix b/nixos/modules/programs/tsm-client.nix
index e9e8d14c0e4c..bf240ea64cb3 100644
--- a/nixos/modules/programs/tsm-client.nix
+++ b/nixos/modules/programs/tsm-client.nix
@@ -2,23 +2,13 @@
 
 let
 
-  inherit (builtins) length map;
   inherit (lib.attrsets) attrNames filterAttrs hasAttr mapAttrs mapAttrsToList optionalAttrs;
+  inherit (lib.lists) allUnique map;
   inherit (lib.modules) mkDefault mkIf;
   inherit (lib.options) literalExpression mkEnableOption mkOption mkPackageOption;
   inherit (lib.strings) concatLines optionalString toLower;
   inherit (lib.types) addCheck attrsOf lines nonEmptyStr nullOr path port str strMatching submodule;
 
-  # Checks if given list of strings contains unique
-  # elements when compared without considering case.
-  # Type: checkIUnique :: [string] -> bool
-  # Example: checkIUnique ["foo" "Foo"] => false
-  checkIUnique = lst:
-    let
-      lenUniq = l: length (lib.lists.unique l);
-    in
-      lenUniq lst == lenUniq (map toLower lst);
-
   # TSM rejects servername strings longer than 64 chars.
   servernameType = strMatching ".{1,64}";
 
@@ -110,7 +100,7 @@ let
       # differ only by upper and lower case.
       type = addCheck
         (attrsOf (nullOr str))
-        (attrs: checkIUnique (attrNames attrs));
+        (attrs: allUnique (map toLower (attrNames attrs)));
       default = {};
       example.compression = "yes";
       example.passwordaccess = null;
@@ -238,7 +228,7 @@ let
 
   assertions = [
     {
-      assertion = checkIUnique (mapAttrsToList (k: v: v.name) cfg.servers);
+      assertion = allUnique (mapAttrsToList (k: v: toLower v.name) cfg.servers);
       message = ''
         TSM servernames contain duplicate name
         (note that case doesn't matter!)

From 5bc6eb731ea5a3824e7e45e1641098198475f62f Mon Sep 17 00:00:00 2001
From: Yarny0 <41838844+Yarny0@users.noreply.github.com>
Date: Mon, 21 Aug 2023 16:45:24 +0200
Subject: [PATCH 5/9] nixos/tsm-client: server alias names cannot have spaces

---
 nixos/modules/programs/tsm-client.nix | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/nixos/modules/programs/tsm-client.nix b/nixos/modules/programs/tsm-client.nix
index bf240ea64cb3..039eab1af553 100644
--- a/nixos/modules/programs/tsm-client.nix
+++ b/nixos/modules/programs/tsm-client.nix
@@ -10,7 +10,7 @@ let
   inherit (lib.types) addCheck attrsOf lines nonEmptyStr nullOr path port str strMatching submodule;
 
   # TSM rejects servername strings longer than 64 chars.
-  servernameType = strMatching ".{1,64}";
+  servernameType = strMatching "[^[:space:]]{1,64}";
 
   serverOptions = { name, config, ... }: {
     options.name = mkOption {

From 8b918ed8ab43d9f99b2673c0ac5c56e55ef93b25 Mon Sep 17 00:00:00 2001
From: Yarny0 <41838844+Yarny0@users.noreply.github.com>
Date: Mon, 21 Aug 2023 16:45:31 +0200
Subject: [PATCH 6/9] nixos/tsm-client: submodule doesn't need singleton list

---
 nixos/modules/programs/tsm-client.nix | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/nixos/modules/programs/tsm-client.nix b/nixos/modules/programs/tsm-client.nix
index 039eab1af553..39b69054de5d 100644
--- a/nixos/modules/programs/tsm-client.nix
+++ b/nixos/modules/programs/tsm-client.nix
@@ -168,7 +168,7 @@ let
       client system-options file "dsm.sys"
     '');
     servers = mkOption {
-      type = attrsOf (submodule [ serverOptions ]);
+      type = attrsOf (submodule serverOptions);
       default = {};
       example.mainTsmServer = {
         server = "tsmserver.company.com";

From 3fb29fecd5ebeb84432bb693d678a39f6104fe85 Mon Sep 17 00:00:00 2001
From: Yarny0 <41838844+Yarny0@users.noreply.github.com>
Date: Wed, 23 Aug 2023 16:00:32 +0200
Subject: [PATCH 7/9] nixos/tsm-client: use `freeformType` for server config

`tsm-client` uses a global configuration
file that must contain coordinates for each
server that it is supposed to contact.
This configuration consists of text
lines with key-value pairs.

In the NixOS module, these servers may be declared
with an attribute set, where the attribute name
defines an alias for the server, and the value
is again an attribute set with the settings for
the respective server.
This is organized as an option of type `attrsOf submodule...`.

Before this commit:

Important settings have their own option within
the submodule.  For everything else, there is
the "catch-all" option `extraConfig` that may
be used to declare any key-value pairs.
There is also `text` that can be used to
add arbitrary text to each server's
section in the global config file.

After this commit:

`extraConfig` and `text` are gone,
the attribute names and values of each server's attribute
set are translated directly into key-value pairs,
with the following notable rules:

* Lists are translated into multiple lines
  with the same key, as such is permitted by
  the software for certain keys.
* `null` may be used to override/shadow a value that
  is defined elsewhere and hides the corresponding key.

Those "important settings" that have previously been
defined as dedicated options are still defined as such,
but they have been renamed to match their
corresponding key names in the configuration file.
There is a notable exception:
"Our" boolean option `genPasswd` influences the "real"
option `passwordaccess', but the latter one is
uncomfortable to use and might lead
to undesirable outcome if used the wrong way.
So it seems advisable to keep the boolean option
and the warning in its description.
To this end, the value of `getPasswd` itself is
later filtered out when the config file is generated.

The tsm-backup service module and the vm test are adapted.

Migration code will be added in a separate
commit to permit easy reversal later, when the
migration code is no longer deemed necessary.
---
 nixos/modules/programs/tsm-client.nix | 208 ++++++++++++--------------
 nixos/modules/services/backup/tsm.nix |   2 +-
 nixos/tests/tsm-client-gui.nix        |   6 +-
 3 files changed, 98 insertions(+), 118 deletions(-)

diff --git a/nixos/modules/programs/tsm-client.nix b/nixos/modules/programs/tsm-client.nix
index 39b69054de5d..093f0b52c519 100644
--- a/nixos/modules/programs/tsm-client.nix
+++ b/nixos/modules/programs/tsm-client.nix
@@ -2,163 +2,99 @@
 
 let
 
-  inherit (lib.attrsets) attrNames filterAttrs hasAttr mapAttrs mapAttrsToList optionalAttrs;
-  inherit (lib.lists) allUnique map;
+  inherit (lib.attrsets) attrValues mapAttrsToList removeAttrs;
+  inherit (lib.lists) allUnique concatLists elem isList map;
   inherit (lib.modules) mkDefault mkIf;
-  inherit (lib.options) literalExpression mkEnableOption mkOption mkPackageOption;
+  inherit (lib.options) mkEnableOption mkOption mkPackageOption;
   inherit (lib.strings) concatLines optionalString toLower;
-  inherit (lib.types) addCheck attrsOf lines nonEmptyStr nullOr path port str strMatching submodule;
+  inherit (lib.trivial) isInt;
+  inherit (lib.types) addCheck attrsOf coercedTo either enum int lines listOf nonEmptyStr nullOr oneOf path port singleLineStr strMatching submodule;
+
+  scalarType =
+    # see the option's description below for the
+    # handling/transformation of each possible type
+    oneOf [ (enum [ true null ]) int path singleLineStr ];
 
   # TSM rejects servername strings longer than 64 chars.
   servernameType = strMatching "[^[:space:]]{1,64}";
 
   serverOptions = { name, config, ... }: {
-    options.name = mkOption {
+    freeformType = attrsOf (either scalarType (listOf scalarType));
+    # Client system-options file directives are explained here:
+    # https://www.ibm.com/docs/en/storage-protect/8.1.20?topic=commands-processing-options
+    options.servername = mkOption {
       type = servernameType;
+      default = name;
       example = "mainTsmServer";
       description = lib.mdDoc ''
         Local name of the IBM TSM server,
-        must be uncapitalized and no longer than 64 chars.
-        The value will be used for the
-        `server`
-        directive in {file}`dsm.sys`.
+        must not contain space or more than 64 chars.
       '';
     };
-    options.server = mkOption {
+    options.tcpserveraddress = mkOption {
       type = nonEmptyStr;
       example = "tsmserver.company.com";
       description = lib.mdDoc ''
         Host/domain name or IP address of the IBM TSM server.
-        The value will be used for the
-        `tcpserveraddress`
-        directive in {file}`dsm.sys`.
       '';
     };
-    options.port = mkOption {
+    options.tcpport = mkOption {
       type = addCheck port (p: p<=32767);
       default = 1500;  # official default
       description = lib.mdDoc ''
         TCP port of the IBM TSM server.
-        The value will be used for the
-        `tcpport`
-        directive in {file}`dsm.sys`.
         TSM does not support ports above 32767.
       '';
     };
-    options.node = mkOption {
+    options.nodename = mkOption {
       type = nonEmptyStr;
       example = "MY-TSM-NODE";
       description = lib.mdDoc ''
         Target node name on the IBM TSM server.
-        The value will be used for the
-        `nodename`
-        directive in {file}`dsm.sys`.
       '';
     };
     options.genPasswd = mkEnableOption (lib.mdDoc ''
       automatic client password generation.
-      This option influences the
-      `passwordaccess`
-      directive in {file}`dsm.sys`.
+      This option does *not* cause a line in
+      {file}`dsm.sys` by itself, but generates a
+      corresponding `passwordaccess` directive.
       The password will be stored in the directory
-      given by the option {option}`passwdDir`.
+      given by the option {option}`passworddir`.
       *Caution*:
       If this option is enabled and the server forces
       to renew the password (e.g. on first connection),
       a random password will be generated and stored
     '');
-    options.passwdDir = mkOption {
-      type = path;
+    options.passwordaccess = mkOption {
+      type = enum [ "generate" "prompt" ];
+      visible = false;
+    };
+    options.passworddir = mkOption {
+      type = nullOr path;
+      default = null;
       example = "/home/alice/tsm-password";
       description = lib.mdDoc ''
         Directory that holds the TSM
         node's password information.
-        The value will be used for the
-        `passworddir`
-        directive in {file}`dsm.sys`.
       '';
     };
-    options.includeExclude = mkOption {
-      type = lines;
-      default = "";
+    options.inclexcl = mkOption {
+      type = coercedTo lines
+        (pkgs.writeText "inclexcl.dsm.sys")
+        (nullOr path);
+      default = null;
       example = ''
         exclude.dir     /nix/store
         include.encrypt /home/.../*
       '';
       description = lib.mdDoc ''
-        `include.*` and
-        `exclude.*` directives to be
-        used when sending files to the IBM TSM server.
-        The lines will be written into a file that the
-        `inclexcl`
-        directive in {file}`dsm.sys` points to.
+        Text lines with `include.*` and `exclude.*` directives
+        to be used when sending files to the IBM TSM server,
+        or an absolute path pointing to a file with such lines.
       '';
     };
-    options.extraConfig = mkOption {
-      # TSM option keys are case insensitive;
-      # we have to ensure there are no keys that
-      # differ only by upper and lower case.
-      type = addCheck
-        (attrsOf (nullOr str))
-        (attrs: allUnique (map toLower (attrNames attrs)));
-      default = {};
-      example.compression = "yes";
-      example.passwordaccess = null;
-      description = lib.mdDoc ''
-        Additional key-value pairs for the server stanza.
-        Values must be strings, or `null`
-        for the key not to be used in the stanza
-        (e.g. to overrule values generated by other options).
-      '';
-    };
-    options.text = mkOption {
-      type = lines;
-      example = literalExpression
-        ''lib.modules.mkAfter "compression no"'';
-      description = lib.mdDoc ''
-        Additional text lines for the server stanza.
-        This option can be used if certion configuration keys
-        must be used multiple times or ordered in a certain way
-        as the {option}`extraConfig` option can't
-        control the order of lines in the resulting stanza.
-        Note that the `server`
-        line at the beginning of the stanza is
-        not part of this option's value.
-      '';
-    };
-    options.stanza = mkOption {
-      type = str;
-      internal = true;
-      visible = false;
-      description = lib.mdDoc "Server stanza text generated from the options.";
-    };
-    config.name = mkDefault name;
-    # Client system-options file directives are explained here:
-    # https://www.ibm.com/docs/en/storage-protect/8.1.20?topic=commands-processing-options
-    config.extraConfig =
-      mapAttrs (lib.trivial.const mkDefault) (
-        {
-          commmethod = "v6tcpip";  # uses v4 or v6, based on dns lookup result
-          tcpserveraddress = config.server;
-          tcpport = builtins.toString config.port;
-          nodename = config.node;
-          passwordaccess = if config.genPasswd then "generate" else "prompt";
-          passworddir = ''"${config.passwdDir}"'';
-        } // optionalAttrs (config.includeExclude!="") {
-          inclexcl = ''"${pkgs.writeText "inclexcl.dsm.sys" config.includeExclude}"'';
-        }
-      );
-    config.text =
-      let
-        attrset = filterAttrs (k: v: v!=null) config.extraConfig;
-        mkLine = k: v: k + optionalString (v!="") "  ${v}";
-        lines = mapAttrsToList mkLine attrset;
-      in
-        concatLines lines;
-    config.stanza = ''
-      server  ${config.name}
-      ${config.text}
-    '';
+    config.commmethod = mkDefault "v6tcpip";  # uses v4 or v6, based on dns lookup result
+    config.passwordaccess = if config.genPasswd then "generate" else "prompt";
   };
 
   options.programs.tsmClient = {
@@ -171,13 +107,24 @@ let
       type = attrsOf (submodule serverOptions);
       default = {};
       example.mainTsmServer = {
-        server = "tsmserver.company.com";
-        node = "MY-TSM-NODE";
-        extraConfig.compression = "yes";
+        tcpserveraddress = "tsmserver.company.com";
+        nodename = "MY-TSM-NODE";
+        compression = "yes";
       };
       description = lib.mdDoc ''
         Server definitions ("stanzas")
         for the client system-options file.
+        The name of each entry will be used for
+        the internal `servername` by default.
+        Each attribute will be transformed into a line
+        with a key-value pair within the server's stanza.
+        Integers as values will be
+        canonically turned into strings.
+        The boolean value `true` will be turned
+        into a line with just the attribute's name.
+        The value `null` will not generate a line.
+        A list as values generates an entry for
+        each value, according to the rules above.
       '';
     };
     defaultServername = mkOption {
@@ -225,21 +172,54 @@ let
   };
 
   cfg = config.programs.tsmClient;
+  servernames = map (s: s.servername) (attrValues cfg.servers);
 
-  assertions = [
+  assertions =
+  [
     {
-      assertion = allUnique (mapAttrsToList (k: v: toLower v.name) cfg.servers);
+      assertion = allUnique (map toLower servernames);
       message = ''
-        TSM servernames contain duplicate name
-        (note that case doesn't matter!)
+        TSM server names
+        (option `programs.tsmClient.servers`)
+        contain duplicate name
+        (note that server names are case insensitive).
       '';
     }
     {
-      assertion = (cfg.defaultServername!=null)->(hasAttr cfg.defaultServername cfg.servers);
-      message = "TSM defaultServername not found in list of servers";
+      assertion = (cfg.defaultServername!=null)->(elem cfg.defaultServername servernames);
+      message = ''
+        TSM default server name
+        `programs.tsmClient.defaultServername="${cfg.defaultServername}"`
+        not found in server names in
+        `programs.tsmClient.servers`.
+      '';
     }
   ];
 
+  makeDsmSysLines = key: value:
+    # Turn a key-value pair from the server options attrset
+    # into zero (value==null), one (scalar value) or
+    # more (value is list) configuration stanza lines.
+    if isList value then map (makeDsmSysLines key) value else  # recurse into list
+    if value == null then [ ] else  # skip `null` value
+    [ ("  ${key}${
+      if value == true then "" else  # just output key if value is `true`
+      if isInt value then "  ${builtins.toString value}" else
+      if path.check value then "  \"${value}\"" else  # enclose path in ".."
+      if singleLineStr.check value then "  ${value}" else
+      throw "assertion failed: cannot convert type"  # should never happen
+    }") ];
+
+  makeDsmSysStanza = {servername, ... }@serverCfg:
+    let
+      # drop special values that should not go into server config block
+      attrs = removeAttrs serverCfg [ "servername" "genPasswd" ];
+    in
+      ''
+        servername  ${servername}
+        ${concatLines (concatLists (mapAttrsToList makeDsmSysLines attrs))}
+      '';
+
   dsmSysText = ''
     ****  IBM Storage Protect (Tivoli Storage Manager)
     ****  client system-options file "dsm.sys".
@@ -248,7 +228,7 @@ let
 
     ${optionalString (cfg.defaultServername!=null) "defaultserver  ${cfg.defaultServername}"}
 
-    ${concatLines (mapAttrsToList (k: v: v.stanza) cfg.servers)}
+    ${concatLines (map makeDsmSysStanza (attrValues cfg.servers))}
   '';
 
 in
diff --git a/nixos/modules/services/backup/tsm.nix b/nixos/modules/services/backup/tsm.nix
index 583334cba298..6798b18b3af7 100644
--- a/nixos/modules/services/backup/tsm.nix
+++ b/nixos/modules/services/backup/tsm.nix
@@ -79,7 +79,7 @@ in
   config = mkIf cfg.enable {
     inherit assertions;
     programs.tsmClient.enable = true;
-    programs.tsmClient.servers.${cfg.servername}.passwdDir =
+    programs.tsmClient.servers.${cfg.servername}.passworddir =
       mkDefault "/var/lib/tsm-backup/password";
     systemd.services.tsm-backup = {
       description = "IBM Storage Protect (Tivoli Storage Manager) Backup";
diff --git a/nixos/tests/tsm-client-gui.nix b/nixos/tests/tsm-client-gui.nix
index e11501da53d0..c9632546db6e 100644
--- a/nixos/tests/tsm-client-gui.nix
+++ b/nixos/tests/tsm-client-gui.nix
@@ -18,9 +18,9 @@ import ./make-test-python.nix ({ lib, pkgs, ... }: {
       defaultServername = "testserver";
       servers.testserver = {
         # 192.0.0.8 is a "dummy address" according to RFC 7600
-        server = "192.0.0.8";
-        node = "SOME-NODE";
-        passwdDir = "/tmp";
+        tcpserveraddress = "192.0.0.8";
+        nodename = "SOME-NODE";
+        passworddir = "/tmp";
       };
     };
   };

From 98c03bf8c6b173a9eb4eb0296a9abadcb9a65873 Mon Sep 17 00:00:00 2001
From: Yarny0 <41838844+Yarny0@users.noreply.github.com>
Date: Wed, 23 Aug 2023 16:56:55 +0200
Subject: [PATCH 8/9] nixos/tsm-client: stricter assertions

Check for spaces or duplicate names in server config keys.
Since server config keys are case insensitive,
a setting like

```
{
  compression = "yes";
  Compression = "no";
}
```

would lead to an ambiguous configuration.
---
 nixos/modules/programs/tsm-client.nix | 53 +++++++++++++++++----------
 1 file changed, 34 insertions(+), 19 deletions(-)

diff --git a/nixos/modules/programs/tsm-client.nix b/nixos/modules/programs/tsm-client.nix
index 093f0b52c519..03dc50eea801 100644
--- a/nixos/modules/programs/tsm-client.nix
+++ b/nixos/modules/programs/tsm-client.nix
@@ -2,11 +2,11 @@
 
 let
 
-  inherit (lib.attrsets) attrValues mapAttrsToList removeAttrs;
-  inherit (lib.lists) allUnique concatLists elem isList map;
+  inherit (lib.attrsets) attrNames attrValues mapAttrsToList removeAttrs;
+  inherit (lib.lists) all allUnique concatLists elem isList map;
   inherit (lib.modules) mkDefault mkIf;
   inherit (lib.options) mkEnableOption mkOption mkPackageOption;
-  inherit (lib.strings) concatLines optionalString toLower;
+  inherit (lib.strings) concatLines match optionalString toLower;
   inherit (lib.trivial) isInt;
   inherit (lib.types) addCheck attrsOf coercedTo either enum int lines listOf nonEmptyStr nullOr oneOf path port singleLineStr strMatching submodule;
 
@@ -175,26 +175,41 @@ let
   servernames = map (s: s.servername) (attrValues cfg.servers);
 
   assertions =
-  [
-    {
-      assertion = allUnique (map toLower servernames);
+    [
+      {
+        assertion = allUnique (map toLower servernames);
+        message = ''
+          TSM server names
+          (option `programs.tsmClient.servers`)
+          contain duplicate name
+          (note that server names are case insensitive).
+        '';
+      }
+      {
+        assertion = (cfg.defaultServername!=null)->(elem cfg.defaultServername servernames);
+        message = ''
+          TSM default server name
+          `programs.tsmClient.defaultServername="${cfg.defaultServername}"`
+          not found in server names in
+          `programs.tsmClient.servers`.
+        '';
+      }
+    ] ++ (mapAttrsToList (name: serverCfg: {
+      assertion = all (key: null != match "[^[:space:]]+" key) (attrNames serverCfg);
       message = ''
-        TSM server names
-        (option `programs.tsmClient.servers`)
-        contain duplicate name
-        (note that server names are case insensitive).
+        TSM server setting names in
+        `programs.tsmClient.servers.${name}.*`
+        contain spaces, but that's not allowed.
       '';
-    }
-    {
-      assertion = (cfg.defaultServername!=null)->(elem cfg.defaultServername servernames);
+    }) cfg.servers) ++ (mapAttrsToList (name: serverCfg: {
+      assertion = allUnique (map toLower (attrNames serverCfg));
       message = ''
-        TSM default server name
-        `programs.tsmClient.defaultServername="${cfg.defaultServername}"`
-        not found in server names in
-        `programs.tsmClient.servers`.
+        TSM server setting names in
+        `programs.tsmClient.servers.${name}.*`
+        contain duplicate names
+        (note that setting names are case insensitive).
       '';
-    }
-  ];
+    }) cfg.servers);
 
   makeDsmSysLines = key: value:
     # Turn a key-value pair from the server options attrset

From 20a9a21b246ca684e4e5a45e543a8a53be46e1d2 Mon Sep 17 00:00:00 2001
From: Yarny0 <41838844+Yarny0@users.noreply.github.com>
Date: Wed, 23 Aug 2023 21:29:29 +0200
Subject: [PATCH 9/9] nixos/tsm-client: add migration code for freeform
 settings

To help users migrate from the previous
settings to new freeform settings type,
the commit at hand adds some
`mkRemovedOptionModule` and `mkRenamedOptionModule`.

These modules are not designed to work
inside an attribute set of submodules.
They create values for `assertions` and
`warnings` to inform the user of required changes.
Also, these informational texts do not contain
the full attribute path of the changed options.
To work around these deficiencies,
we define the required options `assertions` and `warnings`
inside the submodule and later add the values collected
inside these options to the corresponding top-level options.
In the course of doing so, we also add the full attribute path
to the informational texts so the user knows these warning
and error messages refer to the `tsmClient.servers` option.

Also, we have to filter out `warnings`, `assertions`, and
the "old" options when rendering the target config file.
---
 nixos/modules/programs/tsm-client.nix | 36 ++++++++++++++++++++++++---
 1 file changed, 33 insertions(+), 3 deletions(-)

diff --git a/nixos/modules/programs/tsm-client.nix b/nixos/modules/programs/tsm-client.nix
index 03dc50eea801..45d436221ee3 100644
--- a/nixos/modules/programs/tsm-client.nix
+++ b/nixos/modules/programs/tsm-client.nix
@@ -1,4 +1,5 @@
-{ config, lib, pkgs, ... }:
+{ config, lib, options, pkgs, ... }:  # XXX migration code for freeform settings: `options` can be removed in 2025
+let optionsGlobal = options; in
 
 let
 
@@ -95,6 +96,19 @@ let
     };
     config.commmethod = mkDefault "v6tcpip";  # uses v4 or v6, based on dns lookup result
     config.passwordaccess = if config.genPasswd then "generate" else "prompt";
+    # XXX migration code for freeform settings, these can be removed in 2025:
+    options.warnings = optionsGlobal.warnings;
+    options.assertions = optionsGlobal.assertions;
+    imports = let inherit (lib.modules) mkRemovedOptionModule mkRenamedOptionModule; in [
+      (mkRemovedOptionModule [ "extraConfig" ] "Please just add options directly to the server attribute set, cf. the description of `programs.tsmClient.servers`.")
+      (mkRemovedOptionModule [ "text" ] "Please just add options directly to the server attribute set, cf. the description of `programs.tsmClient.servers`.")
+      (mkRenamedOptionModule [ "name" ] [ "servername" ])
+      (mkRenamedOptionModule [ "server" ] [ "tcpserveraddress" ])
+      (mkRenamedOptionModule [ "port" ] [ "tcpport" ])
+      (mkRenamedOptionModule [ "node" ] [ "nodename" ])
+      (mkRenamedOptionModule [ "passwdDir" ] [ "passworddir" ])
+      (mkRenamedOptionModule [ "includeExclude" ] [ "inclexcl" ])
+    ];
   };
 
   options.programs.tsmClient = {
@@ -209,7 +223,9 @@ let
         contain duplicate names
         (note that setting names are case insensitive).
       '';
-    }) cfg.servers);
+    }) cfg.servers)
+    # XXX migration code for freeform settings, this can be removed in 2025:
+    ++ (enrichMigrationInfos "assertions" (addText: { assertion, message }: { inherit assertion; message = addText message; }));
 
   makeDsmSysLines = key: value:
     # Turn a key-value pair from the server options attrset
@@ -228,7 +244,12 @@ let
   makeDsmSysStanza = {servername, ... }@serverCfg:
     let
       # drop special values that should not go into server config block
-      attrs = removeAttrs serverCfg [ "servername" "genPasswd" ];
+      attrs = removeAttrs serverCfg [ "servername" "genPasswd"
+        # XXX migration code for freeform settings, these can be removed in 2025:
+        "assertions" "warnings"
+        "extraConfig" "text"
+        "name" "server" "port" "node" "passwdDir" "includeExclude"
+      ];
     in
       ''
         servername  ${servername}
@@ -246,6 +267,13 @@ let
     ${concatLines (map makeDsmSysStanza (attrValues cfg.servers))}
   '';
 
+  # XXX migration code for freeform settings, this can be removed in 2025:
+  enrichMigrationInfos = what: how: concatLists (
+    mapAttrsToList
+    (name: serverCfg: map (how (text: "In `programs.tsmClient.servers.${name}`: ${text}")) serverCfg."${what}")
+    cfg.servers
+  );
+
 in
 
 {
@@ -260,6 +288,8 @@ in
       dsmSysApi = dsmSysCli;
     };
     environment.systemPackages = [ cfg.wrappedPackage ];
+    # XXX migration code for freeform settings, this can be removed in 2025:
+    warnings = enrichMigrationInfos "warnings" (addText: addText);
   };
 
   meta.maintainers = [ lib.maintainers.yarny ];