From 74dbae8968f478438a3f1f677eb44db06b26b201 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Thu, 25 Apr 2024 16:33:28 +0200 Subject: [PATCH 1/3] kernel: Test laziness of certain arguments --- pkgs/os-specific/linux/kernel/generic.nix | 27 +++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/pkgs/os-specific/linux/kernel/generic.nix b/pkgs/os-specific/linux/kernel/generic.nix index 77c6ee031956d..579f1cd1f4f38 100644 --- a/pkgs/os-specific/linux/kernel/generic.nix +++ b/pkgs/os-specific/linux/kernel/generic.nix @@ -1,5 +1,6 @@ { buildPackages , callPackage +, emptyFile , perl , bison ? null , flex ? null @@ -15,6 +16,7 @@ , nixosTests }@args': +let overridableKernel = lib.makeOverridable ({ # The kernel source tarball. src @@ -241,7 +243,28 @@ kernel.overrideAttrs (finalAttrs: previousAttrs: { + toString (lib.attrNames (lib.toFunction args { })) ) overridableKernel; }; - in [ (nixosTests.kernel-generic.passthru.testsForKernel overridableKernel) ] ++ kernelTests; + 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 + ] ++ kernelTests; }; -})) +})); +in overridableKernel From 3e83fe9aa593bc514d948f83becd6e8e270fc774 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Thu, 25 Apr 2024 16:33:58 +0200 Subject: [PATCH 2/3] kernel: Make lazier (fix infinite recursion) --- pkgs/os-specific/linux/kernel/generic.nix | 10 +++++++--- pkgs/os-specific/linux/kernel/manual-config.nix | 9 ++++++++- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/pkgs/os-specific/linux/kernel/generic.nix b/pkgs/os-specific/linux/kernel/generic.nix index 579f1cd1f4f38..0e2d85a0971d1 100644 --- a/pkgs/os-specific/linux/kernel/generic.nix +++ b/pkgs/os-specific/linux/kernel/generic.nix @@ -213,11 +213,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: { 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; From debe527772a72ee4821063503f1dfa5893962087 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Thu, 25 Apr 2024 16:42:22 +0200 Subject: [PATCH 3/3] kernel: Add NixOS evaluation test --- pkgs/os-specific/linux/kernel/generic.nix | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/pkgs/os-specific/linux/kernel/generic.nix b/pkgs/os-specific/linux/kernel/generic.nix index 0e2d85a0971d1..631217735d8fa 100644 --- a/pkgs/os-specific/linux/kernel/generic.nix +++ b/pkgs/os-specific/linux/kernel/generic.nix @@ -1,6 +1,5 @@ { buildPackages , callPackage -, emptyFile , perl , bison ? null , flex ? null @@ -13,6 +12,9 @@ , rustc , rustPlatform , rust-bindgen +# testing +, emptyFile +, nixos , nixosTests }@args': @@ -247,6 +249,17 @@ kernel.overrideAttrs (finalAttrs: previousAttrs: { + toString (lib.attrNames (lib.toFunction args { })) ) overridableKernel; }; + /* 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 // ( @@ -267,6 +280,8 @@ kernel.overrideAttrs (finalAttrs: previousAttrs: { 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; };