Merge pull request #106767 from erikarvstedt/fix-container-pkgs-2

nixos-container: fix `nixpkgs` container options being ignored

2 files changed, 57 insertions, 48 deletions

diff --git a/nixos/modules/virtualisation/nixos-containers.nix b/nixos/modules/virtualisation/nixos-containers.nix
index 7bec1b1ff26e8..f06977f88fc16 100644
--- a/nixos/modules/virtualisation/nixos-containers.nix
+++ b/nixos/modules/virtualisation/nixos-containers.nix
@@ -463,21 +463,15 @@ in
         { config, options, name, ... }:
         {
           options = {
-
             config = mkOption {
               description = ''
                 A specification of the desired configuration of this
                 container, as a NixOS module.
               '';
-              type = let
-                confPkgs = if config.pkgs == null then pkgs else config.pkgs;
-              in lib.mkOptionType {
+              type = lib.mkOptionType {
                 name = "Toplevel NixOS config";
-                merge = loc: defs: (import (confPkgs.path + "/nixos/lib/eval-config.nix") {
+                merge = loc: defs: (import "${toString config.nixpkgs}/nixos/lib/eval-config.nix" {
                   inherit system;
-                  pkgs = confPkgs;
-                  baseModules = import (confPkgs.path + "/nixos/modules/module-list.nix");
-                  inherit (confPkgs) lib;
                   modules =
                     let
                       extraConfig = {
@@ -526,12 +520,18 @@ in
               '';
             };
 
-            pkgs = mkOption {
-              type = types.nullOr types.attrs;
-              default = null;
-              example = literalExample "pkgs";
+            nixpkgs = mkOption {
+              type = types.path;
+              default = pkgs.path;
+              defaultText = "pkgs.path";
               description = ''
-                Customise which nixpkgs to use for this container.
+                A path to the nixpkgs that provide the modules, pkgs and lib for evaluating the container.
+
+                To only change the <literal>pkgs</literal> argument used inside the container modules,
+                set the <literal>nixpkgs.*</literal> options in the container <option>config</option>.
+                Setting <literal>config.nixpkgs.pkgs = pkgs</literal> speeds up the container evaluation
+                by reusing the system pkgs, but the <literal>nixpkgs.config</literal> option in the
+                container config is ignored in this case.
               '';
             };
 
@@ -672,14 +672,31 @@ in
               '';
             };
 
+            # Removed option. See `checkAssertion` below for the accompanying error message.
+            pkgs = mkOption { visible = false; };
           } // networkOptions;
 
-          config = mkMerge
-            [
-              (mkIf options.config.isDefined {
-                path = config.config.system.build.toplevel;
-              })
-            ];
+          config = let
+            # Throw an error when removed option `pkgs` is used.
+            # Because this is a submodule we cannot use `mkRemovedOptionModule` or option `assertions`.
+            optionPath = "containers.${name}.pkgs";
+            files = showFiles options.pkgs.files;
+            checkAssertion = if options.pkgs.isDefined then throw ''
+              The option definition `${optionPath}' in ${files} no longer has any effect; please remove it.
+
+              Alternatively, you can use the following options:
+              - containers.${name}.nixpkgs
+                This sets the nixpkgs (and thereby the modules, pkgs and lib) that
+                are used for evaluating the container.
+
+              - containers.${name}.config.nixpkgs.pkgs
+                This only sets the `pkgs` argument used inside the container modules.
+            ''
+            else null;
+          in {
+            path = builtins.seq checkAssertion
+              mkIf options.config.isDefined config.config.system.build.toplevel;
+          };
         }));
 
       default = {};
diff --git a/nixos/tests/containers-custom-pkgs.nix b/nixos/tests/containers-custom-pkgs.nix
index 397a4a905e6d9..1412c32bfb5f1 100644
--- a/nixos/tests/containers-custom-pkgs.nix
+++ b/nixos/tests/containers-custom-pkgs.nix
@@ -1,42 +1,34 @@
-# Test for NixOS' container support.
-
 import ./make-test-python.nix ({ pkgs, lib, ...} : let
 
-  customPkgs = pkgs // {
-    hello = pkgs.hello.overrideAttrs(old: {
-      name = "custom-hello";
+  customPkgs = pkgs.appendOverlays [ (self: super: {
+    hello = super.hello.overrideAttrs (old: {
+       name = "custom-hello";
     });
-  };
+  }) ];
 
 in {
-  name = "containers-hosts";
+  name = "containers-custom-pkgs";
   meta = with lib.maintainers; {
-    maintainers = [ adisbladis ];
+    maintainers = [ adisbladis earvstedt ];
   };
 
-  machine =
-    { ... }:
-    {
-      virtualisation.memorySize = 256;
-      virtualisation.vlans = [];
+  machine = { config, ... }: {
+    assertions = let
+      helloName = (builtins.head config.containers.test.config.system.extraDependencies).name;
+    in [ {
+      assertion = helloName == "custom-hello";
+      message = "Unexpected value: ${helloName}";
+    } ];
 
-      containers.simple = {
-        autoStart = true;
-        pkgs = customPkgs;
-        config = {pkgs, config, ... }: {
-          environment.systemPackages = [
-            pkgs.hello
-          ];
-        };
+    containers.test = {
+      autoStart = true;
+      config = { pkgs, config, ... }: {
+        nixpkgs.pkgs = customPkgs;
+        system.extraDependencies = [ pkgs.hello ];
       };
-
     };
+  };
 
-  testScript = ''
-    start_all()
-    machine.wait_for_unit("default.target")
-    machine.succeed(
-        "test $(nixos-container run simple -- readlink -f /run/current-system/sw/bin/hello) = ${customPkgs.hello}/bin/hello"
-    )
-  '';
+  # This test only consists of evaluating the test machine
+  testScript = "";
 })