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/switch-to-configuration: restart changed socket units #73871

Merged
merged 1 commit into from Nov 26, 2019

Conversation

lheckemann
Copy link
Member

Motivation for this change

The bugfix part of #50340

Things done
  • Tested via nixos/tests/mpd.nix
  • Tested via nixos/tests/systemd-confinement.nix
  • Fits CONTRIBUTING.md.

cc author @fpletz

Copy link
Contributor

@flokli flokli left a comment

Choose a reason for hiding this comment

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

Can we also extend this to .socket for user sessions?

This should help with #71095

@domenkozar
Copy link
Member

It's not clear to me how this relates to #71095, but I think there's no need to extend scope. The fix already improves socket handling, any further improvements can be done with whoever does them later on :)

@flokli
Copy link
Contributor

flokli commented Nov 25, 2019 via email

Previously, socket units wouldn't be restarted if they were
changed. To restart the socket, the service the socket is attached
to needs to be stopped first before the socket can be restarted.
@primeos
Copy link
Member

primeos commented Nov 29, 2019

Unfortunately this causes severe regressions by restarting systemd services that should not be restarted, e.g. dbus.service (I assume due to Requires=dbus.socket). This does e.g. crash Sway (and probably other graphical sessions as well) after nixos-rebuild switch.

See #74626 for more details.

@domenkozar
Copy link
Member

Reverted in master until that's fixed.

primeos added a commit to primeos/nixpkgs that referenced this pull request Nov 27, 2021
…proved-socket-handling2"

This reverts commit 57961d2, reversing
changes made to b04f913.
(I.e. this reverts PR NixOS#141192.)

While well-intended, this change does unfortunately introduce very
serious regressions that are especially disruptive/noticeable on desktop
systems (e.g. users of Sway will loose their graphical session when
running "nixos-rebuild switch").

Therefore, this change has to be reverted ASAP instead of trying to fix
it in "production".
Note: An updated version should be extensively discussed, reviewed, and
tested before re-landing this change as an earlier version also had to
be reverted for the exact same issues [0].

Fix: NixOS#146727

[0]: NixOS#73871 (comment)
github-actions bot pushed a commit that referenced this pull request Nov 27, 2021
…d-socket-handling2"

This reverts commit 57961d2, reversing
changes made to b04f913.
(I.e. this reverts PR #141192.)

While well-intended, this change does unfortunately introduce very
serious regressions that are especially disruptive/noticeable on desktop
systems (e.g. users of Sway will loose their graphical session when
running "nixos-rebuild switch").

Therefore, this change has to be reverted ASAP instead of trying to fix
it in "production".
Note: An updated version should be extensively discussed, reviewed, and
tested before re-landing this change as an earlier version also had to
be reverted for the exact same issues [0].

Fix: #146727

[0]: #73871 (comment)

(cherry picked from commit 1cfecb6)
mkg20001 pushed a commit to mkg20001/nixpkgs that referenced this pull request Nov 29, 2021
…proved-socket-handling2"

This reverts commit 57961d2, reversing
changes made to b04f913.
(I.e. this reverts PR NixOS#141192.)

While well-intended, this change does unfortunately introduce very
serious regressions that are especially disruptive/noticeable on desktop
systems (e.g. users of Sway will loose their graphical session when
running "nixos-rebuild switch").

Therefore, this change has to be reverted ASAP instead of trying to fix
it in "production".
Note: An updated version should be extensively discussed, reviewed, and
tested before re-landing this change as an earlier version also had to
be reverted for the exact same issues [0].

Fix: NixOS#146727

[0]: NixOS#73871 (comment)
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

5 participants