nixos/systemd: don't require network-online.target for multi-user.target #258680
Conversation
|
Tests I managed to run and failed prior to something starting to compile firefox and me realizing this is absolutely a hydra-requiring task:
Tests that seem to just be flaky/unrelated broken?
|
ofborg
bot
added
10.rebuild-darwin: 1-10
10.rebuild-darwin: 1
10.rebuild-linux: 1-10
labels
|
Firefox shouldn't have to be built if you use a revision where it's built, rebase onto nixos-unstable to not require hydra to build anything else than the tests. |
Yes, but the tests themselves are also very annoying and resource intensive to build hahaha. |
|
Summary of discussion on Matrix with uep, Raito and others regarding this: our first goal is to get a Hydra run of all the tests with this PR applied. Then we can start fixing ones we know of and apply this change by default to all tests for the 23.11 release. In 24.05 we may be able to roll this out to all systems, and it will plausibly cause some breakage. Another suggestion was to apply it to all systems right after branch-off; this seems also plausible. |
lf-
force-pushed
the
jade/remove-multiuser-netonline-dep
branch
from
5a505bf
to
01e3196
Compare
|
rebased against nixos-unstable so hydra can be more useful. should have done that earlier. |
|
It was quite close from the start, if I looked right. |
lf-
force-pushed
the
jade/remove-multiuser-netonline-dep
branch
from
01e3196
to
82a4bac
Compare
lf-
force-pushed
the
jade/remove-multiuser-netonline-dep
branch
from
82a4bac
to
66b449b
Compare
|
Pipewire tests are known broken in the VM currently. |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/why-does-multi-user-target-depend-on-network-online-target/33565/11 |
lf-
force-pushed
the
jade/remove-multiuser-netonline-dep
branch
from
66b449b
to
da7d09c
Compare
This was done by generating a truly hilarious configuration: rg 'services\.[^.]+\.enable\t' opts-tags | cut -f1 > allonconfig.nix The following were not tested due to other evaluation errors. They should probably be manually audited. services.amule services.castopod services.ceph services.chatgpt-retrieval-plugin services.clamsmtp services.clight services.dante services.dex services.discourse services.dwm-status services.engelsystem services.foundationdb services.frigate services.frp services.grocy services.guacamole-client services.hedgedoc services.home-assistant services.honk services.imaginary services.jitsi-meet services.kerberos_server services.limesurvey services.mastodon services.mediawiki services.mobilizon services.moodle services.mosquitto services.nextcloud services.nullmailer services.patroni services.pfix-srsd services.pgpkeyserver-lite services.postfixadmin services.roundcube services.schleuder services.self-deploy services.slskd services.spacecookie services.statsd services.step-ca services.sympa services.tsmBackup services.vdirsyncer services.vikunja services.yandex-disk services.zabbixWeb
lf-
force-pushed
the
jade/remove-multiuser-netonline-dep
branch
from
14dcc64
to
1323e31
Compare
|
I disabled the Hydra jobset now. |
network-online.target must now be explicitly depended on
Fixes NixOS#282640 Consequence of: NixOS#258680
This flakiness might have been exposed by NixOS#258680 but it definitely was buggy before, since rss2email finishes its job and exits, which means the unit would be considered stopped if it completed. It's not (and shouldn't be) a oneshot service, so we need to use a different method to detect its completion state.
|
Committing to reverting, since we did not have much of any test coverage of network-manager (!!!!!) and thus didn't catch the regression #71151 This is evidence that we need to do another revision of this. |
Previously we required network-online.target for multi-user.target. This has made a lot of people very angry and has been widely regarded as a bad move (or at least, very nonstandard):
15d761a#commitcomment-128564097
This was done because of fragile tests and services declaring dependencies on multi-user.target when they meant network-online.target.
Let's rip off the bandaid and fix our tests.
I have not yet run any tests locally, and this is probably going to break a bunch of stuff. We should get some hydra resources and gather a list of the broken tests.
cc @NixOS/systemd
Previous related PRs/attempts in vain to execute this change:
Description of changes
Things done
sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)