Merge pull request #306790 from roberth/fix-nixos-kernelPackages-kernel-version-kernelPatches-recursion

kernel: Fix infinite recursion between version and patches

2 files changed, 55 insertions, 6 deletions

diff --git a/pkgs/os-specific/linux/kernel/generic.nix b/pkgs/os-specific/linux/kernel/generic.nix
index 77c6ee031956d..631217735d8fa 100644
--- a/pkgs/os-specific/linux/kernel/generic.nix
+++ b/pkgs/os-specific/linux/kernel/generic.nix
@@ -12,9 +12,13 @@
 , rustc
 , rustPlatform
 , rust-bindgen
+# testing
+, emptyFile
+, nixos
 , nixosTests
 }@args':
 
+let overridableKernel =
 lib.makeOverridable ({ # The kernel source tarball.
   src
 
@@ -211,11 +215,15 @@ let
   }; # end of configfile derivation
 
   kernel = (callPackage ./manual-config.nix { inherit lib stdenv buildPackages; }) (basicArgs // {
-    inherit kernelPatches randstructSeed extraMakeFlags extraMeta configfile;
+    inherit kernelPatches randstructSeed extraMakeFlags extraMeta configfile modDirVersion;
     pos = builtins.unsafeGetAttrPos "version" args;
 
-    config = { CONFIG_MODULES = "y"; CONFIG_FW_LOADER = "m"; } // lib.optionalAttrs withRust { CONFIG_RUST = "y"; };
-  } // lib.optionalAttrs (modDirVersion != null) { inherit modDirVersion; });
+    config = {
+      CONFIG_MODULES = "y";
+      CONFIG_FW_LOADER = "m";
+      CONFIG_RUST = lib.mkIf withRust "y";
+    };
+  });
 
 in
 kernel.overrideAttrs (finalAttrs: previousAttrs: {
@@ -241,7 +249,41 @@ kernel.overrideAttrs (finalAttrs: previousAttrs: {
             + toString (lib.attrNames (lib.toFunction args { }))
           ) overridableKernel;
       };
-    in [ (nixosTests.kernel-generic.passthru.testsForKernel overridableKernel) ] ++ kernelTests;
+      /* Certain arguments must be evaluated lazily; so that only the output(s) depend on them.
+         Original reproducer / simplified use case:
+       */
+      versionDoesNotDependOnPatchesEtcNixOS =
+        builtins.seq
+          (nixos ({ config, pkgs, ... }: {
+              boot.kernelPatches = [
+                (builtins.seq config.boot.kernelPackages.kernel.version { patch = pkgs.emptyFile; })
+              ];
+          })).config.boot.kernelPackages.kernel.outPath
+          emptyFile;
+      versionDoesNotDependOnPatchesEtc =
+        builtins.seq
+          (import ./generic.nix args' (args // (
+          let explain = attrName:
+            ''
+              The ${attrName} attribute must be able to access the kernel.version attribute without an infinite recursion.
+              That means that the kernel attrset (attrNames) and the kernel.version attribute must not depend on the ${attrName} argument.
+              The fact that this exception is raised shows that such a dependency does exist.
+              This is a problem for the configurability of ${attrName} in version-aware logic such as that in NixOS.
+              Strictness can creep in through optional attributes, or assertions and warnings that run as part of code that shouldn't access what is checked.
+            '';
+          in {
+            kernelPatches = throw (explain "kernelPatches");
+            structuredExtraConfig = throw (explain "structuredExtraConfig");
+            modDirVersion = throw (explain "modDirVersion");
+          }))).version
+          emptyFile;
+    in [
+      (nixosTests.kernel-generic.passthru.testsForKernel overridableKernel)
+      versionDoesNotDependOnPatchesEtc
+      # Disabled by default, because the infinite recursion is hard to understand. The other test's error is better and produces a shorter trace.
+      # versionDoesNotDependOnPatchesEtcNixOS
+    ] ++ kernelTests;
   };
 
-}))
+}));
+in overridableKernel
diff --git a/pkgs/os-specific/linux/kernel/manual-config.nix b/pkgs/os-specific/linux/kernel/manual-config.nix
index 5b222c4b45eff..cab04ad0c7d80 100644
--- a/pkgs/os-specific/linux/kernel/manual-config.nix
+++ b/pkgs/os-specific/linux/kernel/manual-config.nix
@@ -26,7 +26,7 @@ in lib.makeOverridable ({
   extraMakeFlags ? [],
   # The name of the kernel module directory
   # Needs to be X.Y.Z[-extra], so pad with zeros if needed.
-  modDirVersion ? lib.versions.pad 3 version,
+  modDirVersion ? null /* derive from version */,
   # The kernel source (tarball, git checkout, etc.)
   src,
   # a list of { name=..., patch=..., extraConfig=...} patches
@@ -54,6 +54,13 @@ in lib.makeOverridable ({
 }:
 
 let
+  # Provide defaults. Note that we support `null` so that callers don't need to use optionalAttrs,
+  # which can lead to unnecessary strictness and infinite recursions.
+  modDirVersion_ = if modDirVersion == null then lib.versions.pad 3 version else modDirVersion;
+in
+let
+  # Shadow the un-defaulted parameter; don't want null.
+  modDirVersion = modDirVersion_;
   inherit (lib)
     hasAttr getAttr optional optionals optionalString optionalAttrs maintainers platforms;