Module system improvements for NixOS as a submodule (#75031)

Module system improvements for NixOS as a submodule

lib/default.nix

@@ -102,7 +102,7 @@ let
       commitIdFromGitRepo cleanSourceWith pathHasContext
       canCleanSource;
     inherit (modules) evalModules closeModules unifyModuleSyntax
-      applyIfFunction unpackSubmodule packSubmodule mergeModules
+      applyIfFunction mergeModules
       mergeModules' mergeOptionDecls evalOptionValue mergeDefinitions
       pushDownProperties dischargeProperties filterOverrides
       sortProperties fixupOptionType mkIf mkAssert mkMerge mkOverride

lib/modules.nix

@@ -103,42 +103,42 @@ rec {
       toClosureList = file: parentKey: imap1 (n: x:
         if isAttrs x || isFunction x then
           let key = "${parentKey}:anon-${toString n}"; in
-          unifyModuleSyntax file key (unpackSubmodule (applyIfFunction key) x args)
+          unifyModuleSyntax file key (applyIfFunction key x args)
         else
           let file = toString x; key = toString x; in
           unifyModuleSyntax file key (applyIfFunction key (import x) args));
     in
       builtins.genericClosure {
         startSet = toClosureList unknownModule "" modules;
-        operator = m: toClosureList m.file m.key m.imports;
+        operator = m: toClosureList m._file m.key m.imports;
       };
 
   /* Massage a module into canonical form, that is, a set consisting
      of ‘options’, ‘config’ and ‘imports’ attributes. */
   unifyModuleSyntax = file: key: m:
-    let metaSet = if m ? meta
-      then { meta = m.meta; }
-      else {};
+    let addMeta = config: if m ? meta
+      then mkMerge [ config { meta = m.meta; } ]
+      else config;
     in
     if m ? config || m ? options then
       let badAttrs = removeAttrs m ["_file" "key" "disabledModules" "imports" "options" "config" "meta"]; in
       if badAttrs != {} then
         throw "Module `${key}' has an unsupported attribute `${head (attrNames badAttrs)}'. This is caused by assignments to the top-level attributes `config' or `options'."
       else
-        { file = m._file or file;
+        { _file = m._file or file;
           key = toString m.key or key;
           disabledModules = m.disabledModules or [];
           imports = m.imports or [];
           options = m.options or {};
-          config = mkMerge [ (m.config or {}) metaSet ];
+          config = addMeta (m.config or {});
         }
     else
-      { file = m._file or file;
+      { _file = m._file or file;
         key = toString m.key or key;
         disabledModules = m.disabledModules or [];
         imports = m.require or [] ++ m.imports or [];
         options = {};
-        config = mkMerge [ (removeAttrs m ["_file" "key" "disabledModules" "require" "imports"]) metaSet ];
+        config = addMeta (removeAttrs m ["_file" "key" "disabledModules" "require" "imports"]);
       };
 
   applyIfFunction = key: f: args@{ config, options, lib, ... }: if isFunction f then
@@ -171,17 +171,6 @@ rec {
   else
     f;
 
-  /* We have to pack and unpack submodules. We cannot wrap the expected
-     result of the function as we would no longer be able to list the arguments
-     of the submodule. (see applyIfFunction) */
-  unpackSubmodule = unpack: m: args:
-    if isType "submodule" m then
-      { _file = m.file; } // (unpack m.submodule args)
-    else unpack m args;
-
-  packSubmodule = file: m:
-    { _type = "submodule"; file = file; submodule = m; };
-
   /* Merge a list of modules.  This will recurse over the option
      declarations in all modules, combining them into a single set.
      At the same time, for each option declaration, it will merge the
@@ -189,7 +178,7 @@ rec {
      in the ‘value’ attribute of each option. */
   mergeModules = prefix: modules:
     mergeModules' prefix modules
-      (concatMap (m: map (config: { inherit (m) file; inherit config; }) (pushDownProperties m.config)) modules);
+      (concatMap (m: map (config: { file = m._file; inherit config; }) (pushDownProperties m.config)) modules);
 
   mergeModules' = prefix: options: configs:
     let
@@ -223,7 +212,7 @@ rec {
                ) {} modules;
       # an attrset 'name' => list of submodules that declare ‘name’.
       declsByName = byName "options" (module: option:
-          [{ inherit (module) file; options = option; }]
+          [{ inherit (module) _file; options = option; }]
         ) options;
       # an attrset 'name' => list of submodules that define ‘name’.
       defnsByName = byName "config" (module: value:
@@ -250,7 +239,7 @@ rec {
               firstOption = findFirst (m: isOption m.options) "" decls;
               firstNonOption = findFirst (m: !isOption m.options) "" decls;
             in
-              throw "The option `${showOption loc}' in `${firstOption.file}' is a prefix of options in `${firstNonOption.file}'."
+              throw "The option `${showOption loc}' in `${firstOption._file}' is a prefix of options in `${firstNonOption._file}'."
           else
             mergeModules' loc decls defns
       ))
@@ -267,7 +256,14 @@ rec {
 
      'opts' is a list of modules.  Each module has an options attribute which
      correspond to the definition of 'loc' in 'opt.file'. */
-  mergeOptionDecls = loc: opts:
+  mergeOptionDecls =
+   let
+    packSubmodule = file: m:
+      { _file = file; imports = [ m ]; };
+    coerceOption = file: opt:
+      if isFunction opt then packSubmodule file opt
+      else packSubmodule file { options = opt; };
+   in loc: opts:
     foldl' (res: opt:
       let t  = res.type;
           t' = opt.options.type;
@@ -284,7 +280,7 @@ rec {
          bothHave "apply" ||
          (bothHave "type" && (! typesMergeable))
       then
-        throw "The option `${showOption loc}' in `${opt.file}' is already declared in ${showFiles res.declarations}."
+        throw "The option `${showOption loc}' in `${opt._file}' is already declared in ${showFiles res.declarations}."
       else
         let
           /* Add the modules of the current option to the list of modules
@@ -293,16 +289,14 @@ rec {
              current option declaration as the file use for the submodule.  If the
              submodule defines any filename, then we ignore the enclosing option file. */
           options' = toList opt.options.options;
-          coerceOption = file: opt:
-            if isFunction opt then packSubmodule file opt
-            else packSubmodule file { options = opt; };
+
           getSubModules = opt.options.type.getSubModules or null;
           submodules =
-            if getSubModules != null then map (packSubmodule opt.file) getSubModules ++ res.options
-            else if opt.options ? options then map (coerceOption opt.file) options' ++ res.options
+            if getSubModules != null then map (packSubmodule opt._file) getSubModules ++ res.options
+            else if opt.options ? options then map (coerceOption opt._file) options' ++ res.options
             else res.options;
         in opt.options // res //
-          { declarations = res.declarations ++ [opt.file];
+          { declarations = res.declarations ++ [opt._file];
             options = submodules;
           } // typeSet
     ) { inherit loc; declarations = []; options = []; } opts;

lib/tests/modules.sh

@@ -164,6 +164,24 @@ checkConfigOutput "true" config.enableAlias ./alias-with-priority.nix
 checkConfigOutput "false" config.enable ./alias-with-priority-can-override.nix
 checkConfigOutput "false" config.enableAlias ./alias-with-priority-can-override.nix
 
+# submoduleWith
+
+## specialArgs should work
+checkConfigOutput "foo" config.submodule.foo ./declare-submoduleWith-special.nix
+
+## shorthandOnlyDefines config behaves as expected
+checkConfigOutput "true" config.submodule.config ./declare-submoduleWith-shorthand.nix ./define-submoduleWith-shorthand.nix
+checkConfigError 'is not of type `boolean' config.submodule.config ./declare-submoduleWith-shorthand.nix ./define-submoduleWith-noshorthand.nix
+checkConfigError 'value is a boolean while a set was expected' config.submodule.config ./declare-submoduleWith-noshorthand.nix ./define-submoduleWith-shorthand.nix
+checkConfigOutput "true" config.submodule.config ./declare-submoduleWith-noshorthand.nix ./define-submoduleWith-noshorthand.nix
+
+## submoduleWith should merge all modules in one swoop
+checkConfigOutput "true" config.submodule.inner ./declare-submoduleWith-modules.nix
+checkConfigOutput "true" config.submodule.outer ./declare-submoduleWith-modules.nix
+
+## Paths should be allowed as values and work as expected
+checkConfigOutput "true" config.submodule.enable ./declare-submoduleWith-path.nix
+
 cat <<EOF
 ====== module tests ======
 $pass Pass

lib/tests/modules/declare-submoduleWith-modules.nix

@@ -0,0 +1,30 @@
+{ lib, ... }: {
+  options.submodule = lib.mkOption {
+    type = lib.types.submoduleWith {
+      modules = [
+        {
+          options.inner = lib.mkOption {
+            type = lib.types.bool;
+            default = false;
+          };
+        }
+        {
+          outer = true;
+        }
+      ];
+    };
+    default = {};
+  };
+
+  config.submodule = lib.mkMerge [
+    ({ lib, ... }: {
+      options.outer = lib.mkOption {
+        type = lib.types.bool;
+        default = false;
+      };
+    })
+    {
+      inner = true;
+    }
+  ];
+}

lib/tests/modules/declare-submoduleWith-noshorthand.nix

@@ -0,0 +1,13 @@
+{ lib, ... }: let
+  sub.options.config = lib.mkOption {
+    type = lib.types.bool;
+    default = false;
+  };
+in {
+  options.submodule = lib.mkOption {
+    type = lib.types.submoduleWith {
+      modules = [ sub ];
+    };
+    default = {};
+  };
+}

lib/tests/modules/declare-submoduleWith-path.nix

@@ -0,0 +1,12 @@
+{ lib, ... }: {
+  options.submodule = lib.mkOption {
+    type = lib.types.submoduleWith {
+      modules = [
+        ./declare-enable.nix
+      ];
+    };
+    default = {};
+  };
+
+  config.submodule = ./define-enable.nix;
+}

lib/tests/modules/declare-submoduleWith-shorthand.nix

@@ -0,0 +1,14 @@
+{ lib, ... }: let
+  sub.options.config = lib.mkOption {
+    type = lib.types.bool;
+    default = false;
+  };
+in {
+  options.submodule = lib.mkOption {
+    type = lib.types.submoduleWith {
+      modules = [ sub ];
+      shorthandOnlyDefinesConfig = true;
+    };
+    default = {};
+  };
+}

lib/tests/modules/declare-submoduleWith-special.nix

@@ -0,0 +1,17 @@
+{ lib, ... }: {
+  options.submodule = lib.mkOption {
+    type = lib.types.submoduleWith {
+      modules = [
+        ({ lib, ... }: {
+          options.foo = lib.mkOption {
+            default = lib.foo;
+          };
+        })
+      ];
+      specialArgs.lib = lib // {
+        foo = "foo";
+      };
+    };
+    default = {};
+  };
+}

lib/tests/modules/define-submoduleWith-noshorthand.nix

@@ -0,0 +1,3 @@
+{
+  submodule.config.config = true;
+}

lib/tests/modules/define-submoduleWith-shorthand.nix

@@ -0,0 +1,3 @@
+{
+  submodule.config = true;
+}

lib/types.nix

@@ -358,25 +358,43 @@ rec {
     };
 
     # A submodule (like typed attribute set). See NixOS manual.
-    submodule = opts:
+    submodule = modules: submoduleWith {
+      shorthandOnlyDefinesConfig = true;
+      modules = toList modules;
+    };
+
+    submoduleWith =
+      { modules
+      , specialArgs ? {}
+      , shorthandOnlyDefinesConfig ? false
+      }@attrs:
       let
-        opts' = toList opts;
         inherit (lib.modules) evalModules;
+
+        coerce = unify: value: if isFunction value
+          then setFunctionArgs (args: unify (value args)) (functionArgs value)
+          else unify (if shorthandOnlyDefinesConfig then { config = value; } else value);
+
+        allModules = defs: modules ++ imap1 (n: { value, file }:
+          if isAttrs value || isFunction value then
+            # Annotate the value with the location of its definition for better error messages
+            coerce (lib.modules.unifyModuleSyntax file "${toString file}-${toString n}") value
+          else value
+        ) defs;
+
       in
       mkOptionType rec {
         name = "submodule";
-        check = x: isAttrs x || isFunction x;
+        check = x: isAttrs x || isFunction x || path.check x;
         merge = loc: defs:
-          let
-            coerce = def: if isFunction def then def else { config = def; };
-            modules = opts' ++ map (def: { _file = def.file; imports = [(coerce def.value)]; }) defs;
-          in (evalModules {
-            inherit modules;
+          (evalModules {
+            modules = allModules defs;
+            inherit specialArgs;
             args.name = last loc;
             prefix = loc;
           }).config;
         getSubOptions = prefix: (evalModules
-          { modules = opts'; inherit prefix;
+          { inherit modules prefix specialArgs;
             # This is a work-around due to the fact that some sub-modules,
             # such as the one included in an attribute set, expects a "args"
             # attribute to be given to the sub-module. As the option
@@ -394,13 +412,29 @@ rec {
             # It shouldn't cause an issue since this is cosmetic for the manual.
             args.name = "‹name›";
           }).options;
-        getSubModules = opts';
-        substSubModules = m: submodule m;
-        functor = (defaultFunctor name) // {
-          # Merging of submodules is done as part of mergeOptionDecls, as we have to annotate
-          # each submodule with its location.
-          payload = [];
-          binOp = lhs: rhs: [];
+        getSubModules = modules;
+        substSubModules = m: submoduleWith (attrs // {
+          modules = m;
+        });
+        functor = defaultFunctor name // {
+          type = types.submoduleWith;
+          payload = {
+            modules = modules;
+            specialArgs = specialArgs;
+            shorthandOnlyDefinesConfig = shorthandOnlyDefinesConfig;
+          };
+          binOp = lhs: rhs: {
+            modules = lhs.modules ++ rhs.modules;
+            specialArgs =
+              let intersecting = builtins.intersectAttrs lhs.specialArgs rhs.specialArgs;
+              in if intersecting == {}
+              then lhs.specialArgs // rhs.specialArgs
+              else throw "A submoduleWith option is declared multiple times with the same specialArgs \"${toString (attrNames intersecting)}\"";
+            shorthandOnlyDefinesConfig =
+              if lhs.shorthandOnlyDefinesConfig == rhs.shorthandOnlyDefinesConfig
+              then lhs.shorthandOnlyDefinesConfig
+              else throw "A submoduleWith option is declared multiple times with conflicting shorthandOnlyDefinesConfig values";
+          };
         };
       };