Hi Max
On 22/03/2024 22:11, Max Gautier wrote:
* Distribute the systemd timers used by the `git maintenance start` with
the systemd scheduler as part of git, rather than writing them in
$XDG_CONFIG_HOME.
This allows users to override the units if they wish, and is more
in-line with the usual practices of distribution for systemd units.
Thanks for suggesting this, I think this is a useful change, but the
implementation could be improved.
We also move away from using the random minute, and instead rely on
systemd features to achieve the same goal (see patch 2). This allows us
to go back to using unit templating for the timers. This is also a
prerequisite to have static unit files.
Note that even if we really need more specific OnCalendar= settings for
each timer, we should still do it that way, but instead distribute
override alongside the template, for instance for weekly:
/usr/lib/systemd-user/git-maintenance@daily.timer.d/override.conf:
[Timer]
OnCalendar=<daily specific calendar spec>
We should definitely do that. Using systemd's random delay does not
prevent the different maintenance jobs from running concurrently as one
job may be started before a previous job has finished. It is important
to only have one job running at a time because the first thing "git
maintenance run" does is to try and acquire a lock file so if the hourly
job is running when the daily jobs tries to start the daily job will not
be run. As the daily job is a superset of the hourly job and the weekly
job is a superset of the daily job so it does not make sense to run more
than one job per hour.
The cleanup code for the units written in $XDG_CONFIG_HOME is adapted,
and takes care of not removing legitimate user overrides, by checking
the file start.
This points to an alternate strategy for supporting user overrides -
don't overwrite the unit files if the user has edited them. I think that
there is still a benefit to moving to system wide unit files though as
it means that if we improve the unit files in the future systemd will
pick up these improvements automatically. That is an improvement over
the status quo where the users' unit files are written once and never
updated.
I think it would help to reorder the changes in this series as follows:
1 - simplify the invocation of "systemctl --user"
This would be the current patch 3 without adding "--force" or
moving "--now" combined with the relevant test changes from patch 6.
This would make it clear that those changes are a simple clean up that
is independent of the other changes made in this series.
2 - don't delete user edited unit files
This would be based on the current patch 4 and would show that we can
avoid deleting unit files that the user has edited without the other
changes in this series. This change should have an associated test.
3 - start using systemd's random delay function
This would be the current patch 1 without the template changes and the
commit message should explain that it is in preparation for disturbing
system-wide unit files.
4 - install system-wide systemd unit files
This would be based on the current patch 2 with the addition of
overrides to prevent more than one job running per hour. The unit
files should be installed under $XDG_DATA_HOME when $(prefix) starts
with $(HOME), not just when they are equal. The associated test
changes from patch 6 should be moved here as well as the "--force"
change from patch 3.
5 - documentation updates
I'm on the fence about having these in a separate commit like the
current patch 5 or updating the documentation when the code is
changed.
Best Wishes
Phillip
Testing:
The simplest way to simulate having the units in /usr/lib is probably to
copy them in /etc/systemd/user.
Changes since v1:
- Reorganization of the commits and their messages to try to address
review comments
- Dropped the DON'T APPLY PATCH, added a TODO to the cleanup code
instead
- Updated the git-maintenance tests to work with the new logic.
- Conditional installation of the units files
- Fixing some style/consistency issues
- template the systemd service file to use $(bindir)
Max Gautier (6):
maintenance: use systemd timers builtin randomization
maintenance: use packaged systemd units
maintenance: simplify systemctl calls
maintenance: cleanup $XDG_CONFIG_HOME/systemd/user
maintenance: update systemd scheduler docs
maintenance: update tests for systemd scheduler
Documentation/git-maintenance.txt | 33 ++-
Makefile | 5 +
builtin/gc.c | 298 ++++-------------------
config.mak.uname | 10 +
systemd/user/git-maintenance@xxxxxxxxxxx | 17 ++
systemd/user/git-maintenance@.timer | 12 +
t/t7900-maintenance.sh | 50 ++--
7 files changed, 126 insertions(+), 299 deletions(-)
create mode 100644 systemd/user/git-maintenance@xxxxxxxxxxx
create mode 100644 systemd/user/git-maintenance@.timer
Range-diff against v1:
1: ea54a6e50e < -: ---------- maintenance: package systemd units
2: b29dbb9fdd < -: ---------- maintenance: use packaged systemd units
3: 47bd6712b8 < -: ---------- maintenance: add fixed random delay to systemd timers
-: ---------- > 1: 42d88c7f81 maintenance: use systemd timers builtin randomization
-: ---------- > 2: 18d51b1dd1 maintenance: use packaged systemd units
-: ---------- > 3: 3aa7446e95 maintenance: simplify systemctl calls
-: ---------- > 4: daff7b4d60 maintenance: cleanup $XDG_CONFIG_HOME/systemd/user
4: fac57db55e ! 5: 5f6a8e141f maintenance: update systemd scheduler docs
@@ Metadata
## Commit message ##
maintenance: update systemd scheduler docs
+ The `git maintenance` systemd scheduler no longer writes units in
+ $XDG_CONFIG_HOME.
+
+ Describe the new behavior.
+ Instead of explaining manual ways to modify the timer, suggest the
+ systemd standard tool: `systemctl edit`.
+
Signed-off-by: Max Gautier <mg@xxxxxxxxxxxxxxxx>
## Documentation/git-maintenance.txt ##
5: d888fbd0c3 < -: ---------- DON'T APPLY YET: maintenance: remove cleanup code
-: ---------- > 6: 4d4bcd6233 maintenance: update tests for systemd scheduler
