Nixpkgs: xserver.{desktopManager,windowManager}.default configuration breaks after upgrade to master

[veprbl]

Describe the bug
I have a NixOS installation with a very simple xserver configuration:

services.xserver.enable = true;
services.xserver.windowManager.default = "fluxbox"; 
services.xserver.windowManager.fluxbox.enable = true;

When trying to rebuild with master I get an error:

trace: Default graphical session, 'fluxbox', not found.
Valid names for 'services.xserver.displayManager.defaultSession' are:
  xterm
  xterm+fluxbox
  none+fluxbox

error: The option value `services.xserver.displayManager.defaultSession' in `/home/veprbl/nixpkgs/nixos/modules/services/x11/display-managers/default.nix' is not of type `session name'.

To Reproduce
Steps to reproduce the behavior:

1. Use my configuration.nix lines from the above
2. Do nixos-rebuild build -I nixpkgs=https://github.com/NixOS/nixpkgs/archive/e1ee43f514fa8b510d8d3d142dc1ae83dee7716d.tar.gz

Expected behavior
The old configuration should continue to work unless I opt into system.stateVersion = "20.03";

Metadata
Please run nix run nixpkgs.nix-info -c 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:
  - services.xserver

This has to do with #53843
cc @hedning @jtojnar

[hedning]

Right, looks like there's a bug when constructing the legacy the session name from the legacy defaults. Specifically when only supplying a default for the window manager. Will fix.

[jtojnar]

IMHO, stateVersion is only meant for mutable state like database schemata, that does not offer downgrade/rollback path. Abusing it for versioning option tree only adds complexity for little benefit, especially when we already employ other transition aids.

Nevertheless, this was an oversight on my part. Something like this might fix it:

--- a/nixos/modules/services/x11/display-managers/default.nix
+++ b/nixos/modules/services/x11/display-managers/default.nix
@@ -141,9 +141,11 @@ let
     '';

   dmDefault = cfg.desktopManager.default;
+  # fallback default for cases when only default wm is set
+  dmFallbackDefault = if dmDefault != null then dmDefault else "none";
   wmDefault = cfg.windowManager.default;

-  defaultSessionFromLegacyOptions = concatStringsSep "+" (filter (s: s != null) ([ dmDefault ] ++ optional (wmDefault != "none") wmDefault));
+  defaultSessionFromLegacyOptions = dmFallbackDefault + (optionalString (wmDefault != null && wmDefault != "none") "+${wmDefault}");

 in

@@ -358,7 +360,7 @@ in
             { c = wmDefault; t = "- services.xserver.windowManager.default"; }
             ]))}
           Please use
-            services.xserver.displayManager.defaultSession = "${concatStringsSep "+" (filter (s: s != null) [ dmDefault wmDefault ])}";
+            services.xserver.displayManager.defaultSession = "${concatStringsSep "+" (filter (s: s != null) [ dmFallbackDefault wmDefault ])}";
           instead.
         ''
       ];

[Radvendii]

What is the proper way to update my configuration to the new system? Where should I look for that information? Are there release notes? I couldn't figure it out from #53843

[jtojnar]

Yes, release notes are included in the pull request:

https://github.com/NixOS/nixpkgs/pull/53843/files#diff-9fb30f84bbdfa273940339fa157a012e

Also the module system should give you upgrade instruction when we fix the oversight. Could you test the patch above?

[veprbl]

@jtojnar Your patch fixes the issue for me.

[worldofpeace]

Closing since this is fixed.

[veprbl]

We still need to apply the patch to the nixpkgs.

[worldofpeace]

Oops, thought this was https://github.com/NixOS/nixpkgs/commit/3d70d4ba0b6be256974910e635fadcc0e9579b2a

[veprbl]

Oops, thought this was 3d70d4b

Yeah, I just looked up that one, but it actually predates me filing the issue and it's currently the latest commit for the file.