nixos/switch-to-configuration: Improve socket and timer handling, clean up, minor fixes, add test #141192
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
6.topic: nixos
dasJ
changed the title
12 tasks
dasJ
changed the title
ofborg
bot
added
10.rebuild-darwin: 0
|
@GrahamcOfBorg test switchTest |
|
Ohh, does this fix #74899 |
|
Umm presumably. I didn't know there was an issue for that 😄 |
chrissyjai
approved these changes
Oct 10, 2021
dasJ
force-pushed
the
feat/improved-socket-handling2
branch
2 times, most recently
from
9e5043b to
bcea817
Compare
dasJ
changed the title
dasJ
commented
Oct 14, 2021
Mic92
approved these changes
Oct 14, 2021
dasJ
force-pushed
the
feat/improved-socket-handling2
branch
from
d48d0d1 to
776bccc
Compare
|
cc @stigtsp since your Perl is probably one of the best ;) |
Mic92
reviewed
Oct 14, 2021
dasJ
force-pushed
the
feat/improved-socket-handling2
branch
from
ac96d67 to
4a41568
Compare
Mic92
reviewed
Oct 14, 2021
dasJ
force-pushed
the
feat/improved-socket-handling2
branch
from
4a41568 to
69b2614
Compare
Mic92
reviewed
Oct 14, 2021
They are not managed by us and it might be weird to users to see units they didn't expect to be started.
This commit changes a lot more that you'd expect but it also adds a lot of new testing code so nothing breaks in the future. The main change is that sockets are now restarted when they change. The main reason for the large amount of changes is the ability of activation scripts to restart/reload units. This also works for socket-activated units now, and honors reloadIfChanged and restartIfChanged. The two changes don't really work without each other so they are done in the one large commit. The test should show what works now and ensure it will continue to do so in the future.
The first FIXME is removed because it doesn't make sense to use /proc/1/exe since that points to a directory that doesn't have all tools the activation script needs (like systemd-escape). The second one is removed because there is already no error handling (compare with the restart logic where the return code is checked).
The previous logic failed to detect that units were socket-activated when the socket was stopped before switch-to-configuration was run. This commit fixes that and also starts the socket in question.
This makes the order of operations the same in dry-activate and a "true" activate. Also fixes the indentation I messed up and drop a useless unlink() call (we are already unlinking that file earlier).
dasJ
force-pushed
the
feat/improved-socket-handling2
branch
from
ae5c3ae to
4cdbb2d
Compare
|
Tested on some machines. Works for me. |
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)
13 tasks
|
Unfortunately this causes the exact same issues as before and I urge to revert it ASAP because it is very annoying (no offense): #147609 I'm a bit late to the party as I didn't update in a while as I lacked time to deal with such issues (and I was afraid when I saw this change). |
nrdxp
added a commit
that referenced
this pull request
Nov 27, 2021
…ket-units Revert "Merge pull request #141192 from helsinki-systems/feat/improve…
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)
nrdxp
added a commit
that referenced
this pull request
Nov 27, 2021
[Backport release-21.11] Revert "Merge pull request #141192 from helsinki-systems/feat/improve…
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)
13 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation for this change
This is the second attempt to restart sockets properly.
The last time people were not completely happy so I kind of went over the top here.
cc @alyssais to make sure I don't break cross again
cc @grahamc maybe you could take a look at the Perl if you find time
Fixes #74899
Closes #33661
Things done
sandbox = trueset innix.conf? (See Nix manual)switch-test.nixnix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)