Hello, Thank you all for your valuable feedback! I tried to address all the discussed points in this new version of the patch. Do not hesitate to let me know if I forgot anything. The main new thing in this version is the `--scheduler=<scheduler>` parameter that has been added to `git maintenance start` command. It allows the end user to choose between `cron` or user systemd timers for scheduling git maintenance tasks. I also addressed the migration problematic during an upgrade. If a user invokes `git maintenance start --scheduler=systemd-timer` first, and then invokes `git maintenance start --scheduler=cron` without invoking `git maintenance stop` in between, the git maintenance tasks will be removed from `systemd-timer` to be sure that the same tasks won’t be scheduled concurrently twice by both `systemd-timer` and `cron`. And the same in the other way round. On its side, `git maintenance stop` don’t have any `--scheduler=<scheduler>` parameter as it will try to remove the git maintenance tasks from all the schedulers available on the system. The default scheduler when `--scheduler=<scheduler>` isn’t specified is `auto` which means “choose an appropriate scheduler”. On Windows and MacOS, it always chooses the specific scheduler on those platforms, `schtasks` and `launchctl`. On Linux, it chooses user systemd timers if they are available and `cron` otherwise. This order has been a subject of discussion so, let me explain why I chose this order. On my system, I uses systemd and all the packages of my distribution that are needing regular scheduled tasks are defining systemd timers instead of cron task. So, I don’t use crontab anymore. However, a cron package is installed because it is an indirect dependency of a package I installed through my Linux distribution package manager. But the cron daemon isn’t started. `systemctl --user list-timers` is the CLI command that actually talks to the daemon in charge of scheduling timers. So, if `systemctl --user list-timers` succeed, we can be sure that user systemd timers are functional. Concretely, the `is_systemd_timer_available` function of this patch is reliable. `crontab`, on the other hand, only reads and writes to `/var/spool/cron/$USER` but this command can work also if the `cron` daemon isn’t running. In this case, the scheduled tasks will never run. Concretely, the `is_crontab_available` function of this patch is less reliable. We would need to check if the `cron` daemon is really running. Relying on `systemctl` to check if the service is enabled and running isn’t ideal since we want to support systems that don’t have systemd. Parsing directly `/proc` is challenging since its content is OS specific. `/proc` content on a Solaris system is different than its content on a Linux system. We could rely on `ps` but we must keep in mind its interface is very different from one system to another. It doesn’t implement both the standard syntax and the BSD syntax for its arguments on all platforms for example. Moreover, Linux distributions are proposing several different implementations of `cron`: `cronie`, `fcron`, `dcron`, `vixie-cron`, `scron`, `bcron`. See: * https://wiki.archlinux.org/title/Cron#Installation * https://wiki.gentoo.org/wiki/Cron#Which_cron_is_right_for_the_job.3F Depending on the `cron` implementation, the name of the `cron` daemon process might differ. So, reliably detecting if a `cron` daemon is running may require to review each `cron` implementation. With this new version of the patch, advanced users that care about systemd timers versus `cron` can explicitly choose which one they want to use. For less advanced users that don’t care, I prefer to choose the method which has the higher probability of working. But since this order is a subject of debate, what I can propose is a `./configure` compile time option to select which `cron` or systemd timers should be chosen in priority if both are available. In addition to this big change, this new version of the patch also honors the `XDG_CONFIG_HOME` environment variable and removes the code duplication between `systemd_timer_timer_filename()` and `systemd_timer_service_filename()`. It also fixes other points raised in the code review. Lénaïc Huard (1): maintenance: use systemd timers on Linux Documentation/git-maintenance.txt | 60 +++++ builtin/gc.c | 375 ++++++++++++++++++++++++++++-- t/t7900-maintenance.sh | 51 ++++ 3 files changed, 462 insertions(+), 24 deletions(-) -- 2.31.1
