logrotate: do not enable logrotate.service itself#161929
Conversation
github-actions
bot
added
6.topic: nixos
|
@ofborg test switchTest |
|
@ofborg test logrotate |
logrotate.timer is enough for rotating logs. Enabling logrotate.service would make the service start on every configuration switch, leading to tests failure when logrotate is enabled. Also update test to make sure the timer is active and runs the service on date change.
Currently the test-watch.service gets started in a loop as long as /testpath exists, so `rm /testpath /testpath-modified` runs into a race condition where if the service was just getting activated, it will create /testpath-modified and make the test fail. This is fixed by making the service RemainAfterExit so that it only starts once, and stopping it manually after we remove /testpath.
martinetd
force-pushed
the
switchTest
branch
from
cfbb232 to
8022c82
Compare
|
(force push just now updated commit message of first commit, sorry I just noticed non-sense ('for what rotating' had extra what). Content is identical so I'm not re-triggering ofborg, feel free to if that's appropriate) |
ofborg
bot
added
10.rebuild-darwin: 0
|
I confirm that this version of test succeeds reliably on my machine. It still feels like this particular tests is (and was) very expensive to evaluate, but that would belong to a different discussion thread and I don't have energy for it anyway. |
|
I didn't do the legwork to check but if nixos-unstable is really blocked on this should it be merged? The channel has been stuck for 6 days and I'd rather not be responsible for too many delayed security fixes... (and that'll perhaps teach me that such defaults changes should go to staging even if it looked harmless, sorry for the trouble that caused :() |
|
Right now the channel is also not evaluating, but yes this should be merged ASAP |
4 participants
logrotate.timer is enough for rotating logs. Enabling logrotate.service would
make the service start on every configuration switch, leading to tests failure when
logrotate is enabled.
Also update test to make sure the timer is active and runs the service
on date change.
supersedes #161859 as a fix for testSwitch, I've taken the second commit that was unrelated as well...
cc @ncfavier @vcunat @dasJ