From: Phillip Wood <phillip.wood123@gmail.com>
To: "Lénaïc Huard" <lenaic@lhuard.fr>, git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
"Derrick Stolee" <dstolee@microsoft.com>,
"Eric Sunshine" <sunshine@sunshineco.com>,
"Đoàn Trần Công Danh" <congdanhqx@gmail.com>,
"Felipe Contreras" <felipe.contreras@gmail.com>,
"Martin Ågren" <martin.agren@gmail.com>,
"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
"Bagas Sanjaya" <bagasdotme@gmail.com>,
"brian m . carlson" <sandals@crustytoothpaste.net>,
"Johannes Schindelin" <Johannes.Schindelin@gmx.de>
Subject: Re: [PATCH v4 3/4] maintenance: `git maintenance run` learned `--scheduler=<scheduler>`
Date: Mon, 24 May 2021 11:12:10 +0100 [thread overview]
Message-ID: <ce0e096d-cd82-800a-9ef5-5bcc4b25046d@gmail.com> (raw)
In-Reply-To: <20210524071538.46862-4-lenaic@lhuard.fr>
Hi Lénaïc
On 24/05/2021 08:15, Lénaïc Huard wrote:
> Depending on the system, different schedulers can be used to schedule
> the hourly, daily and weekly executions of `git maintenance run`:
> * `launchctl` for MacOS,
> * `schtasks` for Windows and
> * `crontab` for everything else.
>
> `git maintenance run` now has an option to let the end-user explicitly
> choose which scheduler he wants to use:
> `--scheduler=auto|crontab|launchctl|schtasks`.
>
> When `git maintenance start --scheduler=XXX` is run, it not only
> registers `git maintenance run` tasks in the scheduler XXX, it also
> removes the `git maintenance run` tasks from all the other schedulers to
> ensure we cannot have two schedulers launching concurrent identical
> tasks.
>
> The default value is `auto` which chooses a suitable scheduler for the
> system.
>
> `git maintenance stop` doesn't have any `--scheduler` parameter because
> this command will try to remove the `git maintenance run` tasks from all
> the available schedulers.
I like this change, it makes the test infrastructure less intrusive.
> Signed-off-by: Lénaïc Huard <lenaic@lhuard.fr>
> ---
> Documentation/git-maintenance.txt | 11 +
> builtin/gc.c | 333 ++++++++++++++++++++++++------
> t/t7900-maintenance.sh | 56 ++++-
> 3 files changed, 333 insertions(+), 67 deletions(-)
>
> diff --git a/Documentation/git-maintenance.txt b/Documentation/git-maintenance.txt
> index 80ddd33ceb..7c4bb38a2f 100644
> --- a/Documentation/git-maintenance.txt
> +++ b/Documentation/git-maintenance.txt
> @@ -181,6 +181,17 @@ OPTIONS
> `maintenance.<task>.enabled` configured as `true` are considered.
> See the 'TASKS' section for the list of accepted `<task>` values.
>
> +--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
> + `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.
> +
>
> TROUBLESHOOTING
> ---------------
> diff --git a/builtin/gc.c b/builtin/gc.c
> index 0caf8d45c4..bf21cec059 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -1544,6 +1544,60 @@ static const char *get_frequency(enum schedule_priority schedule)
> }
> }
>
> +static int get_schedule_cmd(const char **cmd, int *is_available)
> +{
> + char *item;
> + static char test_cmd[32];
> + char *testing = xstrdup_or_null(getenv("GIT_TEST_MAINT_SCHEDULER"));
> +
> + if (!testing)
> + return 0;
> +
> + if (is_available)
> + *is_available = 0;
> +
> + for(item = testing;;) {
> + char *sep;
> + char *end_item = strchr(item, ',');
> + if (end_item)
> + *end_item = '\0';
> +
> + sep = strchr(item, ':');
> + if (!sep)
> + die("GIT_TEST_MAINT_SCHEDULER unparseable: %s", testing);
> + *sep = '\0';
> +
> + if (!strcmp(*cmd, item)) {
> + strlcpy(test_cmd, sep+1, ARRAY_SIZE(test_cmd));
This falls into the trap of assuming strlcpy() is safe and does not
check the return value. It will silently truncate the command if it is
too long. As this is for testing I'd be happy just to return 'sep + 1'
in *cmd and leak it. We could mark 'testing' with UNLEAK() to keep asan
happy.
> + *cmd = test_cmd;
> + if (is_available)
> + *is_available = 1;
> + break;
> + }
> +
> + if (!end_item)
> + break;
> + item = end_item + 1;
> + }
> +
> + free(testing);
> + return 1;
> +}
> +
> +static int is_launchctl_available(void)
> +{
> + const char *cmd = "launchctl";
> + int is_available;
> + if (get_schedule_cmd(&cmd, &is_available))
> + return is_available;
> +
> +#ifdef __APPLE__
> + return 1;
> +#else
> + return 0;
> +#endif
> +}
> +
> static char *launchctl_service_name(const char *frequency)
> {
> struct strbuf label = STRBUF_INIT;
> @@ -1576,12 +1630,14 @@ enum enable_or_disable {
> };
>
> static int launchctl_boot_plist(enum enable_or_disable enable,
> - const char *filename, const char *cmd)
> + const char *filename)
I'm so pleased to see all these 'cmd' arguments disappearing!
> {
> + const char *cmd = "launchctl";
> int result;
> struct child_process child = CHILD_PROCESS_INIT;
> char *uid = launchctl_get_uid();
>
> + get_schedule_cmd(&cmd, NULL);
> strvec_split(&child.args, cmd);
It's a shame we still have to have this strvec_split() call just to
handle testing but your changes are an improvement.
> strvec_pushl(&child.args, enable == ENABLE ? "bootstrap" : "bootout",
> uid, filename, NULL);
> @@ -1598,26 +1654,26 @@ static int launchctl_boot_plist(enum enable_or_disable enable,
> return result;
> }
>
> -static int launchctl_remove_plist(enum schedule_priority schedule, const char *cmd)
> +static int launchctl_remove_plist(enum schedule_priority schedule)
> {
> const char *frequency = get_frequency(schedule);
> char *name = launchctl_service_name(frequency);
> char *filename = launchctl_service_filename(name);
> - int result = launchctl_boot_plist(DISABLE, filename, cmd);
> + int result = launchctl_boot_plist(DISABLE, filename);
> unlink(filename);
> free(filename);
> free(name);
> return result;
> }
>
> -static int launchctl_remove_plists(const char *cmd)
> +static int launchctl_remove_plists(void)
> {
> - return launchctl_remove_plist(SCHEDULE_HOURLY, cmd) ||
> - launchctl_remove_plist(SCHEDULE_DAILY, cmd) ||
> - launchctl_remove_plist(SCHEDULE_WEEKLY, cmd);
> + return launchctl_remove_plist(SCHEDULE_HOURLY) ||
> + launchctl_remove_plist(SCHEDULE_DAILY) ||
> + launchctl_remove_plist(SCHEDULE_WEEKLY);
> }
>
> -static int launchctl_schedule_plist(const char *exec_path, enum schedule_priority schedule, const char *cmd)
> +static int launchctl_schedule_plist(const char *exec_path, enum schedule_priority schedule)
> {
> FILE *plist;
> int i;
> @@ -1686,8 +1742,8 @@ static int launchctl_schedule_plist(const char *exec_path, enum schedule_priorit
> fclose(plist);
>
> /* bootout might fail if not already running, so ignore */
> - launchctl_boot_plist(DISABLE, filename, cmd);
> - if (launchctl_boot_plist(ENABLE, filename, cmd))
> + launchctl_boot_plist(DISABLE, filename);
> + if (launchctl_boot_plist(ENABLE, filename))
> die(_("failed to bootstrap service %s"), filename);
>
> free(filename);
> @@ -1695,28 +1751,42 @@ static int launchctl_schedule_plist(const char *exec_path, enum schedule_priorit
> return 0;
> }
>
> -static int launchctl_add_plists(const char *cmd)
> +static int launchctl_add_plists(void)
> {
> const char *exec_path = git_exec_path();
>
> - return launchctl_schedule_plist(exec_path, SCHEDULE_HOURLY, cmd) ||
> - launchctl_schedule_plist(exec_path, SCHEDULE_DAILY, cmd) ||
> - launchctl_schedule_plist(exec_path, SCHEDULE_WEEKLY, cmd);
> + return launchctl_schedule_plist(exec_path, SCHEDULE_HOURLY) ||
> + launchctl_schedule_plist(exec_path, SCHEDULE_DAILY) ||
> + launchctl_schedule_plist(exec_path, SCHEDULE_WEEKLY);
> }
>
> static int launchctl_update_schedule(enum enable_or_disable run_maintenance,
> - int fd, const char *cmd)
> + int fd)
> {
> switch (run_maintenance) {
> case ENABLE:
> - return launchctl_add_plists(cmd);
> + return launchctl_add_plists();
> case DISABLE:
> - return launchctl_remove_plists(cmd);
> + return launchctl_remove_plists();
> default:
> BUG("invalid enable_or_disable value");
> }
> }
>
> +static int is_schtasks_available(void)
> +{
> + const char *cmd = "schtasks";
> + int is_available;
> + if (get_schedule_cmd(&cmd, &is_available))
> + return is_available;
> +
> +#ifdef GIT_WINDOWS_NATIVE
> + return 1;
> +#else
> + return 0;
> +#endif
> +}
> +
> static char *schtasks_task_name(const char *frequency)
> {
> struct strbuf label = STRBUF_INIT;
> @@ -1724,13 +1794,15 @@ static char *schtasks_task_name(const char *frequency)
> return strbuf_detach(&label, NULL);
> }
>
> -static int schtasks_remove_task(enum schedule_priority schedule, const char *cmd)
> +static int schtasks_remove_task(enum schedule_priority schedule)
> {
> + const char *cmd = "schtasks";
> int result;
> struct strvec args = STRVEC_INIT;
> const char *frequency = get_frequency(schedule);
> char *name = schtasks_task_name(frequency);
>
> + get_schedule_cmd(&cmd, NULL);
> strvec_split(&args, cmd);
> strvec_pushl(&args, "/delete", "/tn", name, "/f", NULL);
>
> @@ -1741,15 +1813,16 @@ static int schtasks_remove_task(enum schedule_priority schedule, const char *cmd
> return result;
> }
>
> -static int schtasks_remove_tasks(const char *cmd)
> +static int schtasks_remove_tasks(void)
> {
> - return schtasks_remove_task(SCHEDULE_HOURLY, cmd) ||
> - schtasks_remove_task(SCHEDULE_DAILY, cmd) ||
> - schtasks_remove_task(SCHEDULE_WEEKLY, cmd);
> + return schtasks_remove_task(SCHEDULE_HOURLY) ||
> + schtasks_remove_task(SCHEDULE_DAILY) ||
> + schtasks_remove_task(SCHEDULE_WEEKLY);
> }
>
> -static int schtasks_schedule_task(const char *exec_path, enum schedule_priority schedule, const char *cmd)
> +static int schtasks_schedule_task(const char *exec_path, enum schedule_priority schedule)
> {
> + const char *cmd = "schtasks";
> int result;
> struct child_process child = CHILD_PROCESS_INIT;
> const char *xml;
> @@ -1758,6 +1831,8 @@ static int schtasks_schedule_task(const char *exec_path, enum schedule_priority
> char *name = schtasks_task_name(frequency);
> struct strbuf tfilename = STRBUF_INIT;
>
> + get_schedule_cmd(&cmd, NULL);
> +
> strbuf_addf(&tfilename, "%s/schedule_%s_XXXXXX",
> get_git_common_dir(), frequency);
> tfile = xmks_tempfile(tfilename.buf);
> @@ -1862,34 +1937,65 @@ static int schtasks_schedule_task(const char *exec_path, enum schedule_priority
> return result;
> }
>
> -static int schtasks_schedule_tasks(const char *cmd)
> +static int schtasks_schedule_tasks(void)
> {
> const char *exec_path = git_exec_path();
>
> - return schtasks_schedule_task(exec_path, SCHEDULE_HOURLY, cmd) ||
> - schtasks_schedule_task(exec_path, SCHEDULE_DAILY, cmd) ||
> - schtasks_schedule_task(exec_path, SCHEDULE_WEEKLY, cmd);
> + return schtasks_schedule_task(exec_path, SCHEDULE_HOURLY) ||
> + schtasks_schedule_task(exec_path, SCHEDULE_DAILY) ||
> + schtasks_schedule_task(exec_path, SCHEDULE_WEEKLY);
> }
>
> static int schtasks_update_schedule(enum enable_or_disable run_maintenance,
> - int fd, const char *cmd)
> + int fd)
> {
> switch (run_maintenance) {
> case ENABLE:
> - return schtasks_schedule_tasks(cmd);
> + return schtasks_schedule_tasks();
> case DISABLE:
> - return schtasks_remove_tasks(cmd);
> + return schtasks_remove_tasks();
> default:
> BUG("invalid enable_or_disable value");
> }
> }
>
> +static int is_crontab_available(void)
> +{
> + const char *cmd = "crontab";
> + int is_available;
> + static int cached_result = -1;
> + struct child_process child = CHILD_PROCESS_INIT;
> +
> + if (cached_result != -1)
> + return cached_result;
> +
> + if (get_schedule_cmd(&cmd, &is_available) && !is_available)
> + return 0;
> +
> + strvec_split(&child.args, cmd);
> + strvec_push(&child.args, "-l");
> + child.no_stdin = 1;
> + child.no_stdout = 1;
> + child.no_stderr = 1;
> + child.silent_exec_failure = 1;
> +
> + if (start_command(&child)) {
> + cached_result = 0;
> + return cached_result;
> + }
> + /* Ignore exit code, as an empty crontab will return error. */
> + finish_command(&child);
> + cached_result = 1;
> + return cached_result;
> +}
> +
> #define BEGIN_LINE "# BEGIN GIT MAINTENANCE SCHEDULE"
> #define END_LINE "# END GIT MAINTENANCE SCHEDULE"
>
> static int crontab_update_schedule(enum enable_or_disable run_maintenance,
> - int fd, const char *cmd)
> + int fd)
> {
> + const char *cmd = "crontab";
> int result = 0;
> int in_old_region = 0;
> struct child_process crontab_list = CHILD_PROCESS_INIT;
> @@ -1897,6 +2003,7 @@ static int crontab_update_schedule(enum enable_or_disable run_maintenance,
> FILE *cron_list, *cron_in;
> struct strbuf line = STRBUF_INIT;
>
> + get_schedule_cmd(&cmd, NULL);
> strvec_split(&crontab_list.args, cmd);
> strvec_push(&crontab_list.args, "-l");
> crontab_list.in = -1;
> @@ -1972,61 +2079,161 @@ static int crontab_update_schedule(enum enable_or_disable run_maintenance,
> return result;
> }
>
> +enum scheduler {
> + SCHEDULER_INVALID = -1,
> + SCHEDULER_AUTO,
> + SCHEDULER_CRON,
> + SCHEDULER_LAUNCHCTL,
> + SCHEDULER_SCHTASKS,
> +};
> +
> +static const struct {
> + const char *name;
> + int (*is_available)(void);
> + int (*update_schedule)(enum enable_or_disable run_maintenance, int fd);
> +} scheduler_fn[] = {
> + [SCHEDULER_CRON] = {
> + .name = "crontab",
> + .is_available = is_crontab_available,
> + .update_schedule = crontab_update_schedule,
> + },
> + [SCHEDULER_LAUNCHCTL] = {
> + .name = "launchctl",
> + .is_available = is_launchctl_available,
> + .update_schedule = launchctl_update_schedule,
> + },
> + [SCHEDULER_SCHTASKS] = {
> + .name = "schtasks",
> + .is_available = is_schtasks_available,
> + .update_schedule = schtasks_update_schedule,
> + },
> +};
> +
> +static enum scheduler parse_scheduler(const char *value)
> +{
> + if (!value)
> + return SCHEDULER_INVALID;
> + else if (!strcasecmp(value, "auto"))
> + return SCHEDULER_AUTO;
> + else if (!strcasecmp(value, "cron") || !strcasecmp(value, "crontab"))
> + return SCHEDULER_CRON;
> + else if (!strcasecmp(value, "launchctl"))
> + return SCHEDULER_LAUNCHCTL;
> + else if (!strcasecmp(value, "schtasks"))
> + return SCHEDULER_SCHTASKS;
> + else
> + return SCHEDULER_INVALID;
> +}
> +
> +static int maintenance_opt_scheduler(const struct option *opt, const char *arg,
> + int unset)
> +{
> + enum scheduler *scheduler = opt->value;
> +
> + BUG_ON_OPT_NEG(unset);
> +
> + *scheduler = parse_scheduler(arg);
> + if (*scheduler == SCHEDULER_INVALID)
> + return error(_("unrecognized --scheduler argument '%s'"), arg);
> + return 0;
> +}
> +
> +struct maintenance_start_opts {
> + enum scheduler scheduler;
> +};
> +
> +static void resolve_auto_scheduler(enum scheduler *scheduler)
> +{
> + if (*scheduler != SCHEDULER_AUTO)
> + return;
> +
> #if defined(__APPLE__)
> -static const char platform_scheduler[] = "launchctl";
> + *scheduler = SCHEDULER_LAUNCHCTL;
> + return;
> +
> #elif defined(GIT_WINDOWS_NATIVE)
> -static const char platform_scheduler[] = "schtasks";
> + *scheduler = SCHEDULER_SCHTASKS;
> + return;
> +
> #else
> -static const char platform_scheduler[] = "crontab";
> + *scheduler = SCHEDULER_CRON;
> + return;
> #endif
> +}
>
> -static int update_background_schedule(int enable)
> +static void validate_scheduler(enum scheduler scheduler)
> {
> - int result;
> - const char *scheduler = platform_scheduler;
> - const char *cmd = scheduler;
> - char *testing;
> + if (scheduler == SCHEDULER_INVALID)
> + BUG("invalid scheduler");
> + if (scheduler == SCHEDULER_AUTO)
> + BUG("resolve_auto_scheduler should have been called before");
> +
> + if (!scheduler_fn[scheduler].is_available())
> + die(_("%s scheduler is not available"),
> + scheduler_fn[scheduler].name);
> +}
> +
> +static int update_background_schedule(const struct maintenance_start_opts *opts,
> + enum enable_or_disable enable)
> +{
> + unsigned int i;
> + int res, result = 0;
> struct lock_file lk;
> char *lock_path = xstrfmt("%s/schedule", the_repository->objects->odb->path);
>
> - testing = xstrdup_or_null(getenv("GIT_TEST_MAINT_SCHEDULER"));
> - if (testing) {
> - char *sep = strchr(testing, ':');
> - if (!sep)
> - die("GIT_TEST_MAINT_SCHEDULER unparseable: %s", testing);
> - *sep = '\0';
> - scheduler = testing;
> - cmd = sep + 1;
> - }
> -
> if (hold_lock_file_for_update(&lk, lock_path, LOCK_NO_DEREF) < 0)
> return error(_("another process is scheduling background maintenance"));
>
> - if (!strcmp(scheduler, "launchctl"))
> - result = launchctl_update_schedule(enable, get_lock_file_fd(&lk), cmd);
> - else if (!strcmp(scheduler, "schtasks"))
> - result = schtasks_update_schedule(enable, get_lock_file_fd(&lk), cmd);
> - else if (!strcmp(scheduler, "crontab"))
> - result = crontab_update_schedule(enable, get_lock_file_fd(&lk), cmd);
> - else
> - die("unknown background scheduler: %s", scheduler);
> + for (i = 1; i < ARRAY_SIZE(scheduler_fn); i++) {
> + enum enable_or_disable enable_scheduler =
> + (enable == ENABLE && (opts->scheduler == i)) ?
Strictly speaking none of the parenthesis are required, I'd certainly
drop the inner ones.
> + ENABLE : DISABLE;
> + if (!scheduler_fn[i].is_available())
> + continue;
> + res = scheduler_fn[i].update_schedule(
> + enable_scheduler, get_lock_file_fd(&lk));
It does not matter for this patch as we're not really disabling anything
with the existing range of schedulers but in the next patch we can end
up enabling the new scheduler before disabling the old one leading to a
race where they can both start 'git maintenance' on the same repo. As I
said in my previous review it would be clearer to disable all the
schedulers first before enabling the new one.
> + if (enable_scheduler)
> + result = res;
> + }
>
> rollback_lock_file(&lk);
> - free(testing);
> return result;
> }
>
> -static int maintenance_start(void)
> +static const char *const builtin_maintenance_start_usage[] = {
> + N_("git maintenance start [--scheduler=<scheduler>]"), NULL
> +};
I'm not sure what the { } and NULL are doing here, it should just be
assigning a string. You could put it inside maintenance_start() and just
call the variable "usage"
> +static int maintenance_start(int argc, const char **argv, const char *prefix)
> {
> + struct maintenance_start_opts opts;
> + struct option builtin_maintenance_start_options[] = {
As this variable is local to the function you could call it something
shorter like options.
> + OPT_CALLBACK_F(
> + 0, "scheduler", &opts.scheduler, N_("scheduler"),
> + N_("scheduler to use to trigger git maintenance run"),
> + PARSE_OPT_NONEG, maintenance_opt_scheduler),
> + OPT_END()
> + };
> + memset(&opts, 0, sizeof(opts));
> +
> + argc = parse_options(argc, argv, prefix,
> + builtin_maintenance_start_options,
> + builtin_maintenance_start_usage, 0);
> + if (argc)
> + usage_with_options(builtin_maintenance_start_usage,
> + builtin_maintenance_start_options);
> +
> + resolve_auto_scheduler(&opts.scheduler);
> + validate_scheduler(opts.scheduler);
> +
> if (maintenance_register())
> warning(_("failed to add repo to global config"));
> -
> - return update_background_schedule(1);
> + return update_background_schedule(&opts, 1);
This conversion got missed in the last patch (that is if we end up
wanting to use an enum).
> }
>
> static int maintenance_stop(void)
> {
> - return update_background_schedule(0);
> + return update_background_schedule(NULL, 0);
> }
>
> static const char builtin_maintenance_usage[] = N_("git maintenance <subcommand> [<options>]");
> @@ -2040,7 +2247,7 @@ int cmd_maintenance(int argc, const char **argv, const char *prefix)
> if (!strcmp(argv[1], "run"))
> return maintenance_run(argc - 1, argv + 1, prefix);
> if (!strcmp(argv[1], "start"))
> - return maintenance_start();
> + return maintenance_start(argc - 1, argv + 1, prefix);
> if (!strcmp(argv[1], "stop"))
> return maintenance_stop();
> if (!strcmp(argv[1], "register"))
> diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
> index 2412d8c5c0..9eac260307 100755
> --- a/t/t7900-maintenance.sh
> +++ b/t/t7900-maintenance.sh
> @@ -488,8 +488,21 @@ test_expect_success !MINGW 'register and unregister with regex metacharacters' '
> maintenance.repo "$(pwd)/$META"
> '
>
> +test_expect_success 'start --scheduler=<scheduler>' '
> + test_expect_code 129 git maintenance start --scheduler=foo 2>err &&
> + test_i18ngrep "unrecognized --scheduler argument" err &&
>
> + test_expect_code 129 git maintenance start --no-scheduler 2>err &&
> + test_i18ngrep "unknown option" err &&
> +
> + test_expect_code 128 \
> + env GIT_TEST_MAINT_SCHEDULER="launchctl:true,schtasks:true" \
> + git maintenance start --scheduler=crontab 2>err &&
> + test_i18ngrep "fatal: crontab scheduler is not available" err
> +'
> +
> test_expect_success 'start from empty cron table' '
> - GIT_TEST_MAINT_SCHEDULER="crontab:test-tool crontab cron.txt" git maintenance start &&
> + GIT_TEST_MAINT_SCHEDULER="crontab:test-tool crontab cron.txt" git maintenance start --scheduler=crontab &&
>
> # start registers the repo
> git config --get --global --fixed-value maintenance.repo "$(pwd)" &&
> @@ -512,7 +525,7 @@ test_expect_success 'stop from existing schedule' '
>
> test_expect_success 'start preserves existing schedule' '
> echo "Important information!" >cron.txt &&
> - GIT_TEST_MAINT_SCHEDULER="crontab:test-tool crontab cron.txt" git maintenance start &&
> + GIT_TEST_MAINT_SCHEDULER="crontab:test-tool crontab cron.txt" git maintenance start --scheduler=crontab &&
> grep "Important information!" cron.txt
> '
>
> @@ -541,7 +554,7 @@ test_expect_success 'start and stop macOS maintenance' '
> EOF
>
> rm -f args &&
> - GIT_TEST_MAINT_SCHEDULER=launchctl:./print-args git maintenance start &&
> + GIT_TEST_MAINT_SCHEDULER=launchctl:./print-args git maintenance start --scheduler=launchctl &&
>
> # start registers the repo
> git config --get --global --fixed-value maintenance.repo "$(pwd)" &&
> @@ -592,7 +605,7 @@ test_expect_success 'start and stop Windows maintenance' '
> EOF
>
> rm -f args &&
> - GIT_TEST_MAINT_SCHEDULER="schtasks:./print-args" git maintenance start &&
> + GIT_TEST_MAINT_SCHEDULER="schtasks:./print-args" git maintenance start --scheduler=schtasks &&
>
> # start registers the repo
> git config --get --global --fixed-value maintenance.repo "$(pwd)" &&
> @@ -615,6 +628,41 @@ test_expect_success 'start and stop Windows maintenance' '
> test_cmp expect args
> '
>
> +test_expect_success 'start and stop when several schedulers are available' '
> + write_script print-args <<-\EOF &&
> + printf "%s\n" "$*" | sed "s:gui/[0-9][0-9]*:gui/[UID]:; s:\(schtasks /create .* /xml\).*:\1:;" >>args
> + EOF
> +
> + rm -f args &&
> + GIT_TEST_MAINT_SCHEDULER="launchctl:./print-args launchctl,schtasks:./print-args schtasks" git maintenance start --scheduler=launchctl &&
> + rm -f expect &&
> + for frequency in hourly daily weekly
> + do
> + PLIST="$pfx/Library/LaunchAgents/org.git-scm.git.$frequency.plist" &&
> + echo "launchctl bootout gui/[UID] $PLIST" >>expect &&
> + echo "launchctl bootstrap gui/[UID] $PLIST" >>expect || return 1
> + done &&
> + printf "schtasks /delete /tn Git Maintenance (%s) /f\n" \
> + hourly daily weekly >>expect &&
> + test_cmp expect args &&
> +
> + rm -f args &&
> + GIT_TEST_MAINT_SCHEDULER="launchctl:./print-args launchctl,schtasks:./print-args schtasks" git maintenance start --scheduler=schtasks &&
> + printf "launchctl bootout gui/[UID] $pfx/Library/LaunchAgents/org.git-scm.git.%s.plist\n" \
> + hourly daily weekly >expect &&
> + printf "schtasks /create /tn Git Maintenance (%s) /f /xml\n" \
> + hourly daily weekly >>expect &&
> + test_cmp expect args &&
> +
> + rm -f args &&
> + GIT_TEST_MAINT_SCHEDULER="launchctl:./print-args launchctl,schtasks:./print-args schtasks" git maintenance stop &&
> + printf "launchctl bootout gui/[UID] $pfx/Library/LaunchAgents/org.git-scm.git.%s.plist\n" \
> + hourly daily weekly >expect &&
> + printf "schtasks /delete /tn Git Maintenance (%s) /f\n" \
> + hourly daily weekly >>expect &&
> + test_cmp expect args
> +'
> +
Thanks for adding tests for the new option and converting the old ones
to use it. I think that with a couple of small changes this patch will
be ready. I've run out of time for now but I'll try and look at the
fourth patch in the next couple of days.
Best Wishes
Phillip
> test_expect_success 'register preserves existing strategy' '
> git config maintenance.strategy none &&
> git maintenance register &&
>
next prev parent reply other threads:[~2021-05-24 10:12 UTC|newest]
Thread overview: 138+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-01 14:52 [PATCH] maintenance: use systemd timers on Linux Lénaïc Huard
2021-05-01 20:02 ` brian m. carlson
2021-05-02 5:28 ` Bagas Sanjaya
2021-05-02 6:49 ` Eric Sunshine
2021-05-02 6:45 ` Eric Sunshine
2021-05-02 14:10 ` Phillip Wood
2021-05-05 12:19 ` Đoàn Trần Công Danh
2021-05-05 14:57 ` Phillip Wood
2021-05-05 12:01 ` Ævar Arnfjörð Bjarmason
2021-05-09 22:34 ` Lénaïc Huard
2021-05-10 13:03 ` Ævar Arnfjörð Bjarmason
2021-05-02 11:12 ` Bagas Sanjaya
2021-05-03 12:04 ` Derrick Stolee
2021-05-09 21:32 ` [PATCH v2 0/1] " Lénaïc Huard
2021-05-09 21:32 ` [PATCH v2 1/1] " Lénaïc Huard
2021-05-10 1:20 ` Đoàn Trần Công Danh
2021-05-10 2:48 ` Eric Sunshine
2021-05-10 6:25 ` Junio C Hamano
2021-05-12 0:29 ` Đoàn Trần Công Danh
2021-05-12 6:59 ` Felipe Contreras
2021-05-12 13:26 ` Phillip Wood
2021-05-12 13:38 ` Phillip Wood
2021-05-12 15:41 ` Đoàn Trần Công Danh
2021-05-10 18:03 ` Phillip Wood
2021-05-10 18:25 ` Eric Sunshine
2021-05-10 20:09 ` Phillip Wood
2021-05-10 20:52 ` Eric Sunshine
2021-06-08 14:55 ` Lénaïc Huard
2021-05-10 19:15 ` Martin Ågren
2021-05-11 14:50 ` Phillip Wood
2021-05-11 17:31 ` Derrick Stolee
2021-05-20 22:13 ` [PATCH v3 0/4] " Lénaïc Huard
2021-05-20 22:13 ` [PATCH v3 1/4] cache.h: rename "xdg_config_home" to "xdg_config_home_git" Lénaïc Huard
2021-05-20 23:44 ` Đoàn Trần Công Danh
2021-05-20 22:13 ` [PATCH v3 2/4] maintenance: introduce ENABLE/DISABLE for code clarity Lénaïc Huard
2021-05-20 22:13 ` [PATCH v3 3/4] maintenance: `git maintenance run` learned `--scheduler=<scheduler>` Lénaïc Huard
2021-05-21 9:52 ` Bagas Sanjaya
2021-05-20 22:13 ` [PATCH v3 4/4] maintenance: optionally use systemd timers on Linux Lénaïc Huard
2021-05-21 9:59 ` Bagas Sanjaya
2021-05-21 16:59 ` Derrick Stolee
2021-05-22 6:57 ` Johannes Schindelin
2021-05-23 18:36 ` Felipe Contreras
2021-05-23 23:27 ` brian m. carlson
2021-05-24 1:18 ` Felipe Contreras
2021-05-24 7:03 ` Ævar Arnfjörð Bjarmason
2021-05-24 15:51 ` Junio C Hamano
2021-05-25 1:50 ` Johannes Schindelin
2021-05-25 11:13 ` Felipe Contreras
2021-05-26 10:29 ` CoC, inclusivity etc. (was "Re: [...] systemd timers on Linux") Ævar Arnfjörð Bjarmason
2021-05-26 16:05 ` Felipe Contreras
2021-05-27 14:24 ` Jeff King
2021-05-27 17:30 ` Felipe Contreras
2021-05-27 23:58 ` Junio C Hamano
2021-05-28 14:44 ` Phillip Susi
2021-05-30 21:58 ` Jeff King
2021-05-24 17:52 ` [PATCH v3 4/4] maintenance: optionally use systemd timers on Linux Felipe Contreras
2021-05-24 7:15 ` [PATCH v4 0/4] add support for " Lénaïc Huard
2021-05-24 7:15 ` [PATCH v4 1/4] cache.h: Introduce a generic "xdg_config_home_for(…)" function Lénaïc Huard
2021-05-24 9:33 ` Phillip Wood
2021-05-24 12:23 ` Đoàn Trần Công Danh
2021-05-24 7:15 ` [PATCH v4 2/4] maintenance: introduce ENABLE/DISABLE for code clarity Lénaïc Huard
2021-05-24 9:41 ` Phillip Wood
2021-05-24 12:36 ` Đoàn Trần Công Danh
2021-05-25 7:18 ` Lénaïc Huard
2021-05-25 8:02 ` Junio C Hamano
2021-05-24 9:47 ` Ævar Arnfjörð Bjarmason
2021-05-24 7:15 ` [PATCH v4 3/4] maintenance: `git maintenance run` learned `--scheduler=<scheduler>` Lénaïc Huard
2021-05-24 10:12 ` Phillip Wood [this message]
2021-05-30 6:39 ` Lénaïc Huard
2021-05-30 10:16 ` Phillip Wood
2021-05-24 7:15 ` [PATCH v4 4/4] maintenance: add support for systemd timers on Linux Lénaïc Huard
2021-05-24 9:55 ` Ævar Arnfjörð Bjarmason
2021-05-24 16:39 ` Eric Sunshine
2021-05-24 18:08 ` Felipe Contreras
2021-05-26 10:26 ` Phillip Wood
2021-05-24 9:04 ` [PATCH v4 0/4] " Junio C Hamano
2021-06-08 13:39 ` [PATCH v5 0/3] " Lénaïc Huard
2021-06-08 13:39 ` [PATCH v5 1/3] cache.h: Introduce a generic "xdg_config_home_for(…)" function Lénaïc Huard
2021-06-08 13:39 ` [PATCH v5 2/3] maintenance: `git maintenance run` learned `--scheduler=<scheduler>` Lénaïc Huard
2021-06-08 13:40 ` [PATCH v5 3/3] maintenance: add support for systemd timers on Linux Lénaïc Huard
2021-06-09 9:34 ` Jeff King
2021-06-09 15:01 ` Phillip Wood
2021-06-09 0:21 ` [PATCH v5 0/3] " Junio C Hamano
2021-06-09 14:54 ` Phillip Wood
2021-06-12 16:50 ` [PATCH v6 0/3] maintenance: " Lénaïc Huard
2021-06-12 16:50 ` [PATCH v6 1/3] cache.h: Introduce a generic "xdg_config_home_for(…)" function Lénaïc Huard
2021-06-12 16:50 ` [PATCH v6 2/3] maintenance: `git maintenance run` learned `--scheduler=<scheduler>` Lénaïc Huard
2021-06-14 4:36 ` Eric Sunshine
2021-06-16 18:12 ` Derrick Stolee
2021-06-17 4:11 ` Eric Sunshine
2021-06-17 14:26 ` Phillip Wood
2021-07-02 15:04 ` Lénaïc Huard
2021-06-12 16:50 ` [PATCH v6 3/3] maintenance: add support for systemd timers on Linux Lénaïc Huard
2021-07-02 14:25 ` [PATCH v7 0/3] " Lénaïc Huard
2021-07-02 14:25 ` [PATCH v7 1/3] cache.h: Introduce a generic "xdg_config_home_for(…)" function Lénaïc Huard
2021-07-02 14:25 ` [PATCH v7 2/3] maintenance: `git maintenance run` learned `--scheduler=<scheduler>` Lénaïc Huard
2021-07-06 19:56 ` Ævar Arnfjörð Bjarmason
2021-07-06 20:52 ` Junio C Hamano
2021-07-13 0:15 ` Jeff King
2021-07-13 2:22 ` Eric Sunshine
2021-07-13 3:56 ` Jeff King
2021-07-13 5:17 ` Eric Sunshine
2021-07-13 7:04 ` Bagas Sanjaya
2021-07-06 21:18 ` Felipe Contreras
2021-08-23 20:06 ` Lénaïc Huard
2021-08-23 22:30 ` Junio C Hamano
2021-07-02 14:25 ` [PATCH v7 3/3] maintenance: add support for systemd timers on Linux Lénaïc Huard
2021-07-06 20:03 ` Ævar Arnfjörð Bjarmason
2021-07-02 18:18 ` [PATCH v7 0/3] " Junio C Hamano
2021-07-06 13:18 ` Phillip Wood
2021-08-23 20:40 ` [PATCH v8 " Lénaïc Huard
2021-08-23 20:40 ` [PATCH v8 1/3] cache.h: Introduce a generic "xdg_config_home_for(…)" function Lénaïc Huard
2021-08-23 20:40 ` [PATCH v8 2/3] maintenance: `git maintenance run` learned `--scheduler=<scheduler>` Lénaïc Huard
2021-08-24 17:45 ` Derrick Stolee
2021-08-23 20:40 ` [PATCH v8 3/3] maintenance: add support for systemd timers on Linux Lénaïc Huard
2021-08-24 17:45 ` Derrick Stolee
2021-08-24 17:47 ` [PATCH v8 0/3] " Derrick Stolee
2021-08-27 21:02 ` [PATCH v9 " Lénaïc Huard
2021-08-27 21:02 ` [PATCH v9 1/3] cache.h: Introduce a generic "xdg_config_home_for(…)" function Lénaïc Huard
2021-08-27 21:02 ` [PATCH v9 2/3] maintenance: `git maintenance run` learned `--scheduler=<scheduler>` Lénaïc Huard
2021-08-27 23:54 ` Ramsay Jones
2021-08-27 21:02 ` [PATCH v9 3/3] maintenance: add support for systemd timers on Linux Lénaïc Huard
2021-09-04 20:54 ` [PATCH v10 0/3] " Lénaïc Huard
2021-09-04 20:54 ` [PATCH v10 1/3] cache.h: Introduce a generic "xdg_config_home_for(…)" function Lénaïc Huard
2021-09-04 20:54 ` [PATCH v10 2/3] maintenance: `git maintenance run` learned `--scheduler=<scheduler>` Lénaïc Huard
2021-09-04 20:55 ` [PATCH v10 3/3] maintenance: add support for systemd timers on Linux Lénaïc Huard
2021-09-07 16:48 ` [PATCH v10 0/3] " Derrick Stolee
2021-09-08 11:44 ` Derrick Stolee
2021-09-09 5:52 ` Lénaïc Huard
2021-09-09 19:55 ` Derrick Stolee
2021-09-27 12:50 ` Ævar Arnfjörð Bjarmason
2021-09-27 21:44 ` Lénaïc Huard
2021-08-17 17:22 ` [PATCH v5 0/3] " Derrick Stolee
2021-08-17 19:43 ` Phillip Wood
2021-08-17 20:29 ` Derrick Stolee
2021-08-18 5:56 ` Lénaïc Huard
2021-08-18 13:28 ` Derrick Stolee
2021-08-18 18:23 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ce0e096d-cd82-800a-9ef5-5bcc4b25046d@gmail.com \
--to=phillip.wood123@gmail.com \
--cc=Johannes.Schindelin@gmx.de \
--cc=avarab@gmail.com \
--cc=bagasdotme@gmail.com \
--cc=congdanhqx@gmail.com \
--cc=dstolee@microsoft.com \
--cc=felipe.contreras@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=lenaic@lhuard.fr \
--cc=martin.agren@gmail.com \
--cc=sandals@crustytoothpaste.net \
--cc=sunshine@sunshineco.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).