odb: run "pre-auto-gc" hook for all maintenance tasks
The "pre-auto-gc" hook is supposed to run before auto-maintenance starts. The intent of this is to give users the ability to intercept running maintenance in case there's for example an event that is not supposed to run in parallel with repository maintenance. This hook runs via `need_to_gc()`, which is invoked via two paths: - It is called directly by git-gc(1). - It is called indirectly by git-maintenance(1) via the "gc" task. While the former makes sense, the latter is somewhat off. While the hook is indeed strongly tied to gc'ing a repository, the original intent of the hook is rather to inhibit any kind of automated garbage collection. That noticeably also includes all the other maintenance tasks that our new infrastructure may run, but those aren't getting intercepted at all. The move towards our new maintenance strategy has thus somewhat neutered the effectiveness of the hook. Fix this issue by running the hook before the first auto-maintenance task that would run as determined by the tasks's auto condition. Note that this requires us to lift the call to `run_hooks()` out of `needs_to_gc()`, as the hook would otherwise potentially run multiple times. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Patrick Steinhardt committed
Jul 13, 2026 at 07:52 UTC
37deb9b4be807643ef264738f7fa3dc97588e33d
2 files changed
+152
-9
builtin/gc.c
+26
-9
@@ -709,8 +709,6 @@ static int need_to_gc(struct gc_config *cfg, struct strvec *repack_args)
709
else
710
return 0;
711
712
- if (run_hooks(the_repository, "pre-auto-gc"))
713
- return 0;
712
return 1;
713
}
714
@@ -933,7 +931,8 @@ int cmd_gc(int argc,
931
/*
932
* Auto-gc should be least intrusive as possible.
933
*/
936
- if (!need_to_gc(&cfg, &repack_args)) {
934
+ if (!need_to_gc(&cfg, &repack_args) ||
935
+ run_hooks(the_repository, "pre-auto-gc")) {
936
ret = 0;
937
goto out;
938
}
@@ -1755,11 +1754,18 @@ enum task_phase {
1754
TASK_PHASE_BACKGROUND,
1755
};
1756
1757
+enum auto_gc_hook_result {
1758
+ AUTO_GC_HOOK_UNDECIDED = 0,
1759
+ AUTO_GC_HOOK_RUN = 1,
1760
+ AUTO_GC_HOOK_SKIP = 2,
1761
+};
1762
+
1763
static int maybe_run_task(const struct maintenance_task *task,
1764
struct repository *repo,
1765
struct maintenance_run_opts *opts,
1766
struct gc_config *cfg,
1762
- enum task_phase phase)
1767
+ enum task_phase phase,
1768
+ enum auto_gc_hook_result *auto_gc_hook_result)
1769
{
1770
int foreground = (phase == TASK_PHASE_FOREGROUND);
1771
maintenance_task_fn fn = foreground ? task->foreground : task->background;
@@ -1768,9 +1774,19 @@ static int maybe_run_task(const struct maintenance_task *task,
1774
1775
if (!fn)
1776
return 0;
1771
- if (opts->auto_flag &&
1772
- (!task->auto_condition || !task->auto_condition(cfg)))
1773
- return 0;
1777
+ if (opts->auto_flag) {
1778
+ if (*auto_gc_hook_result == AUTO_GC_HOOK_SKIP)
1779
+ return 0;
1780
+
1781
+ if (!task->auto_condition || !task->auto_condition(cfg))
1782
+ return 0;
1783
+
1784
+ if (*auto_gc_hook_result == AUTO_GC_HOOK_UNDECIDED)
1785
+ *auto_gc_hook_result = run_hooks(repo, "pre-auto-gc") ?
1786
+ AUTO_GC_HOOK_SKIP : AUTO_GC_HOOK_RUN;
1787
+ if (*auto_gc_hook_result == AUTO_GC_HOOK_SKIP)
1788
+ return 0;
1789
+ }
1790
1791
trace2_region_enter(region, task->name, repo);
1792
if (fn(opts, cfg)) {
@@ -1789,6 +1805,7 @@ static int maintenance_run_tasks(struct maintenance_run_opts *opts,
1805
struct lock_file lk;
1806
struct repository *r = the_repository;
1807
char *lock_path = xstrfmt("%s/maintenance", r->objects->sources->path);
1808
+ enum auto_gc_hook_result auto_gc_hook_result = AUTO_GC_HOOK_UNDECIDED;
1809
1810
if (hold_lock_file_for_update(&lk, lock_path, LOCK_NO_DEREF) < 0) {
1811
/*
@@ -1808,7 +1825,7 @@ static int maintenance_run_tasks(struct maintenance_run_opts *opts,
1825
1826
for (size_t i = 0; i < opts->tasks_nr; i++)
1827
if (maybe_run_task(&tasks[opts->tasks[i]], r, opts, cfg,
1811
- TASK_PHASE_FOREGROUND))
1828
+ TASK_PHASE_FOREGROUND, &auto_gc_hook_result))
1829
result = 1;
1830
1831
/* Failure to daemonize is ok, we'll continue in foreground. */
@@ -1820,7 +1837,7 @@ static int maintenance_run_tasks(struct maintenance_run_opts *opts,
1837
1838
for (size_t i = 0; i < opts->tasks_nr; i++)
1839
if (maybe_run_task(&tasks[opts->tasks[i]], r, opts, cfg,
1823
- TASK_PHASE_BACKGROUND))
1840
+ TASK_PHASE_BACKGROUND, &auto_gc_hook_result))
1841
result = 1;
1842
1843
rollback_lock_file(&lk);
t/t7900-maintenance.sh
+126
@@ -758,6 +758,132 @@ test_expect_success 'geometric repacking honors configured split factor' '
758
)
759
'
760
761
+test_expect_success 'pre-auto-gc hook runs exactly once' '
762
+ test_when_finished "rm -rf repo" &&
763
+ git init repo &&
764
+ (
765
+ cd repo &&
766
+ write_script .git/hooks/pre-auto-gc <<-\EOF &&
767
+ echo hook >>hook.log
768
+ EOF
769
+
770
+ # Satisfy the auto condition for multiple tasks, both in the
771
+ # foreground and in the background phase.
772
+ git config set maintenance.reflog-expire.auto -1 &&
773
+ git config set maintenance.geometric-repack.auto -1 &&
774
+ git config set maintenance.rerere-gc.auto -1 &&
775
+
776
+ GIT_TRACE2_EVENT="$(pwd)/trace2.txt" \
777
+ git maintenance run --auto 2>/dev/null &&
778
+
779
+ # The successful hook does not inhibit any of the tasks...
780
+ test_maintenance_tasks trace2.txt <<-\EOF &&
781
+ reflog-expire foreground
782
+ geometric-repack
783
+ rerere-gc
784
+ EOF
785
+ # ... but it must only have been executed a single time.
786
+ test_line_count = 1 hook.log
787
+ )
788
+'
789
+
790
+test_expect_success 'pre-auto-gc hook can inhibit geometric strategy' '
791
+ test_when_finished "rm -rf repo" &&
792
+ git init repo &&
793
+ (
794
+ cd repo &&
795
+ write_script .git/hooks/pre-auto-gc <<-\EOF &&
796
+ echo hook >>hook.log
797
+ exit 1
798
+ EOF
799
+
800
+ git config set maintenance.reflog-expire.auto -1 &&
801
+ git config set maintenance.geometric-repack.auto -1 &&
802
+ git config set maintenance.rerere-gc.auto -1 &&
803
+
804
+ # Maintenance would be required...
805
+ git maintenance is-needed --auto &&
806
+
807
+ GIT_TRACE2_EVENT="$(pwd)/trace2.txt" \
808
+ git maintenance run --auto 2>/dev/null &&
809
+
810
+ # ... but the failing hook inhibits all tasks. The hook itself
811
+ # is expected to be the only child process being spawned, and
812
+ # it must only run a single time.
813
+ test_grep "child_start.*pre-auto-gc" trace2.txt &&
814
+ test_maintenance_tasks trace2.txt <<-\EOF &&
815
+ EOF
816
+ test_line_count = 1 hook.log
817
+ )
818
+'
819
+
820
+test_expect_success 'pre-auto-gc hook can inhibit gc strategy' '
821
+ test_when_finished "rm -rf repo" &&
822
+ git init repo &&
823
+ (
824
+ cd repo &&
825
+ write_script .git/hooks/pre-auto-gc <<-\EOF &&
826
+ echo hook >>hook.log
827
+ exit 1
828
+ EOF
829
+
830
+ git config set maintenance.strategy gc &&
831
+ git config set maintenance.auto false &&
832
+ git config set gc.auto 3 &&
833
+
834
+ test_oid_init &&
835
+
836
+ # We need to create two objects whose hashes start with 17
837
+ # since this is what the gc task counts.
838
+ test_commit "$(test_oid blob17_1)" &&
839
+ test_commit "$(test_oid blob17_2)" &&
840
+
841
+ # Maintenance would be required...
842
+ git maintenance is-needed --auto &&
843
+
844
+ GIT_TRACE2_EVENT="$(pwd)/trace2.txt" \
845
+ git maintenance run --auto 2>/dev/null &&
846
+
847
+ # ... but the failing hook inhibits all tasks. The hook itself
848
+ # is expected to be the only child process being spawned, and
849
+ # it must only run a single time.
850
+ test_grep "child_start.*pre-auto-gc" trace2.txt &&
851
+ test_maintenance_tasks trace2.txt <<-\EOF &&
852
+ EOF
853
+ test_subcommand_flex ! git trace2 &&
854
+ test_line_count = 1 hook.log
855
+ )
856
+'
857
+
858
+test_expect_success 'pre-auto-gc hook does not run when no maintenance is needed' '
859
+ test_when_finished "rm -rf repo" &&
860
+ git init repo &&
861
+ (
862
+ cd repo &&
863
+ write_script .git/hooks/pre-auto-gc <<-\EOF &&
864
+ echo hook >>hook.log
865
+ EOF
866
+ test_must_fail git maintenance is-needed --auto &&
867
+ git maintenance run --auto 2>/dev/null &&
868
+ test_path_is_missing hook.log
869
+ )
870
+'
871
+
872
+test_expect_success 'pre-auto-gc hook does not run without --auto' '
873
+ test_when_finished "rm -rf repo" &&
874
+ git init repo &&
875
+ test_hook -C repo pre-auto-gc <<-\EOF &&
876
+ echo hook >>hook.log
877
+ EOF
878
+ (
879
+ cd repo &&
880
+ GIT_TRACE2_EVENT="$(pwd)/trace2.txt" \
881
+ git maintenance run 2>/dev/null &&
882
+ test_grep "\[\"git\",\"repack\"," trace2.txt &&
883
+ test_path_is_missing hook.log
884
+ )
885
+'
886
+
887
test_expect_success 'pack-refs task' '
888
for n in $(test_seq 1 5)
889
do