Merge pull request #238136 from hercules-ci/nixos-nixpkgs-dont-check-when-_module.args.pkgs-is-set

`nixos/nixpkgs`: Don't check when `_module.args.pkgs` is set

lib/modules.nix

@@ -904,6 +904,40 @@ let
     else opt // { type = opt.type.substSubModules opt.options; options = []; };
 
 
+  /*
+    Merge an option's definitions in a way that preserves the priority of the
+    individual attributes in the option value.
+
+    This does not account for all option semantics, such as readOnly.
+
+    Type:
+      option -> attrsOf { highestPrio, value }
+  */
+  mergeAttrDefinitionsWithPrio = opt:
+        let
+            defsByAttr =
+              lib.zipAttrs (
+                lib.concatLists (
+                  lib.concatMap
+                    ({ value, ... }@def:
+                      map
+                        (lib.mapAttrsToList (k: value: { ${k} = def // { inherit value; }; }))
+                        (pushDownProperties value)
+                    )
+                    opt.definitionsWithLocations
+                )
+              );
+        in
+          assert opt.type.name == "attrsOf" || opt.type.name == "lazyAttrsOf";
+          lib.mapAttrs
+                (k: v:
+                  let merging = lib.mergeDefinitions (opt.loc ++ [k]) opt.type.nestedTypes.elemType v;
+                  in {
+                    value = merging.mergedValue;
+                    inherit (merging.defsFinal') highestPrio;
+                  })
+                defsByAttr;
+
   /* Properties. */
 
   mkIf = condition: content:
@@ -1245,6 +1279,7 @@ private //
     importJSON
     importTOML
     mergeDefinitions
+    mergeAttrDefinitionsWithPrio
     mergeOptionDecls  # should be private?
     mkAfter
     mkAliasAndWrapDefinitions

lib/tests/modules.sh

@@ -61,6 +61,8 @@ checkConfigError() {
 # Shorthand meta attribute does not duplicate the config
 checkConfigOutput '^"one two"$' config.result ./shorthand-meta.nix
 
+checkConfigOutput '^true$' config.result ./test-mergeAttrDefinitionsWithPrio.nix
+
 # Check boolean option.
 checkConfigOutput '^false$' config.enable ./declare-enable.nix
 checkConfigError 'The option .* does not exist. Definition values:\n\s*- In .*: true' config.enable ./define-enable.nix

lib/tests/modules/test-mergeAttrDefinitionsWithPrio.nix

@@ -0,0 +1,21 @@
+{ lib, options, ... }:
+
+let
+  defs = lib.modules.mergeAttrDefinitionsWithPrio options._module.args;
+  assertLazy = pos: throw "${pos.file}:${toString pos.line}:${toString pos.column}: The test must not evaluate this the assertLazy thunk, but it did. Unexpected strictness leads to unexpected errors and performance problems.";
+in
+
+{
+  options.result = lib.mkOption { };
+  config._module.args = {
+    default = lib.mkDefault (assertLazy __curPos);
+    regular = null;
+    force = lib.mkForce (assertLazy __curPos);
+    unused = assertLazy __curPos;
+  };
+  config.result =
+    assert defs.default.highestPrio == (lib.mkDefault (assertLazy __curPos)).priority;
+    assert defs.regular.highestPrio == lib.modules.defaultOverridePriority;
+    assert defs.force.highestPrio == (lib.mkForce (assertLazy __curPos)).priority;
+    true;
+}

nixos/modules/misc/nixpkgs.nix

@@ -55,11 +55,6 @@ let
     description = "An evaluation of Nixpkgs; the top level attribute set of packages";
   };
 
-  # Whether `pkgs` was constructed by this module - not if nixpkgs.pkgs or
-  # _module.args.pkgs is set. However, determining whether _module.args.pkgs
-  # is defined elsewhere does not seem feasible.
-  constructedByMe = !opt.pkgs.isDefined;
-
   hasBuildPlatform = opt.buildPlatform.highestPrio < (mkOptionDefault {}).priority;
   hasHostPlatform = opt.hostPlatform.isDefined;
   hasPlatform = hasHostPlatform || hasBuildPlatform;
@@ -337,10 +332,28 @@ in
 
   config = {
     _module.args = {
-      pkgs = finalPkgs.__splicedPackages;
+      pkgs =
+        # We explicitly set the default override priority, so that we do not need
+        # to evaluate finalPkgs in case an override is placed on `_module.args.pkgs`.
+        # After all, to determine a definition priority, we need to evaluate `._type`,
+        # which is somewhat costly for Nixpkgs. With an explicit priority, we only
+        # evaluate the wrapper to find out that the priority is lower, and then we
+        # don't need to evaluate `finalPkgs`.
+        lib.mkOverride lib.modules.defaultOverridePriority
+          finalPkgs.__splicedPackages;
     };
 
-    assertions = [
+    assertions = let
+      # Whether `pkgs` was constructed by this module. This is false when any of
+      # nixpkgs.pkgs or _module.args.pkgs is set.
+      constructedByMe =
+        # We set it with default priority and it can not be merged, so if the
+        # pkgs module argument has that priority, it's from us.
+        (lib.modules.mergeAttrDefinitionsWithPrio options._module.args).pkgs.highestPrio
+          == lib.modules.defaultOverridePriority
+        # Although, if nixpkgs.pkgs is set, we did forward it, but we did not construct it.
+          && !opt.pkgs.isDefined;
+    in [
       (
         let
           nixosExpectedSystem =