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.

Sign up for GitHub

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

Jump to bottom

nixos/plymouth: do not order plymouth-quit after display-manager #71115

Merged
merged 1 commit into from Oct 19, 2019
Merged

nixos/plymouth: do not order plymouth-quit after display-manager #71115

merged 1 commit into from Oct 19, 2019

Conversation

hedning
Copy link
Contributor

@hedning hedning commented Oct 14, 2019

Motivation for this change

GDM now specifies ordering between plymouth-quit and display-manager:
9be5321

This can cause an ordering cycle between GDM and plymouth-quit which can result in
systemd breaking GDM:

plymouth-quit.service: Job display-manager.service/start deleted to break
                       ordering cycle starting with plymouth-quit.service/start

Not sure how often this triggers, as I've run my system with plymouth and
9be5321 without any issues. But I did catch a VM doing this.

NOTE: I also tried to remove the ordering in GDM to see if plymouth managed to
live longer, but it didn't seem to help. So I opted to stick as close to
upstream (upstream GDM specifies ordering, but plymouth does not).

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option 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 nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @

@hedning
nixos/plymouth: do not order plymouth-quit after display-manager
5924bab 
GDM now specifies ordering between `plymouth-quit` and `display-manager`:
9be5321

This causes an ordering cycle between GDM and plymouth-quit which can result in
systemd breaking GDM:
```
plymouth-quit.service: Job display-manager.service/start deleted to break
                       ordering cycle starting with plymouth-quit.service/start
```

Not sure how often this triggers, as I've run my system with plymouth and
9be5321 without any issues. But I did catch a VM doing this.

NOTE: I also tried to remove the ordering in GDM to see if plymouth managed to
live longer, but it didn't seem to help. So I opted to stick as close to
upstream (upstream GDM specifies ordering, but plymouth does not).
@hedning
Copy link
Contributor Author

hedning commented Oct 15, 2019

Yeah, looks like this mostly fails in «nice» way on my hardware:

okt. 15 18:56:30 x1 systemd[1]: display-manager.service: Found ordering cycle on plymouth-quit.service/start
okt. 15 18:56:30 x1 systemd[1]: display-manager.service: Found dependency on display-manager.service/start
okt. 15 18:56:30 x1 systemd[1]: display-manager.service: Job plymouth-quit.service/start deleted to break ordering cycle starting with display-manager.service/start

@hedning
Copy link
Contributor Author

hedning commented Oct 19, 2019

Just got a display-manager failure on a regular boot, so I'll merge this pretty soon, cc @worldofpeace.

@worldofpeace worldofpeace merged commit a13b8ce into NixOS:master Oct 19, 2019
@hedning hedning deleted the plymouth-gdm-fix branch March 1, 2020 11:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@worldofpeace worldofpeace worldofpeace approved these changes

Assignees
No one assigned
Labels
6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@hedning @worldofpeace