@grahamc, thanks for your PR! By analyzing the annotation information on this pull request, we identified @bennofs, @ocharles and @wkennington to be potential reviewers

Originally introduced here: 29caa18 and backported here: 6a12ff4

Seems reasonable to me, but would also recommand trying to understand why the code that defines services.xserver.desktopManager.default in nixos/modules/services/x11/desktop-managers/default.nix doesn't populate it for you.

Isn't the condition always true?

(wm != "" || wm != "none")

Heh, yeah, I probably meant && didn't I? :)

@vcunat fixed.

Another way to solve this is to set xterm to false:

{
  services.xserver.desktopManager.xterm.enable = false;
}

is this intended behavior, and the behavior from 16.03 / 15.09 is a bug? or is there a bug in that i3 should set xterm to false?

Upgrading to a blocker to get some eyes. Without some sort of fix, users of something like i3 and what-not are going to upgrade to 16.09 and have only xterm running. I don't mind what the decision is on how to resolve this but I don't think it is wrong to let this get released without either a release note or a code change.

I think your change made it work just by accident due to the always-true bug: in reality the computed value of services.xserver.desktopManager.default is actually none (note the apply logic on that option definition which acts on the default ""), which makes defaultSessionName = "xterm + none", which then makes (elem defaultSessionName dmcfg.session.names) false, which leads to user-session setting not being added to the config file at all.

But regardless, I think we should keep the obvious thing (services.xserver.windowManager.i3.enable = true;) doing the right thing. Maybe the safest thing would be to totally revert 29caa18 for 16.09 and then properly fix the wm/dm mess in unstable.

Presumably reverting wouldn't fix other display managers, and would break what the commit originally fixed. What's wrong with having i3 disable xterm, or set itself as the default?

I agree @dezgeg, @obadz -- I don't think this is the right solution. I'm beginning to think the right solution is to have a release note -- there are a lot of WMs to fix: https://github.com/NixOS/nixpkgs/tree/master/nixos/modules/services/x11/window-managers and I don't think this just impacts i3.

I was thinking it'd be better to disable xterm by default, and only enable it if the none WM is used, but none has no enabled state, so no place to enable xterm I think. I think the best choice is to revert this change only in 16.09 and work on a better solution for 17.03.

@obadz I would rather keep the commit and add fixes for all the WMs, but it is many WMs and more than I can realistically test changes to before 16.09 is released. I'm going to revert the change now, and I think if we can come up with a fix which doesn't impact existing workflows we'll backport.

The problem here is actually that windowManager.default is none if you don't set it explicitly. If you set windowManager.default = "i3" everything works as expected: i3 and xterm are both started.

I don't know how this used to work in the past. I couldn't find any code that automatically sets windowManager.default based on the window managers that are enabled.

How it "works" is that usually your window manager comes alphabetically before xterm, as in "i3 + none" < '"xterm".

@dezgeg oh, and the default for lightdm is to use the alphabetical lowest session? That makes sense! But I think we shouldn't rely on that, and instead fix windowManager.default.

I think we could solve this the same way that the conflict is solved in desktopManager.default: make it dependent on the order of imports to give i3 and other window managers priority over none if they are enabled.

Agree with @bennofs that you can fix the issue in your config by setting the default you want. However, if you revert, there is nothing you can do to set the WM you want.

As a workaround, after the revert services.xserver.displayManager.lightdm.extraSeatDefaults = "user-session = whatever-you-want"; should work.

So it turns out enabling services.accounts-daemon.enable = true; lets lightdm save which session each user has previously selected. We should probably enable that by default with ligthdm.

Thoughts?

@grahamc so this was reverted in 16.09 and master? So we can close and reopen the original issue?

@domenkozar: yes, it was reverted in both.

25be65f which I've put in master, but not release-16.09, fixes 99.9% of the issue for me. I suspect if nobody complains in the next couple of weeks, it should be safe to cherry-pick to stable, but up to you…

Looking forward to meet you tomorrow!

We should probably also consider cherry-picking cdaf55a ef35c66 8a547ae 1529641 1d8e7d1 755be7e and ff81b74