Skip to content
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

nixos: xserver.displayManager: use slim for automatic logins #46396

Closed
wants to merge 3 commits into from

Conversation

oxij
Copy link
Member

@oxij oxij commented Sep 8, 2018

Motivation for this change

lightdm is not that light for automatic logins: it runs two processes three threads each (while slim runs one of each) all of which stay in memory even after login is finished. Meanwhile, auto users usually want the display manager to get out of their way, not to pollute their system with six useless threads. Moreover, the closure size of slim+theme in its default config in NixOS is 53M, while the similar closure size of lightdm+greeter+theme is 254M.

Things done

`lightdm` is not that light for automatic logins: it runs two processes
three threads each (while slim runs one of each) all of which stay in
memory even after login is finished. Meanwhile, `auto` users usually
want the display manager to get out of their way, not to pollute their
system with six useless threads.  Moreover, the closure size of slim+theme
in its default config in NixOS is 53M, while the similar closure size of
lightdm+greeter+theme is 254M.

This reverts a part of commit fc035da.
This reverts commit 7e9bd2d.

No longer needed after the previous commit.
@oxij oxij changed the title Nixos/slim for auto nixos: xserver.displayManager: use slim for automatic logins Sep 8, 2018
@xeji
Copy link
Contributor

xeji commented Sep 8, 2018

If this is merged (I don't have a strong opinion about it), it should be backported to 18.09 because it can be a pain to debug/fix NixOS tests if the basic test setup is different between master and 18.09.

@GrahamcOfBorg GrahamcOfBorg added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Sep 8, 2018
user = cfg.user;
};
autoLogin = true;
defaultUser = cfg.user;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not an issue with this PR but we should standardize the settings so autoLogin works the same regardless of DM.

@oxij
Copy link
Member Author

oxij commented Sep 23, 2018 via email

@oxij oxij mentioned this pull request Oct 4, 2018
1 task
@wizeman
Copy link
Member

wizeman commented May 16, 2019

This PR also fixes another bug: if the desktop manager crashes, it causes lightdm to restart but it won't auto-login again, therefore getting stuck at the 'enter password' screen.
I was observing this behavior with Kodi, but when I tried applying this PR to my config, the bug is fixed.

@samueldr
Copy link
Member

The "bug" about lightdm not doing auto-login on crash (haven't checked, but I suspect it's when the session exits non-zero) is imo not a bug. This reduces the risk of a broken session causing a tight loop with the display manager logging-in → starting session → crashing → restarting, in those situations switching to a VT could be problematic since AFAIK most DMs will throw away the X server and start a new one, thus pulling the user back to the looping VT.

I do not know how to approach this appropriately, but maybe some tip in the documentation that for unattended autologin slim would be more appropriate due to that behaviour, while the behaviour from lightdm is preferred on an end-user's machine.

@wizeman
Copy link
Member

wizeman commented May 16, 2019

@samueldr IMO your scenario is more appropriately handled by lightdm exiting with a non-zero status (assuming the DM exited with a non-zero status) and letting systemd auto-restart the display-manager service. This would probably work better because systemd already has in-built mechanisms to prevent a service from entering an infinite start-fail loop (after a few tries, systemd stops trying to restart the service when the service fails relatively quickly after being restarted).

This might be what slim does already, although I haven't verified it yet.

@wizeman
Copy link
Member

wizeman commented May 16, 2019

Ok, I have verified that slim behaves exactly as I mentioned in my comment above, which is the behavior I would expect and prevents the infinite start-fail loop in case the desktop manager keeps crashing at startup.

To check this behavior, I have performed a bunch of pkill -9 kodi-x11 in a row (kodi-x11 is the process name of the desktop manager I am using) on a machine with this PR, and I confirm that systemd stopped restarting the display-manager service (i.e. the X server) after I killed kodi 3 times in less than one minute.

Note that systemd already does this by default, but the "stop restarting" behavior is also explicitly fine-tuned here:

# Stop restarting if the display manager stops (crashes) 2 times
# in one minute. Starting X typically takes 3-4s.
StartLimitInterval = "30s";
StartLimitBurst = "3";

To be clear, note that lightdm currently does not behave like this -- instead, if the desktop manager crashes lightdm will simply go back to a login screen even though auto-login is enabled, which is not the behavior I would expect (principle of least surprise?).

So to summarize, IMHO I think this PR fixes a bunch of issues with auto-login and I think it should either be merged or lightdm should be fixed accordingly.

@samueldr
Copy link
Member

Looks like lightdm has an open issue.

@fgaz
Copy link
Member

fgaz commented Dec 6, 2019

Slim was removed in #73251, I think this can be closed

@oxij oxij mentioned this pull request Dec 9, 2019
10 tasks
@oxij
Copy link
Member Author

oxij commented Dec 9, 2019 via email

@oxij oxij mentioned this pull request Dec 9, 2019
3 tasks
oxij added a commit to SLNOS/nixpkgs that referenced this pull request Feb 10, 2020
This reverts commit 4583e29,
reversing changes made to d6fa540.

The fact that SLiM is not officially actively maintained is not a good
reason to remove it. It works, and, in some instances, like automatic
logins, it is the only thing that actually works. See
NixOS#46396 and
NixOS#30890 (comment).
@stale
Copy link

stale bot commented Jun 6, 2020

Thank you for your contributions.

This has been automatically marked as stale because it has had no activity for 180 days.

If this is still important to you, we ask that you leave a comment below. Your comment can be as simple as "still important to me". This lets people see that at least one person still cares about this. Someone will have to do this at most twice a year if there is no other activity.

Here are suggestions that might help resolve this more quickly:

  1. Search for maintainers and people that previously touched the related code and @ mention them in a comment.
  2. Ask on the NixOS Discourse.
  3. Ask on the #nixos channel on irc.freenode.net.

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 6, 2020
@ryantm ryantm added the 2.status: merge conflict This PR has merge conflicts with the target branch label Oct 3, 2020
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Oct 3, 2020
@ryantm ryantm added 2.status: merge conflict This PR has merge conflicts with the target branch and removed 2.status: merge conflict This PR has merge conflicts with the target branch labels Oct 3, 2020
@stale
Copy link

stale bot commented Apr 7, 2021

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

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Apr 7, 2021
@Artturin Artturin closed this Apr 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: merge conflict This PR has merge conflicts with the target branch 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants