nixos/system: make switch-to-configuration script pure #31805
Conversation
|
@FRidh, please take a look. Especially if the added test is the right way to test it. |
|
@phile314 FYI |
| @@ -83,6 +82,7 @@ let | |||
| done | |||
|
|
|||
| mkdir $out/bin | |||
| export localeArchive=${lib.optionalString (pkgs.stdenv.isLinux or false) "${pkgs.glibcLocales}/lib/locale/locale-archive"} | |||
There was a problem hiding this comment.
This should use "${config.i18n.glibcLocales}/lib/locale/locale-archive" so the NixOS locale configuration options are respected.
And no need to make conditional for Linux.
| @@ -65,7 +69,7 @@ | |||
| sub getActiveUnits { | |||
| # FIXME: use D-Bus or whatever to query this, since parsing the | |||
| # output of list-units is likely to break. | |||
| my $lines = `LANG= systemctl list-units --full --no-legend`; | |||
| my $lines = `LANG= @systemd@/bin/systemctl list-units --full --no-legend`; | |||
There was a problem hiding this comment.
It worries me a bit what will happen if the systemd version used here doesn't match the running systemd.
Perhaps use /run/current-system/sw/bin/systemctl here.
There was a problem hiding this comment.
Perhaps use /run/current-system/sw/bin/systemctl here.
If doing that, please add a comment above that command line saying why.
There was a problem hiding this comment.
There are many spots in this file which use @systemd@/bin/systemctl already. Do you think I should switch all of them?
There was a problem hiding this comment.
AFAICT all of them (except one systemd-escape call, which presumably doesn't need to contact the daemon anyway) are run for the purposes of talking to the upgraded systemd, so those ones should be correct.
Although, at that point the activation script has been run and changed /run/current-system to point to the new system (incl. the new systemd), so theoretically it would still work. But sounds somewhat magical to me.
There was a problem hiding this comment.
In such a case, probably the best solution is to use result of sudo readlink /proc/1/exe to get absolute path to previous systemctl and use it before the daemon-reexec command, and after the call use @systemd@/bin/systemctl path. I'll make it so.
There was a problem hiding this comment.
Nah, this is too complex for very little benefit. I will just use /run/current-system/sw/bin/systemctl before the daemon-rexec call
gleber
force-pushed
the
make-switch-to-configuration-pure
branch
from
de27fd3
to
ac383ce
Compare
|
Please take another look. |
gleber
force-pushed
the
make-switch-to-configuration-pure
branch
from
ac383ce
to
e72bf2d
Compare
nixos/tests/switch-test.nix
Outdated
| # Mutable users tests. | ||
|
|
||
| import ./make-test.nix ({ pkgs, ...} : { | ||
| name = "mutable-users"; |
There was a problem hiding this comment.
Probably not the name you intended?
There was a problem hiding this comment.
Good catch.
A more important question: do you think a separate test like this is useful?
Fixes NixOS#28443 Fixed few invocations to `systemctl` to have an absolute path. Additionally add LOCALE_ARCHIVE so that perl stops spewing warning messages.
gleber
force-pushed
the
make-switch-to-configuration-pure
branch
from
e72bf2d
to
2f6148c
Compare
|
I don't have a complete grip of this change, but it looks overall good to me. |
|
@fpletz Added the test to |
Fixes #28443
Fixed few invocations to
systemctlto have an absolute path. Additionally addLOCALE_ARCHIVE so that perl stops spewing warning messages.
Things done
build-use-sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)