[PATCH 10/11] builtin/gc: avoid global state in `gc_before_repack()`
- From
Patrick Steinhardt <ps@pks.im>
- Date
- May 27, 2025, 14:04 UTC
- Message-ID
- <20250527-b4-pks-maintenance-ref-lock-race-v1-10-e1ceb2dea66e@pks.im>
- In-Reply-To
- <20250527-b4-pks-maintenance-ref-lock-race-v1-0-e1ceb2dea66e@pks.im>
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 <ps@pks.im> --- builtin/gc.c | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-)
diff --git a/builtin/gc.c b/builtin/gc.c index e5d1114bd2d..174357b9c25 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -816,22 +816,14 @@ static int report_last_gc_error(void) 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) + exit(127); 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;
-- 2.49.0.1266.g31b7d2e469.dirty