-
-
Notifications
You must be signed in to change notification settings - Fork 15.5k
nixos/displayManager: introduce defaultSession #53843
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Should we perhaps deprecate |
Hmm, perhaps, but I'd imagine they're both used quite heavily. Also recreating the the |
We should probably add some documentation too. |
13c0b1f
to
a691313
Compare
e153a4b
to
6fea76e
Compare
Added |
6fea76e
to
de03a76
Compare
ping (triage) |
Last time I checked @hedning was rethinking this approach. |
de03a76
to
b0569f0
Compare
So it's possible to reuse So thinking on this some more I think it might be better to stick with an explicit So I guess the question is this: |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
a5b6217
to
ee1536f
Compare
Yeah, I noticed that too, though on unstable it retains lightdm background for me for some reason. |
ec20412
to
705d032
Compare
We want access to the valid session names at evaluation time.
There's two ways of providing graphical sessions now: - `displayManager.session` via. `desktopManager.session` and `windowManager.session` - `displayManager.sessionPackages` `sessionPackages` doesn't make a distinction between desktop and window managers. This makes selecting a session provided by a package using `desktopManager.default` nonsensical. We therefor introduce `displayManager.defaultSession` which can select a session from either `displayManager.session` or `displayManager.sessionPackages`. It will default to `desktopManager.default + windowManager.default` as before. If the dm default is "none" it will select the first provided session from `sessionPackages`.
Note: can't launch gnome on wayland due to duplicate entry names: canonical/lightdm#16
The upstream session files display managers use have no concept of sessions being composed from desktop manager and window manager. To be able to set upstream session files as default session, we need a single option. Having two different ways to set default session would be confusing, though, so we decided to deprecate the old method. We also created separate script for each session, just like we already had a separate desktop file for each one, and started using displayManager.sessionPackages mechanism to make the session handling more uniform.
705d032
to
8dc5ff7
Compare
Rebased onto the sway changes and squashed the commits. Is there anything remaining to do? I successfully ran |
It's good with me @jtojnar, and mostly because I don't think I can look over it again differently 😄 I'd say, maybe one more reviewer with a different set of eyes. |
Went over the new code and it looks good to me. 👍 on deprecating the old default options and implementing legacy sessions through sessionPackages. Thanks for bringing this to the finish line :) |
@Profpatsch Agreed to QA this by using it. |
✔️ I upgraded from 19.09 to nixos-unstable, and added this to my configuration:
After deleting an old |
@grahamc: with this pr LightDM will launch wayland sessions (just double checked that it works with sway), and you'll be able to use |
This reminds me I have a branch with canonical/lightdm@03f2189 on top of this. Will PR this after, I'm not sure how it works for sway without the commit though (perhaps it uses the key in the session that forces it) |
Just tried this with my |
Ah sorry |
@xaverdh Autologin requries a defaultSession to be selected. So I believe it choose |
This module allows root autoLogin, so we would break that for users, but they shouldn't be using it anyways. This gives the impression like auto is some special display manager, when it's just lightdm and special pam rules to allow root autoLogin. It was created for NixOS's testing so I believe this is where it belongs.
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
Motivation for this change
Integrate
services.xserver.displayManager.sessionPackages
better with the existing session system.resolves #54089
services.xserver.displayManager.defaultSession
There's two ways of providing graphical sessions now:
displayManager.session
via.desktopManager.session
andwindowManager.session
displayManager.sessionPackages
sessionPackages
doesn't make a distinction between desktop and windowmanagers. This makes selecting a session provided by a package using
desktopManager.default
nonsensical.We therefor introduce
displayManager.defaultSession
which can select a sessionfrom either
displayManager.session
ordisplayManager.sessionPackages
.It will default to
desktopManager.default + windowManager.default
as before.If the dm default is "none" it will select the first provided session from
sessionPackages
.services.xserver.displayManager.sessionPackages
Note
extraSessionFilePackages
is renamed to the less verbosesessionPackages
.When adding a package,
foo
, tosessionPackages
session names are now required to be specified infoo.passthru.providedSessions
, this is used to validatedefaultSession
at evaluation time.At build time
mkDesktops
will check that the sessions provided by a package is actually present, bailing out if not, hopefully preventing typos etc.closes #39871
Things done
Ran these tests succesfully:
After
default
options deprecation: