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:
- Before detaching we execute `gc_before_repack()`, which contains the
logic that git-gc(1) itself would execute before detaching.
- After detaching we spawn git-gc(1), 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_before_detach()` to
better reflect what this function does.
Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
---
builtin/gc.c | 39 ++++++++++++++++++++++++++-------------
t/t7900-maintenance.sh | 12 ++++++------
2 files changed, 32 insertions(+), 19 deletions(-)
diff --git a/builtin/gc.c b/builtin/gc.c
index 174357b9c25..2cf61efcee9 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -816,7 +816,7 @@ static int report_last_gc_error(void)
return ret;
}
-static int gc_before_repack(struct maintenance_run_opts *opts,
+static int gc_before_detach(struct maintenance_run_opts *opts,
struct gc_config *cfg)
{
if (cfg->pack_refs && maintenance_task_pack_refs(opts, cfg))
@@ -837,6 +837,7 @@ int cmd_gc(int argc,
pid_t pid;
int daemonized = 0;
int keep_largest_pack = -1;
+ int skip_maintenance_before_detach = 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-maintenance-before-detach", &skip_maintenance_before_detach,
+ N_("skip maintenance steps typically done before detaching")),
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_maintenance_before_detach) {
+ if (lock_repo_for_gc(force, &pid)) {
+ ret = 0;
+ goto out;
+ }
- if (gc_before_repack(&opts, &cfg) < 0)
- exit(127);
- delete_tempfile(&pidfile);
+ if (gc_before_detach(&opts, &cfg) < 0)
+ exit(127);
+ 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_maintenance_before_detach)
+ gc_before_detach(&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_before_detach(struct maintenance_run_opts *opts,
+ struct gc_config *cfg)
+{
+ return gc_before_detach(opts, cfg);
+}
+
+static int maintenance_task_gc_after_detach(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-maintenance-before-detach");
return run_command(&child);
}
@@ -1561,7 +1573,8 @@ static const struct maintenance_task tasks[] = {
},
[TASK_GC] = {
.name = "gc",
- .after_detach = maintenance_task_gc,
+ .before_detach = maintenance_task_gc_before_detach,
+ .after_detach = maintenance_task_gc_after_detach,
.auto_condition = need_to_gc,
},
[TASK_COMMIT_GRAPH] = {
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 1ada5246606..e09a36ab021 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 <run-no-auto.txt &&
- test_subcommand ! git gc --auto --quiet --no-detach <run-auto.txt &&
- test_subcommand git gc --no-quiet --no-detach <run-no-quiet.txt
+ test_subcommand git gc --quiet --no-detach --skip-maintenance-before-detach <run-no-auto.txt &&
+ test_subcommand ! git gc --auto --quiet --no-detach --skip-maintenance-before-detach <run-auto.txt &&
+ test_subcommand git gc --no-quiet --no-detach --skip-maintenance-before-detach <run-no-quiet.txt
'
test_expect_success 'maintenance.auto config option' '
@@ -154,9 +154,9 @@ test_expect_success 'run --task=<task>' '
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 <run-commit-graph.txt &&
- test_subcommand git gc --quiet --no-detach <run-gc.txt &&
- test_subcommand git gc --quiet --no-detach <run-both.txt &&
+ test_subcommand ! git gc --quiet --no-detach --skip-maintenance-before-detach <run-commit-graph.txt &&
+ test_subcommand git gc --quiet --no-detach --skip-maintenance-before-detach <run-gc.txt &&
+ test_subcommand git gc --quiet --no-detach --skip-maintenance-before-detach <run-both.txt &&
test_subcommand git commit-graph write --split --reachable --no-progress <run-commit-graph.txt &&
test_subcommand ! git commit-graph write --split --reachable --no-progress <run-gc.txt &&
test_subcommand git commit-graph write --split --reachable --no-progress <run-both.txt
--
2.49.0.1266.g31b7d2e469.dirty
