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/accountservice: Don't restart account-daemon on upgrades #45936

Closed
wants to merge 1 commit into from
Closed

nixos/accountservice: Don't restart account-daemon on upgrades #45936

wants to merge 1 commit into from

Conversation

etu
Copy link
Contributor

@etu etu commented Sep 2, 2018 •
edited

Motivation for this change

If account-daemon restarts in a gnome3 session it totally breaks and becomes unusable.

What happens for me if account-daemon restarts is that I see the screen flash like the login animation from gdm. All programs are still up. But the mouse is frozen and I can't even switch to a tty with my keyboard. The only thing that it actually responds to seems to be Ctrl+Alt+Delete which triggers a reboot. And this indicates that I'm actually in a tty but I see an image of my gnome-session.

This is a problem when doing nixos-rebuild switch etc.

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 nox --run "nox-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)
  • Fits CONTRIBUTING.md.

cc @jtojnar

xeji
xeji reviewed Sep 2, 2018
View reviewed changes
nixos/modules/services/desktops/accountsservice.nix Outdated
@@ -11,6 +11,7 @@ with lib;
options = {

services.accounts-daemon = {
restartIfChanged = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this breaks eval because it belongs in the implementation (config below), not the options.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've copy-pasted it wrong from my systems nixpkgs. Fixed.

Copy link
Contributor

@xeji xeji Sep 2, 2018 •
edited

Choose a reason for hiding this comment

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

I'm not sure if this is applicable here, but many other modules use reloadIfChanged = true instead to have the service reload its config without restarting it. Might be worth a try.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tested that, and it gave errors because this service didn't like to be reloaded.

@etu
nixos/accountservice: Don't restart account-daemon on upgrades
c950a88 
If account-daemon restarts in a gnome3 session it totally breaks and
becomes unusable.
@etu etu force-pushed the stop-restarting-accountservice branch from 82e37d2 to c950a88 Compare September 2, 2018 09:15
etu added a commit to etu/nixconfig that referenced this pull request Sep 2, 2018
@etu
my-desktop-gnome: Disable restarts of accounts-daemon to avoid crashes
052b9d6 
Upstream issue: NixOS/nixpkgs#45936
@Mic92 Mic92 added this to the 18.09 milestone Sep 3, 2018
@etu
Copy link
Contributor Author

etu commented Sep 8, 2018

Hmm, this fix that I got in my private config doesn't seem to resolve these crashes anymore... Not sure why...

@etu etu closed this Sep 8, 2018
@etu etu deleted the stop-restarting-accountservice branch September 23, 2018 05:45
@talyz
Copy link
Contributor

talyz commented Sep 28, 2018

Having "systemd.services.accounts-daemon.restartIfChanged = false" in configuration.nix solves the issue for me, so this seems valid...

@etu etu mentioned this pull request Sep 28, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xeji xeji xeji left review comments

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

Successfully merging this pull request may close these issues.

None yet
5 participants
@etu @talyz @xeji @Mic92 @GrahamcOfBorg