I wonder if this changes #70638

@domenkozar Very relevant issue indeed! While this PR doesn't change that behavior, it does allow you to turn it off for specific submodules by replacing types.submodule with types.fullSubmodule {} (it uses configDefault = false by default).

This configDefault behavior transforms all submodule = value assignments (with a non-function value) to submodule.config = value. This is necessary (unfortunately) because there exist many options named config: If this wasn't done you'd have to do submodule.config.config = value to set a config option in the submodule.

Some more documentation will be helpful.

This is nice. Finally a coment left by @nbp makes sense.

Here I've tried to apply this fullSubmodule to containers configs (an excerpt):

diff --git a/nixos/modules/virtualisation/container-config.nix b/nixos/modules/virtualisation/container-config.nix
index f7a37d8c9f3..98a2091aee1 100644
--- a/nixos/modules/virtualisation/container-config.nix
+++ b/nixos/modules/virtualisation/container-config.nix
@@ -3,6 +3,14 @@
 with lib;
 
 {
+  options.boot.isContainer = mkOption {
+    type = types.bool;
+    default = false;
+    description = ''
+      Whether this NixOS machine is a lightweight container running
+      in another NixOS system.
+    '';
+  };
 
   config = mkIf config.boot.isContainer {
 
diff --git a/nixos/modules/virtualisation/containers.nix b/nixos/modules/virtualisation/containers.nix
index 09678ce9ea7..a07fb71acc7 100644
--- a/nixos/modules/virtualisation/containers.nix
+++ b/nixos/modules/virtualisation/containers.nix
@@ -1,4 +1,4 @@
-{ config, lib, pkgs, ... }:
+{ config, lib, pkgs, system, baseModules, ... }:
 
 with lib;
 
@@ -441,15 +441,6 @@ in
 {
   options = {
 
-    boot.isContainer = mkOption {
-      type = types.bool;
-      default = false;
-      description = ''
-        Whether this NixOS machine is a lightweight container running
-        in another NixOS system.
-      '';
-    };
-
     boot.enableContainers = mkOption {
       type = types.bool;
       default = !config.boot.isContainer;
@@ -458,6 +449,31 @@ in
       '';
     };
 
+    containers2 = let
+      m1 = { config, name, lib, ... }: {
+        disabledModules = [ "virtualisation/containers.nix" ];
+        options.autoStart = mkOption {
+          type = types.bool;
+          default = false;
+          description = ''
+            Whether the container is automatically started at boot-time.
+          '';
+        };
+        config = {
+          _module.args.baseModules = baseModules;
+          boot.isContainer = true;
+          networking.hostName = mkDefault name;
+          networking.useDHCP = false;
+          nixpkgs.system = system;
+        };
+      };
+    in mkOption {
+      type = types.attrsOf (types.fullSubmodule {
+        modules = [ m1 ] ++ baseModules;
+        specialArgs.modulesPath = builtins.toString ../.;
+      });
+    };
+
     containers = mkOption {
       type = types.attrsOf (types.submodule (
         { config, options, name, ... }:

I've also made the needed adjustments to containers API, see https://github.com/danbst/nixpkgs/tree/containers-full-submodules for full result. It is indeed much cleaner now.

As for performance, everything is not better than before. I've collected stats, and found out that instantiation is worse than before:

{ lib, ... }: {
    boot.loader.grub.devices = ["/dev/sdv"];
    fileSystems."/".device = "nodev";

    # generating identical containers with names c1, c2, c3...

    # for new containers API
    containers = lib.genAttrs (map (num: "c${toString num}") (lib.range 1 0)) (x: { });
    # for old containers API
    #containers = lib.genAttrs (map (num: "c${toString num}") (lib.range 1 0)) (x: { config = {}; });

    # STATS
    # old approach
    # containers  | time (sec)
    #      0      | 2.9
    #      1      | 6.5
    #      2      | 10.0
    #      5      | 19.7
    #     10      | 39.6
    #     20      | 73.7
    # Estimated formula: 2.9+3.6*n

    # new approach
    # containers  | time (sec)
    #      0      | 7.7
    #      1      | 15.8
    #      2      | 23.5
    #      5      | 47.7
    #     10      | 91.1
    # Estimated formula: 7.7+8.1*n
}

Maybe I made a mistake somewhere?

@danbst After investigating, it seems to have to do with the manual. Before your change with defaulting documentation.enable to false and a single container:

$ NIX_SHOW_STATS=1 nix-instantiate nixos --arg configuration ./config.nix -A config.system.build.toplevel --show-trace
{
  "cpuTime": 1.46572,
  [..]
  "gc": {
    "heapSize": 402976768,
    "totalBytes": 516986768
  }
}

After your change also with documentation.enable defaulting to false:

{
  "cpuTime": 1.45586,
  [..]
  "gc": {
    "heapSize": 402976768,
    "totalBytes": 520466960
  }
}

Patch to disable documentation:

diff --git a/nixos/modules/misc/documentation.nix b/nixos/modules/misc/documentation.nix
index deecb005270..28f686e2ad8 100644
--- a/nixos/modules/misc/documentation.nix
+++ b/nixos/modules/misc/documentation.nix
@@ -74,7 +74,7 @@ in
 
       enable = mkOption {
         type = types.bool;
-        default = true;
+        default = false;
         description = ''
           Whether to install documentation of packages from
           <option>environment.systemPackages</option> into the generated system path.

Also, trying to actually build the manual with the changes, I get

output '/nix/store/a2i90migwip9qia6wwz8sc9knxz43s3y-nixos-manual-html' is not allowed to refer to the following paths:
  /nix/store/0ds9dxm8gprzhfd8dnffh04iky8d03qy-openresolv-3.9.2
  /nix/store/0m71zgh745giq1awy8p442kvkjyhrqs5-xlockmore-5.59
  /nix/store/2xdfs6ia4jxf7kpx79hzplmm7v35aisk-iputils-20190709
  /nix/store/3agbpqiwbxdpi4apmf7pb976cvy3paz7-libnotify-0.7.8
  [..]

This probably comes from the fact that the pkgs scrubbing in

@infinisil
yeah, with documentation disabled (both for system and containers) I get now instantiate time formula 1.2+1.2*n.

However it is still of same performance as previous approach. It uses same amount of memory. I doubt now if it had previously used O(n²) time to instantiate n machines. From my experiments it seems to be linear, with both approaches.

Though it is much cleaner API (for containers example), which is nice!

@danbst Ah yeah, the O(n^2) would only be with n interdependent machines in nixops

@danbst Looking it into your container rewrite a bit more, I figured out some problems:

• All NixOS options would get duplicated under containers.*, which is no good. So there should be a way to turn off submodule options if desired, which the new commit 571c7da allows for
• But there are some container specific options such as enableTun, which should not be hidden. So your rewrite of the container module should still keep containers.*.config as the NixOS submodule (for which we can disable sub options completely), but keep the options containers.*.enableTun just like before.

You can then do something like this to get it to work with the same performance as before (the first xml docs patch shouldn't be necessary anymore once the second point above is done):

diff --git a/nixos/doc/manual/administration/declarative-containers.xml b/nixos/doc/manual/administration/declarative-containers.xml
index d03dbc4d705..8cff2c97c3a 100644
--- a/nixos/doc/manual/administration/declarative-containers.xml
+++ b/nixos/doc/manual/administration/declarative-containers.xml
@@ -33,9 +33,9 @@ containers.database =
   follows:
 <programlisting>
 containers.database = {
-  <link linkend="opt-containers._name_.privateNetwork">privateNetwork</link> = true;
-  <link linkend="opt-containers._name_.hostAddress">hostAddress</link> = "192.168.100.10";
-  <link linkend="opt-containers._name_.localAddress">localAddress</link> = "192.168.100.11";
+  privateNetwork = true;
+  hostAddress = "192.168.100.10";
+  localAddress = "192.168.100.11";
 };
 </programlisting>
   This gives the container a private virtual Ethernet interface with IP address
diff --git a/nixos/modules/misc/documentation.nix b/nixos/modules/misc/documentation.nix
index deecb005270..8445b75f6bb 100644
--- a/nixos/modules/misc/documentation.nix
+++ b/nixos/modules/misc/documentation.nix
@@ -20,7 +20,10 @@ let
     options =
       let
         scrubbedEval = evalModules {
-          modules = [ { nixpkgs.localSystem = config.nixpkgs.localSystem; } ] ++ manualModules;
+          modules = [ {
+            nixpkgs.localSystem = config.nixpkgs.localSystem;
+            documentation.isDocumentationEval = true;
+          } ] ++ manualModules;
           args = (config._module.args) // { modules = [ ]; };
           specialArgs = { pkgs = scrubDerivations "pkgs" pkgs; };
         };
@@ -84,6 +87,15 @@ in
         # which is at ../../../doc/multiple-output.xml
       };
 
+      isDocumentationEval = mkOption {
+        type = types.bool;
+        default = false;
+        internal = true;
+        description = ''
+          Whether this module system evaluation is purely to get an options listing
+        '';
+      };
+
       man.enable = mkOption {
         type = types.bool;
         default = true;
diff --git a/nixos/modules/virtualisation/containers.nix b/nixos/modules/virtualisation/containers.nix
index bbeebdb9db8..8a755e872ff 100644
--- a/nixos/modules/virtualisation/containers.nix
+++ b/nixos/modules/virtualisation/containers.nix
@@ -635,6 +635,7 @@ in
       type = types.attrsOf (types.fullSubmodule {
         modules = [ containerDefaults containerParamsModule ] ++ baseModules;
         specialArgs.modulesPath = builtins.toString ../.;
+        hideSubOptions = config.documentation.isDocumentationEval;
       });
       default = {};
       example = literalExample

Edit: Instead of 571c7da, it would also be possible to inline this type modification into the container module, but that gets ugly because of substSubModules. So I think an attribute like hideSubOptions to do that in fullSubmodule is reasonable

@infinisil what about this danbst@cd74159 ? This solves the containers options problems. I have just verified that generated manual includes only extra options.

BTW, I've just recalled that submodule types are mergeable. #24653 (comment) for insights. I wonder if fullSubmodule has same properties (specifically talking about those extra params for fullSubmodule args).

@danbst I think separating the container options from NixOS options is cleaner, because they're like on a higher level than just a standard NixOS configuration. Similarly I'd like for nixops to not use deployment.* NixOS options for what really are nixops options. Also, doing it this way makes backwards compatibility easier.

Pushed code with @roberth's suggestions and rebased for a clean history. I'll do some final tests with this and if it looks good to both of you too, this should be ready to merge.

Great work!
Also don't forget to document lib/types: Allow paths as submodule values

Changes:

• Added a bunch of tests for submoduleWith
• Added a small mention in the docs that paths can be used as submodule values
• Added a small note to the coerce paths explaining why they're necessary

From my side this is ready to be merged now!

Ah yes that does work, confirmed with this:

{ lib, ... }: {

  options.test = lib.mkOption {
    type = lib.types.submoduleWith {
      modules = [{
        options.foo = lib.mkOption {};
      }];
    };
  };

  config = {
    test.options.bar = lib.mkOption {};

    documentation.nixos.includeAllModules = true;

    boot.loader.grub.device = "nodev";
    fileSystems."/".device = "test";
  };
}

$ cat $(nix-build nixos --arg configuration ./config.nix -A config.system.build.manual.optionsJSON)/share/doc/nixos/options.json | grep test.bar

But yeah the fact that modules defined in the config section can be overridden with priorities doesn't seem very good for this, so I think what I suggested earlier to hide the docs is still preferable, feels cleaner too. I can add a small mention in the docs saying "Note that only the options defined in the modules argument will be included in rendered documentation" though.

@roberth Done did that