From 95b5039f5bda585b3872943e78cb8b136903aa5e Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 3 Jun 2025 16:01:09 +0200 Subject: [PATCH 01/12] builtin/gc: use designated field initializers for maintenance tasks Convert the array of maintenance tasks to use designated field initializers. This makes it easier to add more fields to the struct without having to modify all tasks. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/gc.c | 54 ++++++++++++++++++++++++++-------------------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index e33ba946e4..54fc7f299a 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1550,49 +1550,49 @@ enum maintenance_task_label { static struct maintenance_task tasks[] = { [TASK_PREFETCH] = { - "prefetch", - maintenance_task_prefetch, + .name = "prefetch", + .fn = maintenance_task_prefetch, }, [TASK_LOOSE_OBJECTS] = { - "loose-objects", - maintenance_task_loose_objects, - loose_object_auto_condition, + .name = "loose-objects", + .fn = maintenance_task_loose_objects, + .auto_condition = loose_object_auto_condition, }, [TASK_INCREMENTAL_REPACK] = { - "incremental-repack", - maintenance_task_incremental_repack, - incremental_repack_auto_condition, + .name = "incremental-repack", + .fn = maintenance_task_incremental_repack, + .auto_condition = incremental_repack_auto_condition, }, [TASK_GC] = { - "gc", - maintenance_task_gc, - need_to_gc, - 1, + .name = "gc", + .fn = maintenance_task_gc, + .auto_condition = need_to_gc, + .enabled = 1, }, [TASK_COMMIT_GRAPH] = { - "commit-graph", - maintenance_task_commit_graph, - should_write_commit_graph, + .name = "commit-graph", + .fn = maintenance_task_commit_graph, + .auto_condition = should_write_commit_graph, }, [TASK_PACK_REFS] = { - "pack-refs", - maintenance_task_pack_refs, - pack_refs_condition, + .name = "pack-refs", + .fn = maintenance_task_pack_refs, + .auto_condition = pack_refs_condition, }, [TASK_REFLOG_EXPIRE] = { - "reflog-expire", - maintenance_task_reflog_expire, - reflog_expire_condition, + .name = "reflog-expire", + .fn = maintenance_task_reflog_expire, + .auto_condition = reflog_expire_condition, }, [TASK_WORKTREE_PRUNE] = { - "worktree-prune", - maintenance_task_worktree_prune, - worktree_prune_condition, + .name = "worktree-prune", + .fn = maintenance_task_worktree_prune, + .auto_condition = worktree_prune_condition, }, [TASK_RERERE_GC] = { - "rerere-gc", - maintenance_task_rerere_gc, - rerere_gc_condition, + .name = "rerere-gc", + .fn = maintenance_task_rerere_gc, + .auto_condition = rerere_gc_condition, }, }; From bd19b94a664ee6434a5f1b9d65f3d3ee70a9188c Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 3 Jun 2025 16:01:10 +0200 Subject: [PATCH 02/12] builtin/gc: drop redundant local variable We have two different variables that track the quietness for git-gc(1): - The local variable `quiet`, which we wire up. - The `quiet` field of `struct maintenance_run_opts`. This leads to confusion which of these variables should be used and what the respective effect is. Simplify this logic by dropping the local variable in favor of the options field. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/gc.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index 54fc7f299a..7adda8d2d0 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -818,7 +818,6 @@ int cmd_gc(int argc, struct repository *repo UNUSED) { int aggressive = 0; - int quiet = 0; int force = 0; const char *name; pid_t pid; @@ -831,7 +830,7 @@ int cmd_gc(int argc, const char *prune_expire_arg = prune_expire_sentinel; int ret; struct option builtin_gc_options[] = { - OPT__QUIET(&quiet, N_("suppress progress reporting")), + OPT__QUIET(&opts.quiet, N_("suppress progress reporting")), { .type = OPTION_STRING, .long_name = "prune", @@ -891,7 +890,7 @@ int cmd_gc(int argc, if (cfg.aggressive_window > 0) strvec_pushf(&repack, "--window=%d", cfg.aggressive_window); } - if (quiet) + if (opts.quiet) strvec_push(&repack, "-q"); if (opts.auto_flag) { @@ -906,7 +905,7 @@ int cmd_gc(int argc, goto out; } - if (!quiet) { + if (!opts.quiet) { if (opts.detach > 0) fprintf(stderr, _("Auto packing the repository in background for optimum performance.\n")); else @@ -991,7 +990,7 @@ int cmd_gc(int argc, strvec_pushl(&prune_cmd.args, "prune", "--expire", NULL); /* run `git prune` even if using cruft packs */ strvec_push(&prune_cmd.args, cfg.prune_expire); - if (quiet) + if (opts.quiet) strvec_push(&prune_cmd.args, "--no-progress"); if (repo_has_promisor_remote(the_repository)) strvec_push(&prune_cmd.args, @@ -1019,7 +1018,7 @@ int cmd_gc(int argc, if (the_repository->settings.gc_write_commit_graph == 1) write_commit_graph_reachable(the_repository->objects->odb, - !quiet && !daemonized ? COMMIT_GRAPH_WRITE_PROGRESS : 0, + !opts.quiet && !daemonized ? COMMIT_GRAPH_WRITE_PROGRESS : 0, NULL); if (opts.auto_flag && too_many_loose_objects(&cfg)) From 1bb6bdb646583a2fc2e0e6436f5cdabdd4d14189 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 3 Jun 2025 16:01:11 +0200 Subject: [PATCH 03/12] builtin/maintenance: centralize configuration of explicit tasks Users of git-maintenance(1) can explicitly ask it to run specific tasks by passing the `--task=` command line option. This option can be passed multiple times, which causes us to execute tasks in the same order as the tasks have been provided by the user. The order in which tasks are run is computed in `task_option_parse()`: every time we parse such a command line argument, we modify the global array of tasks by seting the selected index for that specific task. This has two downsides: - We modify global state, which makes it hard to follow the logic. - The configuration of tasks is split across multiple different functions, so it is not easy to figure out the different factors that play a role in selecting tasks. Refactor the logic so that `task_option_parse()` does not modify global state anymore. Instead, this function now only collects the list of configured tasks. The logic to configure ordering of the respective tasks is then deferred to `initialize_task_config()`. This refactoring solves the second problem, that the configuration of tasks is spread across multiple different locations. The first problem, that we modify global state, will be fixed in a subsequent commit. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/gc.c | 47 ++++++++++++++++++++++++----------------------- 1 file changed, 24 insertions(+), 23 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index 7adda8d2d0..c4af9b1128 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1690,15 +1690,22 @@ static void initialize_maintenance_strategy(void) } } -static void initialize_task_config(int schedule) +static void initialize_task_config(const struct string_list *selected_tasks, + int schedule) { - int i; struct strbuf config_name = STRBUF_INIT; + for (size_t i = 0; i < TASK__COUNT; i++) + tasks[i].selected_order = -1; + for (size_t i = 0; i < selected_tasks->nr; i++) { + struct maintenance_task *task = selected_tasks->items[i].util; + task->selected_order = i; + } + if (schedule) initialize_maintenance_strategy(); - for (i = 0; i < TASK__COUNT; i++) { + for (size_t i = 0; i < TASK__COUNT; i++) { int config_value; char *config_str; @@ -1722,33 +1729,28 @@ static void initialize_task_config(int schedule) strbuf_release(&config_name); } -static int task_option_parse(const struct option *opt UNUSED, +static int task_option_parse(const struct option *opt, const char *arg, int unset) { - int i, num_selected = 0; - struct maintenance_task *task = NULL; + struct string_list *selected_tasks = opt->value; + size_t i; BUG_ON_OPT_NEG(unset); - for (i = 0; i < TASK__COUNT; i++) { - if (tasks[i].selected_order >= 0) - num_selected++; - if (!strcasecmp(tasks[i].name, arg)) { - task = &tasks[i]; - } - } - - if (!task) { + for (i = 0; i < TASK__COUNT; i++) + if (!strcasecmp(tasks[i].name, arg)) + break; + if (i >= TASK__COUNT) { error(_("'%s' is not a valid task"), arg); return 1; } - if (task->selected_order >= 0) { + if (unsorted_string_list_has_string(selected_tasks, arg)) { error(_("task '%s' cannot be selected multiple times"), arg); return 1; } - task->selected_order = num_selected + 1; + string_list_append(selected_tasks, arg)->util = &tasks[i]; return 0; } @@ -1756,8 +1758,8 @@ static int task_option_parse(const struct option *opt UNUSED, static int maintenance_run(int argc, const char **argv, const char *prefix, struct repository *repo UNUSED) { - int i; struct maintenance_run_opts opts = MAINTENANCE_RUN_OPTS_INIT; + struct string_list selected_tasks = STRING_LIST_INIT_DUP; struct gc_config cfg = GC_CONFIG_INIT; struct option builtin_maintenance_run_options[] = { OPT_BOOL(0, "auto", &opts.auto_flag, @@ -1769,7 +1771,7 @@ static int maintenance_run(int argc, const char **argv, const char *prefix, maintenance_opt_schedule), OPT_BOOL(0, "quiet", &opts.quiet, N_("do not report progress or other information over stderr")), - OPT_CALLBACK_F(0, "task", NULL, N_("task"), + OPT_CALLBACK_F(0, "task", &selected_tasks, N_("task"), N_("run a specific task"), PARSE_OPT_NONEG, task_option_parse), OPT_END() @@ -1778,9 +1780,6 @@ static int maintenance_run(int argc, const char **argv, const char *prefix, opts.quiet = !isatty(2); - for (i = 0; i < TASK__COUNT; i++) - tasks[i].selected_order = -1; - argc = parse_options(argc, argv, prefix, builtin_maintenance_run_options, builtin_maintenance_run_usage, @@ -1790,13 +1789,15 @@ static int maintenance_run(int argc, const char **argv, const char *prefix, die(_("use at most one of --auto and --schedule=")); gc_config(&cfg); - initialize_task_config(opts.schedule); + initialize_task_config(&selected_tasks, opts.schedule); if (argc != 0) usage_with_options(builtin_maintenance_run_usage, builtin_maintenance_run_options); ret = maintenance_run_tasks(&opts, &cfg); + + string_list_clear(&selected_tasks, 0); gc_config_release(&cfg); return ret; } From a7c86d328ffe2d93cb1bfaf557dba7a2034ec17b Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 3 Jun 2025 16:01:12 +0200 Subject: [PATCH 04/12] builtin/maintenance: mark "--task=" and "--schedule=" as incompatible The "--task=" option explicitly allows the user to say which maintenance tasks should be run, whereas "--schedule=" only respects the maintenance strategy configured for a specific repository. As such, it is not sensible to accept both options at the same time. Mark them as incompatible with one another. While at it, also convert the existing logic that marks "--auto" and "--schedule=" as incompatible to use `die_for_incompatible_opt2()`. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/gc.c | 6 ++++-- t/t7900-maintenance.sh | 7 ++++++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index c4af9b1128..57d7602596 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1785,8 +1785,10 @@ static int maintenance_run(int argc, const char **argv, const char *prefix, builtin_maintenance_run_usage, PARSE_OPT_STOP_AT_NON_OPTION); - if (opts.auto_flag && opts.schedule) - die(_("use at most one of --auto and --schedule=")); + die_for_incompatible_opt2(opts.auto_flag, "--auto", + opts.schedule, "--schedule="); + die_for_incompatible_opt2(selected_tasks.nr, "--task=", + opts.schedule, "--schedule="); gc_config(&cfg); initialize_task_config(&selected_tasks, opts.schedule); diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh index 8cf89e285f..1ada524660 100755 --- a/t/t7900-maintenance.sh +++ b/t/t7900-maintenance.sh @@ -610,7 +610,12 @@ test_expect_success 'rerere-gc task with --auto honors maintenance.rerere-gc.aut test_expect_success '--auto and --schedule incompatible' ' test_must_fail git maintenance run --auto --schedule=daily 2>err && - test_grep "at most one" err + test_grep "cannot be used together" err +' + +test_expect_success '--task and --schedule incompatible' ' + test_must_fail git maintenance run --task=pack-refs --schedule=daily 2>err && + test_grep "cannot be used together" err ' test_expect_success 'invalid --schedule value' ' From 38a8fa5a9a3a74dd606ba37702689a8b07c084bf Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 3 Jun 2025 16:01:13 +0200 Subject: [PATCH 05/12] builtin/maintenance: stop modifying global array of tasks When configuring maintenance tasks run by git-maintenance(1) we do so by modifying the global array of tasks directly. This is already quite bad on its own, as global state makes for logic that is hard to follow. Even more importantly though we use multiple different fields to track whether or not a task should be run: - "enabled" tracks the "maintenance.*.enabled" config key. This field disables execution of a task, unless the user has explicitly asked for the task. - "selected_order" tracks the order in which jobs have been asked for by the user via the "--task=" command line option. It overrides everything else, but only has an effect if at least one job has been selected. - "schedule" tracks the schedule priority for a job, that is how often it should run. This field only plays a role when the user has passed the "--schedule=" command line option. All of this makes it non-trivial to figure out which job really should be running right now. The logic to configure these fields and the logic that interprets them is distributed across multiple functions, making it even harder to follow it. Refactor the logic so that we stop modifying global state. Instead, we now compute which jobs should be run in `initialize_task_config()`, represented as an array of jobs to run that is stored in the options structure. Like this, all logic becomes self-contained and any users of this array only need to iterate through the tasks and execute them one by one. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/gc.c | 208 ++++++++++++++++++++++++++++----------------------- 1 file changed, 113 insertions(+), 95 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index 57d7602596..4d636237ca 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -251,7 +251,24 @@ static enum schedule_priority parse_schedule(const char *value) return SCHEDULE_NONE; } +enum maintenance_task_label { + TASK_PREFETCH, + TASK_LOOSE_OBJECTS, + TASK_INCREMENTAL_REPACK, + TASK_GC, + TASK_COMMIT_GRAPH, + TASK_PACK_REFS, + TASK_REFLOG_EXPIRE, + TASK_WORKTREE_PRUNE, + TASK_RERERE_GC, + + /* Leave as final value */ + TASK__COUNT +}; + struct maintenance_run_opts { + enum maintenance_task_label *tasks; + size_t tasks_nr, tasks_alloc; int auto_flag; int detach; int quiet; @@ -261,6 +278,11 @@ struct maintenance_run_opts { .detach = -1, \ } +static void maintenance_run_opts_release(struct maintenance_run_opts *opts) +{ + free(opts->tasks); +} + static int pack_refs_condition(UNUSED struct gc_config *cfg) { /* @@ -1032,6 +1054,7 @@ int cmd_gc(int argc, } out: + maintenance_run_opts_release(&opts); gc_config_release(&cfg); return 0; } @@ -1524,30 +1547,9 @@ struct maintenance_task { const char *name; maintenance_task_fn *fn; maintenance_auto_fn *auto_condition; - unsigned enabled:1; - - enum schedule_priority schedule; - - /* -1 if not selected. */ - int selected_order; }; -enum maintenance_task_label { - TASK_PREFETCH, - TASK_LOOSE_OBJECTS, - TASK_INCREMENTAL_REPACK, - TASK_GC, - TASK_COMMIT_GRAPH, - TASK_PACK_REFS, - TASK_REFLOG_EXPIRE, - TASK_WORKTREE_PRUNE, - TASK_RERERE_GC, - - /* Leave as final value */ - TASK__COUNT -}; - -static struct maintenance_task tasks[] = { +static const struct maintenance_task tasks[] = { [TASK_PREFETCH] = { .name = "prefetch", .fn = maintenance_task_prefetch, @@ -1566,7 +1568,6 @@ static struct maintenance_task tasks[] = { .name = "gc", .fn = maintenance_task_gc, .auto_condition = need_to_gc, - .enabled = 1, }, [TASK_COMMIT_GRAPH] = { .name = "commit-graph", @@ -1595,18 +1596,9 @@ static struct maintenance_task tasks[] = { }, }; -static int compare_tasks_by_selection(const void *a_, const void *b_) -{ - const struct maintenance_task *a = a_; - const struct maintenance_task *b = b_; - - return b->selected_order - a->selected_order; -} - static int maintenance_run_tasks(struct maintenance_run_opts *opts, struct gc_config *cfg) { - int i, found_selected = 0; int result = 0; struct lock_file lk; struct repository *r = the_repository; @@ -1635,95 +1627,120 @@ static int maintenance_run_tasks(struct maintenance_run_opts *opts, trace2_region_leave("maintenance", "detach", the_repository); } - for (i = 0; !found_selected && i < TASK__COUNT; i++) - found_selected = tasks[i].selected_order >= 0; - - if (found_selected) - QSORT(tasks, TASK__COUNT, compare_tasks_by_selection); - - for (i = 0; i < TASK__COUNT; i++) { - if (found_selected && tasks[i].selected_order < 0) - continue; - - if (!found_selected && !tasks[i].enabled) - continue; - + for (size_t i = 0; i < opts->tasks_nr; i++) { if (opts->auto_flag && - (!tasks[i].auto_condition || - !tasks[i].auto_condition(cfg))) + (!tasks[opts->tasks[i]].auto_condition || + !tasks[opts->tasks[i]].auto_condition(cfg))) continue; - if (opts->schedule && tasks[i].schedule < opts->schedule) - continue; - - trace2_region_enter("maintenance", tasks[i].name, r); - if (tasks[i].fn(opts, cfg)) { - error(_("task '%s' failed"), tasks[i].name); + trace2_region_enter("maintenance", tasks[opts->tasks[i]].name, r); + if (tasks[opts->tasks[i]].fn(opts, cfg)) { + error(_("task '%s' failed"), tasks[opts->tasks[i]].name); result = 1; } - trace2_region_leave("maintenance", tasks[i].name, r); + trace2_region_leave("maintenance", tasks[opts->tasks[i]].name, r); } rollback_lock_file(&lk); return result; } -static void initialize_maintenance_strategy(void) -{ - const char *config_str; +struct maintenance_strategy { + struct { + int enabled; + enum schedule_priority schedule; + } tasks[TASK__COUNT]; +}; - if (git_config_get_string_tmp("maintenance.strategy", &config_str)) - return; +static const struct maintenance_strategy none_strategy = { 0 }; +static const struct maintenance_strategy default_strategy = { + .tasks = { + [TASK_GC].enabled = 1, + }, +}; +static const struct maintenance_strategy incremental_strategy = { + .tasks = { + [TASK_COMMIT_GRAPH].enabled = 1, + [TASK_COMMIT_GRAPH].schedule = SCHEDULE_HOURLY, + [TASK_PREFETCH].enabled = 1, + [TASK_PREFETCH].schedule = SCHEDULE_HOURLY, + [TASK_INCREMENTAL_REPACK].enabled = 1, + [TASK_INCREMENTAL_REPACK].schedule = SCHEDULE_DAILY, + [TASK_LOOSE_OBJECTS].enabled = 1, + [TASK_LOOSE_OBJECTS].schedule = SCHEDULE_DAILY, + [TASK_PACK_REFS].enabled = 1, + [TASK_PACK_REFS].schedule = SCHEDULE_WEEKLY, + }, +}; - if (!strcasecmp(config_str, "incremental")) { - tasks[TASK_GC].schedule = SCHEDULE_NONE; - tasks[TASK_COMMIT_GRAPH].enabled = 1; - tasks[TASK_COMMIT_GRAPH].schedule = SCHEDULE_HOURLY; - tasks[TASK_PREFETCH].enabled = 1; - tasks[TASK_PREFETCH].schedule = SCHEDULE_HOURLY; - tasks[TASK_INCREMENTAL_REPACK].enabled = 1; - tasks[TASK_INCREMENTAL_REPACK].schedule = SCHEDULE_DAILY; - tasks[TASK_LOOSE_OBJECTS].enabled = 1; - tasks[TASK_LOOSE_OBJECTS].schedule = SCHEDULE_DAILY; - tasks[TASK_PACK_REFS].enabled = 1; - tasks[TASK_PACK_REFS].schedule = SCHEDULE_WEEKLY; - } -} - -static void initialize_task_config(const struct string_list *selected_tasks, - int schedule) +static void initialize_task_config(struct maintenance_run_opts *opts, + const struct string_list *selected_tasks) { struct strbuf config_name = STRBUF_INIT; + struct maintenance_strategy strategy; + const char *config_str; - for (size_t i = 0; i < TASK__COUNT; i++) - tasks[i].selected_order = -1; - for (size_t i = 0; i < selected_tasks->nr; i++) { - struct maintenance_task *task = selected_tasks->items[i].util; - task->selected_order = i; + /* + * In case the user has asked us to run tasks explicitly we only use + * those specified tasks. Specifically, we do _not_ want to consult the + * config or maintenance strategy. + */ + if (selected_tasks->nr) { + for (size_t i = 0; i < selected_tasks->nr; i++) { + enum maintenance_task_label label = (intptr_t)selected_tasks->items[i].util;; + ALLOC_GROW(opts->tasks, opts->tasks_nr + 1, opts->tasks_alloc); + opts->tasks[opts->tasks_nr++] = label; + } + + return; } - if (schedule) - initialize_maintenance_strategy(); + /* + * Otherwise, the strategy depends on whether we run as part of a + * scheduled job or not: + * + * - Scheduled maintenance does not perform any housekeeping by + * default, but requires the user to pick a maintenance strategy. + * + * - Unscheduled maintenance uses our default strategy. + * + * Both of these are affected by the gitconfig though, which may + * override specific aspects of our strategy. + */ + if (opts->schedule) { + strategy = none_strategy; + + if (!git_config_get_string_tmp("maintenance.strategy", &config_str)) { + if (!strcasecmp(config_str, "incremental")) + strategy = incremental_strategy; + } + } else { + strategy = default_strategy; + } for (size_t i = 0; i < TASK__COUNT; i++) { int config_value; - char *config_str; strbuf_reset(&config_name); strbuf_addf(&config_name, "maintenance.%s.enabled", tasks[i].name); - if (!git_config_get_bool(config_name.buf, &config_value)) - tasks[i].enabled = config_value; + strategy.tasks[i].enabled = config_value; + if (!strategy.tasks[i].enabled) + continue; - strbuf_reset(&config_name); - strbuf_addf(&config_name, "maintenance.%s.schedule", - tasks[i].name); - - if (!git_config_get_string(config_name.buf, &config_str)) { - tasks[i].schedule = parse_schedule(config_str); - free(config_str); + if (opts->schedule) { + strbuf_reset(&config_name); + strbuf_addf(&config_name, "maintenance.%s.schedule", + tasks[i].name); + if (!git_config_get_string_tmp(config_name.buf, &config_str)) + strategy.tasks[i].schedule = parse_schedule(config_str); + if (strategy.tasks[i].schedule < opts->schedule) + continue; } + + ALLOC_GROW(opts->tasks, opts->tasks_nr + 1, opts->tasks_alloc); + opts->tasks[opts->tasks_nr++] = i; } strbuf_release(&config_name); @@ -1750,7 +1767,7 @@ static int task_option_parse(const struct option *opt, return 1; } - string_list_append(selected_tasks, arg)->util = &tasks[i]; + string_list_append(selected_tasks, arg)->util = (void *)(intptr_t)i; return 0; } @@ -1791,7 +1808,7 @@ static int maintenance_run(int argc, const char **argv, const char *prefix, opts.schedule, "--schedule="); gc_config(&cfg); - initialize_task_config(&selected_tasks, opts.schedule); + initialize_task_config(&opts, &selected_tasks); if (argc != 0) usage_with_options(builtin_maintenance_run_usage, @@ -1800,6 +1817,7 @@ static int maintenance_run(int argc, const char **argv, const char *prefix, ret = maintenance_run_tasks(&opts, &cfg); string_list_clear(&selected_tasks, 0); + maintenance_run_opts_release(&opts); gc_config_release(&cfg); return ret; } From 2aa9ee7eecc696fe93a8f00b0eb2369de5279aaa Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 3 Jun 2025 16:01:14 +0200 Subject: [PATCH 06/12] builtin/maintenance: extract function to run tasks Extract the function to run maintenance tasks. This function will be reused in a subsequent commit where we introduce a split between maintenance tasks that run before and after daemonizing the process. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/gc.c | 35 +++++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index 4d636237ca..cfbf9d8a2b 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1596,6 +1596,27 @@ static const struct maintenance_task tasks[] = { }, }; +static int maybe_run_task(const struct maintenance_task *task, + struct repository *repo, + struct maintenance_run_opts *opts, + struct gc_config *cfg) +{ + int ret = 0; + + if (opts->auto_flag && + (!task->auto_condition || !task->auto_condition(cfg))) + return 0; + + trace2_region_enter("maintenance", task->name, repo); + if (task->fn(opts, cfg)) { + error(_("task '%s' failed"), task->name); + ret = 1; + } + trace2_region_leave("maintenance", task->name, repo); + + return ret; +} + static int maintenance_run_tasks(struct maintenance_run_opts *opts, struct gc_config *cfg) { @@ -1627,19 +1648,9 @@ static int maintenance_run_tasks(struct maintenance_run_opts *opts, trace2_region_leave("maintenance", "detach", the_repository); } - for (size_t i = 0; i < opts->tasks_nr; i++) { - if (opts->auto_flag && - (!tasks[opts->tasks[i]].auto_condition || - !tasks[opts->tasks[i]].auto_condition(cfg))) - continue; - - trace2_region_enter("maintenance", tasks[opts->tasks[i]].name, r); - if (tasks[opts->tasks[i]].fn(opts, cfg)) { - error(_("task '%s' failed"), tasks[opts->tasks[i]].name); + for (size_t i = 0; i < opts->tasks_nr; i++) + if (maybe_run_task(&tasks[opts->tasks[i]], r, opts, cfg)) result = 1; - } - trace2_region_leave("maintenance", tasks[opts->tasks[i]].name, r); - } rollback_lock_file(&lk); return result; From 3236e03c666d66ac94379e192eee5c098bcf7758 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 3 Jun 2025 16:01:15 +0200 Subject: [PATCH 07/12] builtin/maintenance: fix typedef for function pointers The typedefs for `maintenance_task_fn` and `maintenance_auto_fn` are somewhat confusingly not true function pointers. As such, any user of those typedefs needs to manually add the pointer to make use of them. Fix this by making these true function pointers. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/gc.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index cfbf9d8a2b..447e580084 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1533,20 +1533,20 @@ static int maintenance_task_incremental_repack(struct maintenance_run_opts *opts return 0; } -typedef int maintenance_task_fn(struct maintenance_run_opts *opts, - struct gc_config *cfg); +typedef int (*maintenance_task_fn)(struct maintenance_run_opts *opts, + struct gc_config *cfg); /* * An auto condition function returns 1 if the task should run * and 0 if the task should NOT run. See needs_to_gc() for an * example. */ -typedef int maintenance_auto_fn(struct gc_config *cfg); +typedef int (*maintenance_auto_fn)(struct gc_config *cfg); struct maintenance_task { const char *name; - maintenance_task_fn *fn; - maintenance_auto_fn *auto_condition; + maintenance_task_fn fn; + maintenance_auto_fn auto_condition; }; static const struct maintenance_task tasks[] = { From 5bb4298acfb57550934cefca85101825eff177e9 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 3 Jun 2025 16:01:16 +0200 Subject: [PATCH 08/12] builtin/maintenance: split into foreground and background tasks Both git-gc(1) and git-maintenance(1) have logic to daemonize so that the maintenance tasks are performed in the background. git-gc(1) has some special logic though to not perform _all_ housekeeping tasks in the background: both references and reflogs are still handled synchronously in the foreground. This split exists because otherwise it may easily happen that git-gc(1) keeps the "packed-refs" file locked for an extended amount of time, where the next Git command that wants to modify any reference could now fail. This was especially important in the past, where git-gc(1) was still executed directly as part of our automatic maintenance: git-gc(1) was invoked via `git gc --auto --detach`, so we knew to handle most of the maintenance tasks in the background while doing those parts that may cause locking issues in the foreground. We have since moved to git-maintenance(1), which is a more flexible replacement for git-gc(1). By default this command runs git-gc(1), only, but it can be configured to run different tasks, as well. This command does not know about the split between maintenance tasks that should run before and after detach though, and this has led to several bug reports about spurious locking errors for the "packed-refs" file. Prepare for a fix by introducing this split for maintenance tasks. Note that this commit does not yet change any of the tasks, so there should not (yet) be a change in behaviour. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/gc.c | 70 ++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 49 insertions(+), 21 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index 447e580084..72a695853e 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1535,84 +1535,106 @@ static int maintenance_task_incremental_repack(struct maintenance_run_opts *opts typedef int (*maintenance_task_fn)(struct maintenance_run_opts *opts, struct gc_config *cfg); - -/* - * An auto condition function returns 1 if the task should run - * and 0 if the task should NOT run. See needs_to_gc() for an - * example. - */ typedef int (*maintenance_auto_fn)(struct gc_config *cfg); struct maintenance_task { const char *name; - maintenance_task_fn fn; + + /* + * Work that will be executed before detaching. This should not include + * tasks that may run for an extended amount of time as it does cause + * auto-maintenance to block until foreground tasks have been run. + */ + maintenance_task_fn foreground; + + /* + * Work that will be executed after detaching. When not detaching the + * work will be run in the foreground, as well. + */ + maintenance_task_fn background; + + /* + * An auto condition function returns 1 if the task should run and 0 if + * the task should NOT run. See needs_to_gc() for an example. + */ maintenance_auto_fn auto_condition; }; static const struct maintenance_task tasks[] = { [TASK_PREFETCH] = { .name = "prefetch", - .fn = maintenance_task_prefetch, + .background = maintenance_task_prefetch, }, [TASK_LOOSE_OBJECTS] = { .name = "loose-objects", - .fn = maintenance_task_loose_objects, + .background = maintenance_task_loose_objects, .auto_condition = loose_object_auto_condition, }, [TASK_INCREMENTAL_REPACK] = { .name = "incremental-repack", - .fn = maintenance_task_incremental_repack, + .background = maintenance_task_incremental_repack, .auto_condition = incremental_repack_auto_condition, }, [TASK_GC] = { .name = "gc", - .fn = maintenance_task_gc, + .background = maintenance_task_gc, .auto_condition = need_to_gc, }, [TASK_COMMIT_GRAPH] = { .name = "commit-graph", - .fn = maintenance_task_commit_graph, + .background = maintenance_task_commit_graph, .auto_condition = should_write_commit_graph, }, [TASK_PACK_REFS] = { .name = "pack-refs", - .fn = maintenance_task_pack_refs, + .background = maintenance_task_pack_refs, .auto_condition = pack_refs_condition, }, [TASK_REFLOG_EXPIRE] = { .name = "reflog-expire", - .fn = maintenance_task_reflog_expire, + .background = maintenance_task_reflog_expire, .auto_condition = reflog_expire_condition, }, [TASK_WORKTREE_PRUNE] = { .name = "worktree-prune", - .fn = maintenance_task_worktree_prune, + .background = maintenance_task_worktree_prune, .auto_condition = worktree_prune_condition, }, [TASK_RERERE_GC] = { .name = "rerere-gc", - .fn = maintenance_task_rerere_gc, + .background = maintenance_task_rerere_gc, .auto_condition = rerere_gc_condition, }, }; +enum task_phase { + TASK_PHASE_FOREGROUND, + TASK_PHASE_BACKGROUND, +}; + static int maybe_run_task(const struct maintenance_task *task, struct repository *repo, struct maintenance_run_opts *opts, - struct gc_config *cfg) + struct gc_config *cfg, + enum task_phase phase) { + int foreground = (phase == TASK_PHASE_FOREGROUND); + maintenance_task_fn fn = foreground ? task->foreground : task->background; + const char *region = foreground ? "maintenance foreground" : "maintenance"; int ret = 0; + if (!fn) + return 0; if (opts->auto_flag && (!task->auto_condition || !task->auto_condition(cfg))) return 0; - trace2_region_enter("maintenance", task->name, repo); - if (task->fn(opts, cfg)) { + trace2_region_enter(region, task->name, repo); + if (fn(opts, cfg)) { error(_("task '%s' failed"), task->name); ret = 1; } - trace2_region_leave("maintenance", task->name, repo); + trace2_region_leave(region, task->name, repo); return ret; } @@ -1641,6 +1663,11 @@ static int maintenance_run_tasks(struct maintenance_run_opts *opts, } free(lock_path); + for (size_t i = 0; i < opts->tasks_nr; i++) + if (maybe_run_task(&tasks[opts->tasks[i]], r, opts, cfg, + TASK_PHASE_FOREGROUND)) + result = 1; + /* Failure to daemonize is ok, we'll continue in foreground. */ if (opts->detach > 0) { trace2_region_enter("maintenance", "detach", the_repository); @@ -1649,7 +1676,8 @@ static int maintenance_run_tasks(struct maintenance_run_opts *opts, } for (size_t i = 0; i < opts->tasks_nr; i++) - if (maybe_run_task(&tasks[opts->tasks[i]], r, opts, cfg)) + if (maybe_run_task(&tasks[opts->tasks[i]], r, opts, cfg, + TASK_PHASE_BACKGROUND)) result = 1; rollback_lock_file(&lk); From c367852d9e3c114fe02e16f4d56f259f12188e2a Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 3 Jun 2025 16:01:17 +0200 Subject: [PATCH 09/12] builtin/maintenance: fix locking race with refs and reflogs tasks As explained in the preceding commit, git-gc(1) knows to detach only after it has already packed references and expired reflogs. This is done to avoid racing around their respective lockfiles. Adapt git-maintenance(1) accordingly and run the "pack-refs" and "reflog-expire" tasks in the foreground. Note that the "gc" task has the same issue, but the fix is a bit more involved there and will thus be done in a subsequent commit. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/gc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index 72a695853e..fdd0dd09be 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1587,12 +1587,12 @@ static const struct maintenance_task tasks[] = { }, [TASK_PACK_REFS] = { .name = "pack-refs", - .background = maintenance_task_pack_refs, + .foreground = maintenance_task_pack_refs, .auto_condition = pack_refs_condition, }, [TASK_REFLOG_EXPIRE] = { .name = "reflog-expire", - .background = maintenance_task_reflog_expire, + .foreground = maintenance_task_reflog_expire, .auto_condition = reflog_expire_condition, }, [TASK_WORKTREE_PRUNE] = { From 697202b0b1c7c02208c620f96c608e0817d079dd Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 3 Jun 2025 16:01:18 +0200 Subject: [PATCH 10/12] usage: allow dying without writing an error message Sometimes code wants to die in a situation where it already has written an error message. To use the same error code as `die()` we have to use `exit(128)`, which is easy to get wrong and leaves magic numbers all over our codebase. Teach `die_message_builtin()` to not print any error when passed a `NULL` pointer as error string. Like this, such users can now call `die(NULL)` to achieve the same result without any hardcoded error codes. Adapt a couple of builtins to use this new pattern to demonstrate that there is a need for such a helper. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/am.c | 4 ++-- builtin/checkout.c | 4 ++-- builtin/fetch.c | 2 +- builtin/submodule--helper.c | 12 ++++++------ usage.c | 2 ++ 5 files changed, 13 insertions(+), 11 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index e32a3b4c97..a800003340 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -1000,7 +1000,7 @@ static void am_setup(struct am_state *state, enum patch_format patch_format, if (!patch_format) { fprintf_ln(stderr, _("Patch format detection failed.")); - exit(128); + die(NULL); } if (mkdir(state->dir, 0777) < 0 && errno != EEXIST) @@ -1178,7 +1178,7 @@ static void NORETURN die_user_resolve(const struct am_state *state) strbuf_release(&sb); } - exit(128); + die(NULL); } /** diff --git a/builtin/checkout.c b/builtin/checkout.c index d185982f3a..536192d345 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -838,7 +838,7 @@ static int merge_working_tree(const struct checkout_opts *opts, init_tree_desc(&trees[0], &tree->object.oid, tree->buffer, tree->size); if (parse_tree(new_tree) < 0) - exit(128); + die(NULL); tree = new_tree; init_tree_desc(&trees[1], &tree->object.oid, tree->buffer, tree->size); @@ -913,7 +913,7 @@ static int merge_working_tree(const struct checkout_opts *opts, work, old_tree); if (ret < 0) - exit(128); + die(NULL); ret = reset_tree(new_tree, opts, 0, writeout_error, new_branch_info); diff --git a/builtin/fetch.c b/builtin/fetch.c index cda6eaf1fd..b0800ea582 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -992,7 +992,7 @@ static int update_local_ref(struct ref *ref, fast_forward = repo_in_merge_bases(the_repository, current, updated); if (fast_forward < 0) - exit(128); + die(NULL); forced_updates_ms += (getnanotime() - t_before) / 1000000; } else { fast_forward = 1; diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 53da2116dd..4255caca57 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -303,7 +303,7 @@ static void runcommand_in_submodule_cb(const struct cache_entry *list_item, char *displaypath; if (validate_submodule_path(path) < 0) - exit(128); + die(NULL); displaypath = get_submodule_displaypath(path, info->prefix, info->super_prefix); @@ -643,7 +643,7 @@ static void status_submodule(const char *path, const struct object_id *ce_oid, }; if (validate_submodule_path(path) < 0) - exit(128); + die(NULL); if (!submodule_from_path(the_repository, null_oid(the_hash_algo), path)) die(_("no submodule mapping found in .gitmodules for path '%s'"), @@ -1257,7 +1257,7 @@ static void sync_submodule(const char *path, const char *prefix, return; if (validate_submodule_path(path) < 0) - exit(128); + die(NULL); sub = submodule_from_path(the_repository, null_oid(the_hash_algo), path); @@ -1402,7 +1402,7 @@ static void deinit_submodule(const char *path, const char *prefix, char *sub_git_dir = xstrfmt("%s/.git", path); if (validate_submodule_path(path) < 0) - exit(128); + die(NULL); sub = submodule_from_path(the_repository, null_oid(the_hash_algo), path); @@ -1724,7 +1724,7 @@ static int clone_submodule(const struct module_clone_data *clone_data, char *to_free = NULL; if (validate_submodule_path(clone_data_path) < 0) - exit(128); + die(NULL); if (!is_absolute_path(clone_data->path)) clone_data_path = to_free = xstrfmt("%s/%s", repo_get_work_tree(the_repository), @@ -3524,7 +3524,7 @@ static int module_add(int argc, const char **argv, const char *prefix, strip_dir_trailing_slashes(add_data.sm_path); if (validate_submodule_path(add_data.sm_path) < 0) - exit(128); + die(NULL); die_on_index_match(add_data.sm_path, force); die_on_repo_without_commits(add_data.sm_path); diff --git a/usage.c b/usage.c index 38b46bbbfe..cd7b57d644 100644 --- a/usage.c +++ b/usage.c @@ -67,6 +67,8 @@ static NORETURN void usage_builtin(const char *err, va_list params) static void die_message_builtin(const char *err, va_list params) { + if (!err) + return; trace2_cmd_error_va(err, params); vreportf(_("fatal: "), err, params); } From d2b084c66037f1a77b3e061ae7e4d96cbdbf6c05 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 3 Jun 2025 16:01:19 +0200 Subject: [PATCH 11/12] builtin/gc: avoid global state in `gc_before_repack()` The `gc_before_repack()` should only ever run once in git-gc(1), but we may end up calling it twice when the "--detach" flag is passed. The duplicated call is avoided though via a static flag in this function. This pattern is somewhat unintuitive though. Refactor it to drop the static flag and instead guard the second call of `gc_before_repack()` via `opts.detach`. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/gc.c | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index fdd0dd09be..4a5c4b2044 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -816,22 +816,14 @@ done: return ret; } -static void gc_before_repack(struct maintenance_run_opts *opts, - struct gc_config *cfg) +static int gc_before_repack(struct maintenance_run_opts *opts, + struct gc_config *cfg) { - /* - * We may be called twice, as both the pre- and - * post-daemonized phases will call us, but running these - * commands more than once is pointless and wasteful. - */ - static int done = 0; - if (done++) - return; - if (cfg->pack_refs && maintenance_task_pack_refs(opts, cfg)) - die(FAILED_RUN, "pack-refs"); + return error(FAILED_RUN, "pack-refs"); if (cfg->prune_reflogs && maintenance_task_reflog_expire(opts, cfg)) - die(FAILED_RUN, "reflog"); + return error(FAILED_RUN, "reflog"); + return 0; } int cmd_gc(int argc, @@ -965,7 +957,8 @@ int cmd_gc(int argc, goto out; } - gc_before_repack(&opts, &cfg); /* dies on failure */ + if (gc_before_repack(&opts, &cfg) < 0) + die(NULL); delete_tempfile(&pidfile); /* @@ -995,7 +988,8 @@ int cmd_gc(int argc, free(path); } - gc_before_repack(&opts, &cfg); + if (opts.detach <= 0) + gc_before_repack(&opts, &cfg); if (!repository_format_precious_objects) { struct child_process repack_cmd = CHILD_PROCESS_INIT; From 1b5074e614d9b32bd760d2583e7435685ca4faab Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 3 Jun 2025 16:01:20 +0200 Subject: [PATCH 12/12] builtin/maintenance: fix locking race when handling "gc" task The "gc" task has a similar locking race as the one that we have fixed for the "pack-refs" and "reflog-expire" tasks in preceding commits. Fix this by splitting up the logic of the "gc" task: - We execute `gc_before_repack()` in the foreground, which contains the logic that git-gc(1) itself would execute in the foreground, as well. - We spawn git-gc(1) after detaching, but with a new hidden flag that suppresses calling `gc_before_repack()`. Like this we have roughly the same logic as git-gc(1) itself and know to repack refs and reflogs before detaching, thus fixing the race. Note that `gc_before_repack()` is renamed to `gc_foreground_tasks()` to better reflect what this function does. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/gc.c | 41 +++++++++++++++++++++++++++-------------- t/t7900-maintenance.sh | 12 ++++++------ 2 files changed, 33 insertions(+), 20 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index 4a5c4b2044..b5e6519d59 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -816,8 +816,8 @@ done: return ret; } -static int gc_before_repack(struct maintenance_run_opts *opts, - struct gc_config *cfg) +static int gc_foreground_tasks(struct maintenance_run_opts *opts, + struct gc_config *cfg) { if (cfg->pack_refs && maintenance_task_pack_refs(opts, cfg)) return error(FAILED_RUN, "pack-refs"); @@ -837,6 +837,7 @@ int cmd_gc(int argc, pid_t pid; int daemonized = 0; int keep_largest_pack = -1; + int skip_foreground_tasks = 0; timestamp_t dummy; struct maintenance_run_opts opts = MAINTENANCE_RUN_OPTS_INIT; struct gc_config cfg = GC_CONFIG_INIT; @@ -869,6 +870,8 @@ int cmd_gc(int argc, N_("repack all other packs except the largest pack")), OPT_STRING(0, "expire-to", &cfg.repack_expire_to, N_("dir"), N_("pack prefix to store a pack containing pruned objects")), + OPT_HIDDEN_BOOL(0, "skip-foreground-tasks", &skip_foreground_tasks, + N_("skip maintenance tasks typically done in the foreground")), OPT_END() }; @@ -952,14 +955,16 @@ int cmd_gc(int argc, goto out; } - if (lock_repo_for_gc(force, &pid)) { - ret = 0; - goto out; - } + if (!skip_foreground_tasks) { + if (lock_repo_for_gc(force, &pid)) { + ret = 0; + goto out; + } - if (gc_before_repack(&opts, &cfg) < 0) - die(NULL); - delete_tempfile(&pidfile); + if (gc_foreground_tasks(&opts, &cfg) < 0) + die(NULL); + delete_tempfile(&pidfile); + } /* * failure to daemonize is ok, we'll continue @@ -988,8 +993,8 @@ int cmd_gc(int argc, free(path); } - if (opts.detach <= 0) - gc_before_repack(&opts, &cfg); + if (opts.detach <= 0 && !skip_foreground_tasks) + gc_foreground_tasks(&opts, &cfg); if (!repository_format_precious_objects) { struct child_process repack_cmd = CHILD_PROCESS_INIT; @@ -1225,8 +1230,14 @@ static int maintenance_task_prefetch(struct maintenance_run_opts *opts, return 0; } -static int maintenance_task_gc(struct maintenance_run_opts *opts, - struct gc_config *cfg UNUSED) +static int maintenance_task_gc_foreground(struct maintenance_run_opts *opts, + struct gc_config *cfg) +{ + return gc_foreground_tasks(opts, cfg); +} + +static int maintenance_task_gc_background(struct maintenance_run_opts *opts, + struct gc_config *cfg UNUSED) { struct child_process child = CHILD_PROCESS_INIT; @@ -1240,6 +1251,7 @@ static int maintenance_task_gc(struct maintenance_run_opts *opts, else strvec_push(&child.args, "--no-quiet"); strvec_push(&child.args, "--no-detach"); + strvec_push(&child.args, "--skip-foreground-tasks"); return run_command(&child); } @@ -1571,7 +1583,8 @@ static const struct maintenance_task tasks[] = { }, [TASK_GC] = { .name = "gc", - .background = maintenance_task_gc, + .foreground = maintenance_task_gc_foreground, + .background = maintenance_task_gc_background, .auto_condition = need_to_gc, }, [TASK_COMMIT_GRAPH] = { diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh index 1ada524660..ddd273d8dc 100755 --- a/t/t7900-maintenance.sh +++ b/t/t7900-maintenance.sh @@ -49,9 +49,9 @@ test_expect_success 'run [--auto|--quiet]' ' git maintenance run --auto 2>/dev/null && GIT_TRACE2_EVENT="$(pwd)/run-no-quiet.txt" \ git maintenance run --no-quiet 2>/dev/null && - test_subcommand git gc --quiet --no-detach ' ' git maintenance run --task=commit-graph 2>/dev/null && GIT_TRACE2_EVENT="$(pwd)/run-both.txt" \ git maintenance run --task=commit-graph --task=gc 2>/dev/null && - test_subcommand ! git gc --quiet --no-detach