builtin/gc: remove global `repack` variable
The global `repack` variable is used to store all command line arguments that we eventually want to pass to git-repack(1). It is being appended to from multiple different functions, which makes it hard to follow the logic. Besides being hard to follow, it also makes it unnecessarily hard to reuse this infrastructure in new code. Refactor the code so that we store this variable on the stack and pass a pointer to it around as needed. This is done so that we can reuse `add_repack_all_options()` in a subsequent commit. The refactoring itself is straight-forward. One function that deserves attention though is `need_to_gc()`: this function determines whether or not we need to execute garbage collection for `git gc --auto`, but also for `git maintenance run --auto`. But besides figuring out whether we have to perform GC, the function also sets up the `repack` arguments. For `git gc --auto` it's trivial to adapt, as we already have the on-stack variable at our fingertips. But for the maintenance condition it's less obvious what to do. As it turns out, we can just use another temporary variable there that we then immediately discard. If we need to perform GC we execute a child git-gc(1) process to repack objects for us, and that process will have to recompute the arguments anyway. Signed-off-by: Patrick Steinhardt <ps@pks.im> Acked-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Patrick Steinhardt committed
Oct 24, 2025 at 08:57 UTC
0ea94b023a64d1341a11252954bb7ce37dd3d922
1 file changed
+45
-29
builtin/gc.c
+45
-29
@@ -55,7 +55,6 @@ static const char * const builtin_gc_usage[] = {
55
};
56
57
static timestamp_t gc_log_expire_time;
58
-static struct strvec repack = STRVEC_INIT;
58
static struct tempfile *pidfile;
59
static struct lock_file log_lock;
60
static struct string_list pack_garbage = STRING_LIST_INIT_DUP;
@@ -618,48 +617,50 @@ static uint64_t estimate_repack_memory(struct gc_config *cfg,
617
return os_cache + heap;
618
}
619
621
-static int keep_one_pack(struct string_list_item *item, void *data UNUSED)
620
+static int keep_one_pack(struct string_list_item *item, void *data)
621
{
623
- strvec_pushf(&repack, "--keep-pack=%s", basename(item->string));
622
+ struct strvec *args = data;
623
+ strvec_pushf(args, "--keep-pack=%s", basename(item->string));
624
return 0;
625
}
626
627
static void add_repack_all_option(struct gc_config *cfg,
628
- struct string_list *keep_pack)
628
+ struct string_list *keep_pack,
629
+ struct strvec *args)
630
{
631
if (cfg->prune_expire && !strcmp(cfg->prune_expire, "now")
632
&& !(cfg->cruft_packs && cfg->repack_expire_to))
632
- strvec_push(&repack, "-a");
633
+ strvec_push(args, "-a");
634
else if (cfg->cruft_packs) {
634
- strvec_push(&repack, "--cruft");
635
+ strvec_push(args, "--cruft");
636
if (cfg->prune_expire)
636
- strvec_pushf(&repack, "--cruft-expiration=%s", cfg->prune_expire);
637
+ strvec_pushf(args, "--cruft-expiration=%s", cfg->prune_expire);
638
if (cfg->max_cruft_size)
638
- strvec_pushf(&repack, "--max-cruft-size=%lu",
639
+ strvec_pushf(args, "--max-cruft-size=%lu",
640
cfg->max_cruft_size);
641
if (cfg->repack_expire_to)
641
- strvec_pushf(&repack, "--expire-to=%s", cfg->repack_expire_to);
642
+ strvec_pushf(args, "--expire-to=%s", cfg->repack_expire_to);
643
} else {
643
- strvec_push(&repack, "-A");
644
+ strvec_push(args, "-A");
645
if (cfg->prune_expire)
645
- strvec_pushf(&repack, "--unpack-unreachable=%s", cfg->prune_expire);
646
+ strvec_pushf(args, "--unpack-unreachable=%s", cfg->prune_expire);
647
}
648
649
if (keep_pack)
649
- for_each_string_list(keep_pack, keep_one_pack, NULL);
650
+ for_each_string_list(keep_pack, keep_one_pack, args);
651
652
if (cfg->repack_filter && *cfg->repack_filter)
652
- strvec_pushf(&repack, "--filter=%s", cfg->repack_filter);
653
+ strvec_pushf(args, "--filter=%s", cfg->repack_filter);
654
if (cfg->repack_filter_to && *cfg->repack_filter_to)
654
- strvec_pushf(&repack, "--filter-to=%s", cfg->repack_filter_to);
655
+ strvec_pushf(args, "--filter-to=%s", cfg->repack_filter_to);
656
}
657
657
-static void add_repack_incremental_option(void)
658
+static void add_repack_incremental_option(struct strvec *args)
659
{
659
- strvec_push(&repack, "--no-write-bitmap-index");
660
+ strvec_push(args, "--no-write-bitmap-index");
661
}
662
662
-static int need_to_gc(struct gc_config *cfg)
663
+static int need_to_gc(struct gc_config *cfg, struct strvec *repack_args)
664
{
665
/*
666
* Setting gc.auto to 0 or negative can disable the
@@ -700,10 +701,10 @@ static int need_to_gc(struct gc_config *cfg)
701
string_list_clear(&keep_pack, 0);
702
}
703
703
- add_repack_all_option(cfg, &keep_pack);
704
+ add_repack_all_option(cfg, &keep_pack, repack_args);
705
string_list_clear(&keep_pack, 0);
706
} else if (too_many_loose_objects(cfg))
706
- add_repack_incremental_option();
707
+ add_repack_incremental_option(repack_args);
708
else
709
return 0;
710
@@ -852,6 +853,7 @@ int cmd_gc(int argc,
853
int keep_largest_pack = -1;
854
int skip_foreground_tasks = 0;
855
timestamp_t dummy;
856
+ struct strvec repack_args = STRVEC_INIT;
857
struct maintenance_run_opts opts = MAINTENANCE_RUN_OPTS_INIT;
858
struct gc_config cfg = GC_CONFIG_INIT;
859
const char *prune_expire_sentinel = "sentinel";
@@ -891,7 +893,7 @@ int cmd_gc(int argc,
893
show_usage_with_options_if_asked(argc, argv,
894
builtin_gc_usage, builtin_gc_options);
895
894
- strvec_pushl(&repack, "repack", "-d", "-l", NULL);
896
+ strvec_pushl(&repack_args, "repack", "-d", "-l", NULL);
897
898
gc_config(&cfg);
899
@@ -914,14 +916,14 @@ int cmd_gc(int argc,
916
die(_("failed to parse prune expiry value %s"), cfg.prune_expire);
917
918
if (aggressive) {
917
- strvec_push(&repack, "-f");
919
+ strvec_push(&repack_args, "-f");
920
if (cfg.aggressive_depth > 0)
919
- strvec_pushf(&repack, "--depth=%d", cfg.aggressive_depth);
921
+ strvec_pushf(&repack_args, "--depth=%d", cfg.aggressive_depth);
922
if (cfg.aggressive_window > 0)
921
- strvec_pushf(&repack, "--window=%d", cfg.aggressive_window);
923
+ strvec_pushf(&repack_args, "--window=%d", cfg.aggressive_window);
924
}
925
if (opts.quiet)
924
- strvec_push(&repack, "-q");
926
+ strvec_push(&repack_args, "-q");
927
928
if (opts.auto_flag) {
929
if (cfg.detach_auto && opts.detach < 0)
@@ -930,7 +932,7 @@ int cmd_gc(int argc,
932
/*
933
* Auto-gc should be least intrusive as possible.
934
*/
933
- if (!need_to_gc(&cfg)) {
935
+ if (!need_to_gc(&cfg, &repack_args)) {
936
ret = 0;
937
goto out;
938
}
@@ -952,7 +954,7 @@ int cmd_gc(int argc,
954
find_base_packs(&keep_pack, cfg.big_pack_threshold);
955
}
956
955
- add_repack_all_option(&cfg, &keep_pack);
957
+ add_repack_all_option(&cfg, &keep_pack, &repack_args);
958
string_list_clear(&keep_pack, 0);
959
}
960
@@ -1014,9 +1016,9 @@ int cmd_gc(int argc,
1016
1017
repack_cmd.git_cmd = 1;
1018
repack_cmd.close_object_store = 1;
1017
- strvec_pushv(&repack_cmd.args, repack.v);
1019
+ strvec_pushv(&repack_cmd.args, repack_args.v);
1020
if (run_command(&repack_cmd))
1019
- die(FAILED_RUN, repack.v[0]);
1021
+ die(FAILED_RUN, repack_args.v[0]);
1022
1023
if (cfg.prune_expire) {
1024
struct child_process prune_cmd = CHILD_PROCESS_INIT;
@@ -1067,6 +1069,7 @@ int cmd_gc(int argc,
1069
1070
out:
1071
maintenance_run_opts_release(&opts);
1072
+ strvec_clear(&repack_args);
1073
gc_config_release(&cfg);
1074
return 0;
1075
}
@@ -1269,6 +1272,19 @@ static int maintenance_task_gc_background(struct maintenance_run_opts *opts,
1272
return run_command(&child);
1273
}
1274
1275
+static int gc_condition(struct gc_config *cfg)
1276
+{
1277
+ /*
1278
+ * Note that it's fine to drop the repack arguments here, as we execute
1279
+ * git-gc(1) as a separate child process anyway. So it knows to compute
1280
+ * these arguments again.
1281
+ */
1282
+ struct strvec repack_args = STRVEC_INIT;
1283
+ int ret = need_to_gc(cfg, &repack_args);
1284
+ strvec_clear(&repack_args);
1285
+ return ret;
1286
+}
1287
+
1288
static int prune_packed(struct maintenance_run_opts *opts)
1289
{
1290
struct child_process child = CHILD_PROCESS_INIT;
@@ -1596,7 +1612,7 @@ static const struct maintenance_task tasks[] = {
1612
.name = "gc",
1613
.foreground = maintenance_task_gc_foreground,
1614
.background = maintenance_task_gc_background,
1599
- .auto_condition = need_to_gc,
1615
+ .auto_condition = gc_condition,
1616
},
1617
[TASK_COMMIT_GRAPH] = {
1618
.name = "commit-graph",