Merge pull request #177656 from hercules-ci/submodule-refactor

Submodule refactor
NixOS · Jun 14, 2022 · 6616de3 · 6616de3
2 parents 61f6379 + 9dead55
commit 6616de3
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 26 deletions.
diff --git a/lib/modules.nix b/lib/modules.nix
@@ -113,10 +113,6 @@ rec {
                   args ? {}
                 , # This would be remove in the future, Prefer _module.check option instead.
                   check ? true
-                  # Internal variable to avoid `_key` collisions regardless
-                  # of `extendModules`. Used in `submoduleWith`.
-                  # Test case: lib/tests/modules, "168767"
-                , extensionOffset ? 0
                 }:
     let
       withWarnings = x:
@@ -345,17 +341,15 @@ rec {
         modules ? [],
         specialArgs ? {},
         prefix ? [],
-        extensionOffset ? length modules,
         }:
           evalModules (evalModulesArgs // {
             modules = regularModules ++ modules;
             specialArgs = evalModulesArgs.specialArgs or {} // specialArgs;
             prefix = extendArgs.prefix or evalModulesArgs.prefix or [];
-            inherit extensionOffset;
           });
 
       type = lib.types.submoduleWith {
-        inherit modules specialArgs extensionOffset;
+        inherit modules specialArgs;
       };
 
       result = withWarnings {

diff --git a/lib/tests/modules.sh b/lib/tests/modules.sh
@@ -194,6 +194,10 @@ checkConfigOutput '^"submodule"$' options.submodule.type.description ./declare-s
 ## Paths should be allowed as values and work as expected
 checkConfigOutput '^true$' config.submodule.enable ./declare-submoduleWith-path.nix
 
+# Check the file location information is propagated into submodules
+checkConfigOutput the-file.nix config.submodule.internalFiles.0 ./submoduleFiles.nix
+
+
 # Check that disabledModules works recursively and correctly
 checkConfigOutput '^true$' config.enable ./disable-recursive/main.nix
 checkConfigOutput '^true$' config.enable ./disable-recursive/{main.nix,disable-foo.nix}

diff --git a/lib/tests/modules/submoduleFiles.nix b/lib/tests/modules/submoduleFiles.nix
@@ -0,0 +1,21 @@
+{ lib, ... }: {
+  options.submodule = lib.mkOption {
+    default = {};
+    type = lib.types.submoduleWith {
+      modules = [ ({ options, ... }: {
+        options.value = lib.mkOption {};
+
+        options.internalFiles = lib.mkOption {
+          default = options.value.files;
+        };
+      })];
+    };
+  };
+
+  imports = [
+    {
+      _file = "the-file.nix";
+      submodule.value = 10;
+    }
+  ];
+}
diff --git a/lib/types.nix b/lib/types.nix
@@ -571,28 +571,14 @@ rec {
       , specialArgs ? {}
       , shorthandOnlyDefinesConfig ? false
       , description ? null
-
-        # Internal variable to avoid `_key` collisions regardless
-        # of `extendModules`. Wired through by `evalModules`.
-        # Test case: lib/tests/modules, "168767"
-      , extensionOffset ? 0
       }@attrs:
       let
         inherit (lib.modules) evalModules;
 
-        shorthandToModule = if shorthandOnlyDefinesConfig == false
-          then value: value
-          else value: { config = value; };
-
-        allModules = defs: imap1 (n: { value, file }:
-          if isFunction value
-          then setFunctionArgs
-                (args: lib.modules.unifyModuleSyntax file "${toString file}-${toString (n + extensionOffset)}" (value args))
-                (functionArgs value)
-          else if isAttrs value
-          then
-            lib.modules.unifyModuleSyntax file "${toString file}-${toString (n + extensionOffset)}" (shorthandToModule value)
-          else value
+        allModules = defs: map ({ value, file }:
+          if isAttrs value && shorthandOnlyDefinesConfig
+          then { _file = file; config = value; }
+          else { _file = file; imports = [ value ]; }
         ) defs;
 
         base = evalModules {
@@ -632,7 +618,6 @@ rec {
           (base.extendModules {
             modules = [ { _module.args.name = last loc; } ] ++ allModules defs;
             prefix = loc;
-            extensionOffset = extensionOffset + length defs;
           }).config;
         emptyValue = { value = {}; };
         getSubOptions = prefix: (base.extendModules