From 08e8701673a84b800e0960c50f78793a23e635ba Mon Sep 17 00:00:00 2001
From: Robert Helgesson <robert@rycee.net>
Date: Sat, 24 Mar 2018 17:38:45 +0100
Subject: [PATCH] lib.types: fix loaOf behavior for long lists

Assigning a list of 10 or more elements to an option having the type
`loaOf a` produces a configuration value that is not honoring the
order of the original list. This commit fixes this and a related issue
arising when 10 or more lists are merged into this type of option.
---
 lib/tests/modules.sh                          |  6 ++++
 lib/tests/modules/loaOf-with-long-list.nix    | 19 ++++++++++++
 .../modules/loaOf-with-many-list-merges.nix   | 19 ++++++++++++
 lib/types.nix                                 | 29 +++++++++++++------
 4 files changed, 64 insertions(+), 9 deletions(-)
 create mode 100644 lib/tests/modules/loaOf-with-long-list.nix
 create mode 100644 lib/tests/modules/loaOf-with-many-list-merges.nix

diff --git a/lib/tests/modules.sh b/lib/tests/modules.sh
index c6b3fd912a9..b83e1eb7d82 100755
--- a/lib/tests/modules.sh
+++ b/lib/tests/modules.sh
@@ -143,6 +143,12 @@ checkConfigOutput "12" config.value ./declare-coerced-value-unsound.nix
 checkConfigError 'The option value .* in .* is not.*8 bit signed integer.* or string convertible to it' config.value ./declare-coerced-value-unsound.nix ./define-value-string-bigint.nix
 checkConfigError 'unrecognised JSON value' config.value ./declare-coerced-value-unsound.nix ./define-value-string-arbitrary.nix
 
+# Check loaOf with long list.
+checkConfigOutput "1 2 3 4 5 6 7 8 9 10" config.result ./loaOf-with-long-list.nix
+
+# Check loaOf with many merges of lists.
+checkConfigOutput "1 2 3 4 5 6 7 8 9 10" config.result ./loaOf-with-many-list-merges.nix
+
 cat <<EOF
 ====== module tests ======
 $pass Pass
diff --git a/lib/tests/modules/loaOf-with-long-list.nix b/lib/tests/modules/loaOf-with-long-list.nix
new file mode 100644
index 00000000000..f30903c47e5
--- /dev/null
+++ b/lib/tests/modules/loaOf-with-long-list.nix
@@ -0,0 +1,19 @@
+{ config, lib, ... }:
+
+{
+  options = {
+    loaOfInt = lib.mkOption {
+      type = lib.types.loaOf lib.types.int;
+    };
+
+    result = lib.mkOption {
+      type = lib.types.str;
+    };
+  };
+
+  config = {
+    loaOfInt = [ 1 2 3 4 5 6 7 8 9 10 ];
+
+    result = toString (lib.attrValues config.loaOfInt);
+  };
+}
diff --git a/lib/tests/modules/loaOf-with-many-list-merges.nix b/lib/tests/modules/loaOf-with-many-list-merges.nix
new file mode 100644
index 00000000000..f8f8a8da82b
--- /dev/null
+++ b/lib/tests/modules/loaOf-with-many-list-merges.nix
@@ -0,0 +1,19 @@
+{ config, lib, ... }:
+
+{
+  options = {
+    loaOfInt = lib.mkOption {
+      type = lib.types.loaOf lib.types.int;
+    };
+
+    result = lib.mkOption {
+      type = lib.types.str;
+    };
+  };
+
+  config = {
+    loaOfInt = lib.mkMerge (map lib.singleton [ 1 2 3 4 5 6 7 8 9 10 ]);
+
+    result = toString (lib.attrValues config.loaOfInt);
+  };
+}
diff --git a/lib/types.nix b/lib/types.nix
index 17acb92a944..77271689772 100644
--- a/lib/types.nix
+++ b/lib/types.nix
@@ -280,15 +280,26 @@ rec {
     # List or attribute set of ...
     loaOf = elemType:
       let
-        convertIfList = defIdx: def:
+        convertAllLists = defs:
+          let
+            padWidth = stringLength (toString (length defs));
+            unnamedPrefix = i: "unnamed-" + fixedWidthNumber padWidth i + ".";
+          in
+            imap1 (i: convertIfList (unnamedPrefix i)) defs;
+
+        convertIfList = unnamedPrefix: def:
           if isList def.value then
-            { inherit (def) file;
-              value = listToAttrs (
-                imap1 (elemIdx: elem:
-                  { name = elem.name or "unnamed-${toString defIdx}.${toString elemIdx}";
-                    value = elem;
-                  }) def.value);
-            }
+            let
+              padWidth = stringLength (toString (length def.value));
+              unnamed = i: unnamedPrefix + fixedWidthNumber padWidth i;
+            in
+              { inherit (def) file;
+                value = listToAttrs (
+                  imap1 (elemIdx: elem:
+                    { name = elem.name or (unnamed elemIdx);
+                      value = elem;
+                    }) def.value);
+              }
           else
             def;
         listOnly = listOf elemType;
@@ -297,7 +308,7 @@ rec {
         name = "loaOf";
         description = "list or attribute set of ${elemType.description}s";
         check = x: isList x || isAttrs x;
-        merge = loc: defs: attrOnly.merge loc (imap1 convertIfList defs);
+        merge = loc: defs: attrOnly.merge loc (convertAllLists defs);
         getSubOptions = prefix: elemType.getSubOptions (prefix ++ ["<name?>"]);
         getSubModules = elemType.getSubModules;
         substSubModules = m: loaOf (elemType.substSubModules m);