Hello,
Please find hereafter a new reroll of my patchset to add support for
systemd timers on Linux for the `git maintenance start` command.
The changes compared to the previous version address the remarks
raised during the previous review.
The patches are:
* cache.h: Introduce a generic "xdg_config_home_for(…)" function
This patch introduces a function to compute configuration files
paths inside $XDG_CONFIG_HOME.
It is used in the latest patch of this series to compute systemd
unit files location.
This patch is unchanged compared to its previous version.
* maintenance: `git maintenance run` learned `--scheduler=<scheduler>`
This patch adds a new parameter to the `git maintenance run` to let
the user choose a scheduler.
This patch contains the following changes compared to its previous
version:
* `is_crontab_available` was not returning directly `is_available`
under tests condition (when `GIT_TEST_MAINT_SCHEDULER` is set).
This was indeed a bug as it means `cron` could be invoked by tests
whereas it should be mocked.
This is now fixed and `is_crontab_available` now has exactly the
same behavior as the `is_systemd_timer_available` function that is
introduced in the last patch.
* The `get_schedule_cmd` function that centralizes the testing logic
is now prefixed by a comment block describing its behavior and the
expected values for the `GIT_TEST_MAINT_SCHEDULER` environment
variable.
* The help message for the `--scheduler` option of the `git
maintenance start` command has been reworded following Eric’s
suggestion.
I’ve however kept the “When combined with the `start` subcommand…”
opening to keep consistency with the other options documented on
the same page.
* `resolve_auto_scheduler` function has been renamed
`resolve_scheduler` and it is now returning a value instead of
altering its parameter.
* maintenance: add support for systemd timers on Linux
This patch implements the support of systemd timers on top of
crontab scheduler on Linux systems.
The changes in this patch are only followups of changes mentioned in
the previous patch:
* `resolve_scheduler` is now returning a value instead of altering
its parameter.
Best wishes,
Lénaïc.
Lénaïc Huard (3):
cache.h: Introduce a generic "xdg_config_home_for(…)" function
maintenance: `git maintenance run` learned `--scheduler=<scheduler>`
maintenance: add support for systemd timers on Linux
Documentation/git-maintenance.txt | 57 +++
builtin/gc.c | 597 ++++++++++++++++++++++++++----
cache.h | 7 +
path.c | 13 +-
t/t7900-maintenance.sh | 110 +++++-
5 files changed, 706 insertions(+), 78 deletions(-)
Diff-intervalle contre v6 :
-: ---------- > 1: 899b11ed5b cache.h: Introduce a generic "xdg_config_home_for(…)" function
1: 604627f347 ! 2: f3e2f0256b maintenance: `git maintenance run` learned `--scheduler=<scheduler>`
@@ Documentation/git-maintenance.txt: OPTIONS
+--scheduler=auto|crontab|launchctl|schtasks::
+ When combined with the `start` subcommand, specify the scheduler
-+ to use to run the hourly, daily and weekly executions of
++ for running the hourly, daily and weekly executions of
+ `git maintenance run`.
-+ The possible values for `<scheduler>` depend on the system: `crontab`
-+ is available on POSIX systems, `launchctl` is available on
-+ MacOS and `schtasks` is available on Windows.
-+ By default or when `auto` is specified, a suitable scheduler for
-+ the system is used. On MacOS, `launchctl` is used. On Windows,
-+ `schtasks` is used. On all other systems, `crontab` is used.
++ Possible values for `<scheduler>` are `auto`, `crontab` (POSIX),
++ `launchctl` (macOS), and `schtasks` (Windows).
++ When `auto` is specified, the appropriate platform-specific
++ scheduler is used. Default is `auto`.
+
TROUBLESHOOTING
@@ builtin/gc.c: static const char *get_frequency(enum schedule_priority schedule)
}
}
++/*
++ * get_schedule_cmd` reads the GIT_TEST_MAINT_SCHEDULER environment variable
++ * to mock the schedulers that `git maintenance start` rely on.
++ *
++ * For test purpose, GIT_TEST_MAINT_SCHEDULER can be set to a comma-separated
++ * list of colon-separated key/value pairs where each pair contains a scheduler
++ * and its corresponding mock.
++ *
++ * * If $GET_TEST_MAINT_SCHEDULER is not set, return false and leave the
++ * arguments unmodified.
++ *
++ * * If $GET_TEST_MAINT_SCHEDULER is set, return true.
++ * In this case, the *cmd value is read as input.
++ *
++ * * if the input value *cmd is the key of one of the comma-separated list
++ * item, then *is_available is set to true and *cmd is modified and becomes
++ * the mock command.
++ *
++ * * if the input value *cmd isn’t the key of any of the comma-separated list
++ * item, then *is_available is set to false.
++ *
++ * Ex.:
++ * GIT_TEST_MAINT_SCHEDULER not set
++ * ┏━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
++ * ┃ Input ┃ Output ┃
++ * ┃ *cmd ┃ return code │ *cmd │ *is_available ┃
++ * ┣━━━━━━━╋━━━━━━━━━━━━━┿━━━━━━━━━━━━━━━━━━━┿━━━━━━━━━━━━━━━┫
++ * ┃ "foo" ┃ false │ "foo" (unchanged) │ (unchanged) ┃
++ * ┗━━━━━━━┻━━━━━━━━━━━━━┷━━━━━━━━━━━━━━━━━━━┷━━━━━━━━━━━━━━━┛
++ *
++ * GIT_TEST_MAINT_SCHEDULER set to “foo:./mock_foo.sh,bar:./mock_bar.sh”
++ * ┏━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
++ * ┃ Input ┃ Output ┃
++ * ┃ *cmd ┃ return code │ *cmd │ *is_available ┃
++ * ┣━━━━━━━╋━━━━━━━━━━━━━┿━━━━━━━━━━━━━━━━━━━┿━━━━━━━━━━━━━━━┫
++ * ┃ "foo" ┃ true │ "./mock.foo.sh" │ true ┃
++ * ┃ "qux" ┃ true │ "qux" (unchanged) │ false ┃
++ * ┗━━━━━━━┻━━━━━━━━━━━━━┷━━━━━━━━━━━━━━━━━━━┷━━━━━━━━━━━━━━━┛
++ */
+static int get_schedule_cmd(const char **cmd, int *is_available)
+{
+ char *item;
@@ builtin/gc.c: static int schtasks_schedule_task(const char *exec_path, enum sche
+ int is_available;
+ struct child_process child = CHILD_PROCESS_INIT;
+
-+ if (get_schedule_cmd(&cmd, &is_available) && !is_available)
-+ return 0;
++ if (get_schedule_cmd(&cmd, &is_available))
++ return is_available;
+
+ strvec_split(&child.args, cmd);
+ strvec_push(&child.args, "-l");
@@ builtin/gc.c: static int crontab_update_schedule(int run_maintenance, int fd, co
+ enum scheduler scheduler;
+};
+
-+static void resolve_auto_scheduler(enum scheduler *scheduler)
++static enum scheduler resolve_scheduler(enum scheduler scheduler)
+{
-+ if (*scheduler != SCHEDULER_AUTO)
-+ return;
++ if (scheduler != SCHEDULER_AUTO)
++ return scheduler;
+
#if defined(__APPLE__)
-static const char platform_scheduler[] = "launchctl";
-+ *scheduler = SCHEDULER_LAUNCHCTL;
-+ return;
++ return SCHEDULER_LAUNCHCTL;
+
#elif defined(GIT_WINDOWS_NATIVE)
-static const char platform_scheduler[] = "schtasks";
-+ *scheduler = SCHEDULER_SCHTASKS;
-+ return;
++ return SCHEDULER_SCHTASKS;
+
#else
-static const char platform_scheduler[] = "crontab";
-+ *scheduler = SCHEDULER_CRON;
-+ return;
++ return SCHEDULER_CRON;
#endif
+}
@@ builtin/gc.c: static int crontab_update_schedule(int run_maintenance, int fd, co
+ if (scheduler == SCHEDULER_INVALID)
+ BUG("invalid scheduler");
+ if (scheduler == SCHEDULER_AUTO)
-+ BUG("resolve_auto_scheduler should have been called before");
++ BUG("resolve_scheduler should have been called before");
+
+ if (!scheduler_fn[scheduler].is_available())
+ die(_("%s scheduler is not available"),
@@ builtin/gc.c: static int crontab_update_schedule(int run_maintenance, int fd, co
+ struct option options[] = {
+ OPT_CALLBACK_F(
+ 0, "scheduler", &opts.scheduler, N_("scheduler"),
-+ N_("scheduler to use to trigger git maintenance run"),
++ N_("scheduler to trigger git maintenance run"),
+ PARSE_OPT_NONEG, maintenance_opt_scheduler),
+ OPT_END()
+ };
@@ builtin/gc.c: static int crontab_update_schedule(int run_maintenance, int fd, co
+ if (argc)
+ usage_with_options(builtin_maintenance_start_usage, options);
+
-+ resolve_auto_scheduler(&opts.scheduler);
++ opts.scheduler = resolve_scheduler(opts.scheduler);
+ validate_scheduler(opts.scheduler);
+
if (maintenance_register())
2: 29628b5a92 ! 3: 0ea5b2fc45 maintenance: add support for systemd timers on Linux
@@ Documentation/git-maintenance.txt: OPTIONS
---scheduler=auto|crontab|launchctl|schtasks::
+--scheduler=auto|crontab|systemd-timer|launchctl|schtasks::
When combined with the `start` subcommand, specify the scheduler
- to use to run the hourly, daily and weekly executions of
+ for running the hourly, daily and weekly executions of
`git maintenance run`.
- The possible values for `<scheduler>` depend on the system: `crontab`
-- is available on POSIX systems, `launchctl` is available on
-- MacOS and `schtasks` is available on Windows.
-+ is available on POSIX systems, `systemd-timer` is available on Linux
-+ systems, `launchctl` is available on MacOS and `schtasks` is available
-+ on Windows.
- By default or when `auto` is specified, a suitable scheduler for
- the system is used. On MacOS, `launchctl` is used. On Windows,
-- `schtasks` is used. On all other systems, `crontab` is used.
-+ `schtasks` is used. On Linux, `systemd-timer` is used if user systemd
-+ timers are available, otherwise, `crontab` is used. On all other systems,
-+ `crontab` is used.
+- Possible values for `<scheduler>` are `auto`, `crontab` (POSIX),
+- `launchctl` (macOS), and `schtasks` (Windows).
+- When `auto` is specified, the appropriate platform-specific
+- scheduler is used. Default is `auto`.
++ Possible values for `<scheduler>` are `auto`, `crontab`
++ (POSIX), `systemd-timer` (Linux), `launchctl` (macOS), and
++ `schtasks` (Windows). When `auto` is specified, the
++ appropriate platform-specific scheduler is used; on Linux,
++ `systemd-timer` is used if available, otherwise
++ `crontab`. Default is `auto`.
TROUBLESHOOTING
@@ builtin/gc.c: static enum scheduler parse_scheduler(const char *value)
else if (!strcasecmp(value, "launchctl"))
return SCHEDULER_LAUNCHCTL;
else if (!strcasecmp(value, "schtasks"))
-@@ builtin/gc.c: static void resolve_auto_scheduler(enum scheduler *scheduler)
- *scheduler = SCHEDULER_SCHTASKS;
- return;
+@@ builtin/gc.c: static enum scheduler resolve_scheduler(enum scheduler scheduler)
+ #elif defined(GIT_WINDOWS_NATIVE)
+ return SCHEDULER_SCHTASKS;
+#elif defined(__linux__)
+ if (is_systemd_timer_available())
-+ *scheduler = SCHEDULER_SYSTEMD;
++ return SCHEDULER_SYSTEMD;
+ else if (is_crontab_available())
-+ *scheduler = SCHEDULER_CRON;
++ return SCHEDULER_CRON;
+ else
+ die(_("neither systemd timers nor crontab are available"));
-+ return;
+
#else
- *scheduler = SCHEDULER_CRON;
- return;
+ return SCHEDULER_CRON;
+ #endif
## t/t7900-maintenance.sh ##
@@ t/t7900-maintenance.sh: test_xmllint () {
--
2.32.0
