nixos/zfs: still pulls in zfsStable when enableUnstable

[Atemu]

Describe the bug
A clear and concise description of what the bug is.

When you set boot.zfs.enableUnstable, zfsUnstable is used but zfsStable is still put into the system closure. This causes an abort when the kernel is marked as incompatible with zfsStable which is very confusing because it should only be using zfsUnstable.

This only happens on 20.09, not on master.

To Reproduce
Steps to reproduce the behavior:

1. Mark zfsStable as incompatible with your kernel version
2. Set boot.zfs.enableUnstable
3. Build system closure

Expected behavior
A clear and concise description of what you expected to happen.

Screenshots
If applicable, add screenshots to help explain your problem.

Additional context
Add any other context about the problem here.

#108944

Notify maintainers

@Mic92 @NeQuissimus

Metadata
Please run nix-shell -p nix-info --run "nix-info -m" and paste the result.

Maintainer information:

# a list of nixpkgs attributes affected by the problem
attribute:
# a list of nixos modules affected by the problem
module:

[Atemu]

The old version of ZFS seems to be drawn into the closure by libvirt, Docker and GRUB on my machine.

[Atemu]

Those are just the pkg versions though, not the modules we care about. The old modules are not actually in the drv but they're still being evaluated in boot.extraModulePackages for some odd reason.

If you remove the throw from pkgs/os-specific/linux/zfs/default.nix, the package now evals to a string when incompatible which obviously breaks regular use of the package but in our bugged case, zfsStable (=string) can now be evaluated (doesn't matter that it's not a drv) and then zfsUnstable is put in boot.extraModulePackages.
Is there a rogue seq somewhere perhaps?

[Mic92]

This could be solved by having an read-only option set by the zfs module similar to config.systemd.package and having this option beeing used in other modules i.e. libvirt and docker.

[Atemu]

As I said, those are the ZFS packages, not modules. They're independent of the kernel.

We should still have a look into that as there might be incompatibilities in libvirt and Docker when they're compiled against a different ZFS version from the kernel module that's running in the system but that's a separate issue.

The actual problem seems to be that zfsStable is somehow evaluated in boot.extraModulePackages but not actually part of that list in the end (zfsUnstable is).

[NeQuissimus]

@Atemu @Mic92

If you look at the definition of modules in the kernel packages, you have zfs = zfsStable (all-packages.nix).
Also, the actual zfs package is set to zfsStable, which will be pulled into various derivations (such as grub).
If you issue the test with --show-trace, you'll see that boot.kernelPackages.kernel is evaluated via system.build.toplevel and pulls in zfsStable regardless of enableUnstable.
So I went and looked at the linuxPackages again.
Right before zfs = zfsStable you find

    inherit (callPackages ../os-specific/linux/zfs {
      configFile = "kernel";
      inherit kernel;
     }) zfsStable zfsUnstable;

zfsStable and zfsUnstable are both evaluated because of this!

So I went and removed zfsStable there and exchanged the other two references to zfsStable with zfsUnstable and 🎉 , the test runs.
However, with all these changes, I have effectively gotten rid of zfsStable altogether, so the solution is not feasible.
But I think we can work with it from here...
I think the problem is that ZFS needs the kernel version and the kernel modules need ZFS. So this is almost like a chicken & egg problem that we need to untangle.

This is my diff:

diff --git a/pkgs/top-level/all-packages.nix b/pkgs/top-level/all-packages.nix
index b5344a45157..d7fdde695cb 100644
--- a/pkgs/top-level/all-packages.nix
+++ b/pkgs/top-level/all-packages.nix
@@ -17913,9 +17913,10 @@ in
     inherit (callPackages ../os-specific/linux/zfs {
       configFile = "kernel";
       inherit kernel;
-     }) zfsStable zfsUnstable;
+     }) zfsUnstable;

-     zfs = zfsStable;
+
+     zfs = zfsUnstable;

      can-isotp = callPackage ../os-specific/linux/can-isotp { };
   });
@@ -18604,7 +18605,7 @@ in
     configFile = "user";
   }) zfsStable zfsUnstable;

-  zfs = zfsStable;
+  zfs = zfsUnstable;

   ### DATA

Opinions?

[Atemu]

What do you think of that ^

[Mic92]

For some packages we could remove the zfs dependency in the package and only add it at runtime.

[Atemu]

See above issue for that discussion, this is a separate problem.

[stale]

I marked this as stale due to inactivity. → More info