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

Make plymouth work with gdm wayland #39978

Merged
merged 1 commit into from May 15, 2018

Conversation

hedning
Copy link
Contributor

@hedning hedning commented May 4, 2018

Motivation for this change

@jtojnar is getting gdm to use wayland in #39615. Trying it out I ran into a problem where gdm would fail the first time when using plymouth (on a 3rd gen X1 carbon, but not in a VM for some reason).

According to the service directory in plymouth "multi-user.target" should want "plymouth-quit-wait.service":

├── multi-user.target.wants
│   ├── plymouth-quit.service -> ../plymouth-quit.service
│   └── plymouth-quit-wait.service -> ../plymouth-quit-wait.service

This change fixed gdm running wayland, and seems like reasonable change since it's apparently expected by plymouth.

However the change made other display managers not quit plymouth properly, leaving it running in the background consuming cpu. Removing "multi-user.target" from plymouth-quit.after makes plymouth quit as expected. Not sure why having it there was a problem though.

cc @abbradar (sorry for bugging you) as he's the author of the code I'm changing.

Things done

Tested all the display managers in a VM. All are showing the console on stage 1. as expected (I'm assuming this is needed for LUKS password). Also tested without xserver which also worked fine.

Though I haven't tested the configuration with LUKS enabled. If someone knows how to test this easily in eg. a VM that would be great.

  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

This is apparent from the service file directory in plymouth:
├── multi-user.target.wants
│   ├── plymouth-quit.service -> ../plymouth-quit.service
│   └── plymouth-quit-wait.service -> ../plymouth-quit-wait.service

Leaving it unspecified caused gdm-wayland to crash on boot, see NixOS#39615.

The change made other display managers not quit plymouth properly however. By
removing "multi-user.target" from `plymouth-quit.after` this is resolved.
Copy link
Member

@abbradar abbradar left a comment

Choose a reason for hiding this comment

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

LGTM! I can't test this right now by myself but changes look sensible. I'll test&merge in couple of days unless any problem emerges.

@matthewbauer matthewbauer merged commit 2a3399b into NixOS:master May 15, 2018
@hedning hedning deleted the plymouth-quit-wait branch May 15, 2018 21:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants