[RFC] add ability to merge structured configs

[teto]

POC

Motivation for this change

Listed here #41103.
To sum up, I would like programs that require kernel modifications to be able to specify their kernel requirements. This might be disabled etc. but here is a proposition to show how it could look like

Things done

• Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
• Built on platform(s)
  • NixOS
  • macOS
  • other Linux distributions
• Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
• Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
• Tested execution of all binary files (usually in ./result/bin/)
• Determined the impact on package closure size (by running nix path-info -S before and after)
• Fits CONTRIBUTING.md.

---

follow up of #41393

[teto]

@dezgeg thanks for reviewing/merging the structured config PR \o/

Being able to specify required config for packages was one of the motivation. I would love some feedback on how to best achieve this since the current way is not practical aka:

openvswitch = stdenv.mkDerivation {
....
  passthru.kernelExtraConfig = with import ../../../../lib/kernel.nix { inherit (kernel) version; lib = stdenv.lib; }; {
    OPENVSWITCH  = whenAtLeast "2.0" yes;
  };
}

I moved the version dependant filters whenAtLeast etc.. out of lib/kernel.nix so that lib/kernel.nix can be imported by lib/default.nix. This replaces with import ../../../../lib/kernel.nix { inherit (kernel) version; lib = stdenv.lib; }; with with import lib.kernel; but I am still stuck with
passthru.kernelExtraConfig = { OPENVSWITCH = yes; }

The problem is that if we just pass it like this

  my_lenovo_kernel = prev.linux_latest.override({
    structuredExtraConfig = prev.pkgs.openvswitch.passthru.kernelExtraConfig;
})

it erases conditions (e.g., whenAtLeast ) in common-config.nix such as
OPENVSWITCH = optional whenAtLeast "3.4" yes;

I wonder if it wouldn't be better to have packages define a list of config items like passthru.kernelExtraConfig = [ OPENVSWITCH ];and have the kernel config logic enable these the common-config.nix (strip the optional for instance) ?

To sum up, the best interface would be IMO to have in common-config.nix
OPENVSWITCH = optional whenAtLeast "3.4" no;
and find some way to turn this into OPENVSWITCH = whenAtLeast "3.4" yes; when building my_lenovo_kernel (cf previous example).

[teto]

I am experimenting a bit as I am trying to solve the initial issue. Without types, the code is hard to follow but what I've come up with is something similar to the module system so that hopefully merging
{ DEBUG = option yes; } and { DEBUG = yes; } returns sthg like { DEBUG = option yes; }.
So for instance yes is defined as yes = { answer = "y"; }. My fear is that I am recreating the module system so any hindsight would be nice.

[Profpatsch]

Huh, I thought we merged that line already?

[teto]

it's not up to date. I've started modifying the code to adopt somewhat the module backend so it might be best not to review it now

[teto]

Thanks to the advice on https://discourse.nixos.org/t/use-lib-types-system-to-merge-attrsets-without-the-module-system/534/10, I managed to use the module system to merge structured configs. I renamed the pull request so that it focuses on this aspect only. It seems to work for basic things (linux_latest) but I am sure I've broken things in other places.

I tried to preserve the current api (option no; whenOlder "4.3" yes etc) but there are a few difficulties. Each kernel option is now a kernelItem submodule of the form https://github.com/NixOS/nixpkgs/pull/42838/files#diff-810dcf1e9edefca0d59fd0b90d11242bR30 .

One problem is for the "freeform" options like currently
MMC_BLOCK_MINORS = "32";
I could preprocess (slow) the structured config to convert the "32" into a submodule { answer = "32";}
so I preferred to write instead MMC_BLOCK_MINORS = freeform "32"; # equivalent to { answer = "32"} but I wonder if MMC_BLOCK_MINORS.answer = "32"; is better ?
It might be best (to allow for further checking) to distinguish between freeform answers (type = types.str) and tristate values (types.enum [ "y" "n" "m"])

I've converted the items of the form
B43_PCMCIA = option (whenOlder "4.4" yes);
to
B43_PCMCIA = whenOlder "4.4" (option yes);
with the whenOlder now being a mkIf. I suspect this can be a problem for some kernels. I will need to tackle that too.

[teto]

I went ahead and proceeded with the modifications:

• extraStructuredConfig now expects a list of attributes. It makes the composition of the kernel config easier.
• I've proceeded with distinguishing freeform options from tristate ones
• will look for a structured config in kernelPatches too
• one can now access the structuredConfig from a kernel via linux_test.configfile.structuredConfig in order to reinject it another kernel, no need to rewrite the config from scratch
• I've defined the following merge strategies in case of conflict:
  -- freeform items must be equal or they conflict (mergeEqualOption)
  -- for tristate (y/m/n) entries, I use the mergeAnswer strategy which takes the best available value, "best" being defined by the user (by default "y" > "m" > "n", e.g. if one entry is both marked "y" and "n", "y" wins)
  -- if one item is both marked optional/mandatory, mandatory wins (mergeFalseByDefault)

Here is what I test with:

    linux_test = let 
      configStructured = with prev.lib.kernel; [ 


        # common-config.nix default is 32 

        { MMC_BLOCK_MINORS   = freeform "32"; } # same as default, won't trigger any error 
        # { MMC_BLOCK_MINORS   = freeform "64"; } # will trigger an error but the message is not great:
        # The option `settings.MMC_BLOCK_MINORS.freeform' has conflicting definitions, in `<unknown-file>' and `<unknown-file>'

        # mandatory should win by default
        { USB_DEBUG = option yes;}
        # { USB_DEBUG = yes;}

        # default for "8139TOO_PIO" is no
        { "8139TOO_PIO"  = yes; } # yes wins over no by default
      ];
    in
    prev.linux_latest.override {
      kernelPreferBuiltin = true;
      structuredExtraConfig = configStructured;
  };

You can then have a look at the generated config via
nix-build -A linux_latest.configfile ~/nixpkgs3 or look in nix repl for linux_test.configfile.structuredConfig

I am sure the code may be improved and there are some cases that might break. I would like to write some tests but not sure how to do.

[edolstra]

Can lib/kernel.nix be moved out of lib? lib really shouldn't contain package-specific functions.

[teto]

I did well to check several mistakes had crept in.
The diff between hardened kernels now returns nothing \o/ (I just checked for linux_hardened).
The lib/kernels.nix isn't made public anymore and is imported just by the files needing it.
To sum it up, the PR exposes nothing new (structuredExtraConfig is already available in nixos-unstable), it just adds an internal mechanism to merge kernel configurations.
Once this PR is merged, we need to think about what and how to expose kernel configuration to packages / user (boot.kernelConfiguration etc, might warrant an RFC).

[infinisil]

@teto Merge conflict

[teto]

@infinisil ty, pushed an update.

[infinisil]

Evaluation fails now, messed up in all-packages.nix

[dtzWill]

hype hype, thanks all! :D

…

On Sun, 27 Jan 2019 16:11:48 -0800, Matthieu Coudron ***@***.***> wrote: @infinisil ty, pushed an update. -- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: #42838 (comment) part: text/html

[teto]

@infinisil sorry, the lights were green after my last push, I guess some caching problem on the browser. The timeout for the first check seems to appear once the config was generated so I think it's fine.

@dtzWill very excited about this too. The current achievement is the possibility to do extraStructuredConfig=mkMerge [ ] and be warned in case of contradicting configurations. Yet it allows for some exciting future possibilities.

[lopsided98]

I think this broke the legacy extraConfig option, which is used by linux_hardkernel_4_14. The build is failing because a config option is not being applied. That package should probably be switched to structured config, but I don't think this PR intended to break string based config.

[teto]

you are right, it shouldn't sorry for that, let me look into this.

[teto]

@lopsided98 what kind of error do you have ? I have

make: Entering directory '/build/source'
make[1]: Entering directory '/build/source/build'
  HOSTCC  scripts/basic/fixdep
  GEN     ./Makefile
  HOSTCC  scripts/kconfig/conf.o
  SHIPPED scripts/kconfig/zconf.tab.c
  SHIPPED scripts/kconfig/zconf.lex.c
  HOSTCC  scripts/kconfig/zconf.tab.o
  HOSTLD  scripts/kconfig/conf
***
*** Can't find default configuration "arch/x86/configs/odroidxu4_defconfig"!
***

which shouldn't be influenceed by my changes. Also I can reproduce this error on nixos-unstable so it must be some other change
nix build -f channel:nixos-unstable linux_hardkernel_4_14

[lopsided98]

Your error is caused by building it on x86; odroidxu4_defconfig only works on ARM. You should be able to cross compile it with nix build -f channel:nixos-unstable pkgsCross.armv7l-hf-multiplatform.linux_hardkernel_4_14.

My error is:

<3>  CC [M]  drivers/gator/gator_events_mali_midgard.o
<3>../drivers/gator/gator_events_mali_midgard.c:25:10: fatal error: linux/mali_linux_trace.h: No such file or directory
<3> #include "linux/mali_linux_trace.h"
<3>          ^~~~~~~~~~~~~~~~~~~~~~~~~~
<3>compilation terminated.
<3>make[4]: *** [../scripts/Makefile.build:326: drivers/gator/gator_events_mali_midgard.o] Error 1
<3>make[3]: *** [../scripts/Makefile.build:585: drivers/gator] Error 2
<3>make[2]: *** [/build/source/Makefile:1044: drivers] Error 2
<3>make[2]: *** Waiting for unfinished jobs....

This error was fixed in 5ac7334 by adding GATOR n to extraConfig, so the return of this error implies that extraConfig is no longer being applied.

[teto]

seems like you are right, extraConfig is not taken into account.It's very late here so I can try tomorrow, until then does

diff --git a/pkgs/os-specific/linux/kernel/generic.nix b/pkgs/os-specific/linux/kernel/generic.nix
index a41f1eb989b..a83fbc89324 100644
--- a/pkgs/os-specific/linux/kernel/generic.nix
+++ b/pkgs/os-specific/linux/kernel/generic.nix
@@ -74,9 +74,9 @@ let
   };
 
   # extra config in legacy string format
-  extraConfig = extraConfig + lib.optionalString (stdenv.hostPlatform.platform ? kernelExtraConfig) stdenv.hostPlatform.platform.kernelExtraConfig;
+  extraStrConfig = extraConfig + lib.optionalString (stdenv.hostPlatform.platform ? kernelExtraConfig) stdenv.hostPlatform.platform.kernelExtraConfig;
 
-  intermediateNixConfig = configfile.moduleStructuredConfig.intermediateNixConfig;
+  intermediateNixConfig = configfile.moduleStructuredConfig.intermediateNixConfig + extraStrConfig;
 
   structuredConfigFromPatches =
         map ({extraStructuredConfig ? {}, ...}: {settings=extraStructuredConfig;}) kernelPatches;

solve it ?

[lopsided98]

error: while evaluating the attribute 'drvPath' at /home/ben/Documents/Projects/nixpkgs/lib/customisation.nix:149:7:
while evaluating the attribute 'configurePhase' of the derivation 'linux-4.14.87-153-armv7l-unknown-linux-gnueabihf' at /home/ben/Documents/Projects/nixpkgs/pkgs/stdenv/generic/make-derivation.nix:183:11:
while evaluating the attribute 'kernelConfig' of the derivation 'linux-config-4.14.87-153-armv7l-unknown-linux-gnueabihf' at /home/ben/Documents/Projects/nixpkgs/pkgs/stdenv/generic/make-derivation.nix:183:11:
while evaluating 'kernelConfigFun' at /home/ben/Documents/Projects/nixpkgs/pkgs/os-specific/linux/kernel/generic.nix:85:21, called from /home/ben/Documents/Projects/nixpkgs/pkgs/os-specific/linux/kernel/generic.nix:97:20:
infinite recursion encountered, at /home/ben/Documents/Projects/nixpkgs/pkgs/os-specific/linux/kernel/generic.nix:77:17

[teto]

(I've updated the snippet).

[lopsided98]

That patch fixes the issue.

[teto]

Elco didn t want this to leak. Instead of using the alias 'no' you can also pass the submodule directly: SND={answer="n";}