All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/8] builtin/maintenance: use "geometric" strategy by default
@ 2026-02-20 10:15 Patrick Steinhardt
  2026-02-20 10:15 ` [PATCH 1/8] t: fix races caused by background maintenance Patrick Steinhardt
                   ` (9 more replies)
  0 siblings, 10 replies; 38+ messages in thread
From: Patrick Steinhardt @ 2026-02-20 10:15 UTC (permalink / raw)
  To: git; +Cc: Derrick Stolee, Taylor Blau

Hi,

this series converts our default strategy used by git-maintenance(1)
from "gc" to "geometric". The aim of this is twofold:

  - It completes the conversion to a more flexible infrastructure for
    repository maintenance. git-maintenance(1) is structured around
    tasks that can be toggled on/off as needed, and this is a lot easier
    to extend going forward.

  - We start to use a more efficient repacking strategy by default,
    which should especially help large repositories out there.

Out of these two, I think that the first point is actually the more
important one.

Unfortunately, a lot of our tests are racy or will fail with the new
strategy. This is mostly because the new strategy may decide to optimize
data structures in cases where the old strategy didn't, and because the
tasks we perform might be different. The majority of this patch series
thus adapts our tests accordingly. The actual change is a one-line
change in the final commit.

I was a bit torn initially whether or not I want to make the geometric
strategy the default right away, or whether we might first want to use
"feature.experimental" as an additional step. I'm quite happy to adapt
the series accordingly, but for the initial version I thought it might
invite more discussions if I pick the nuclear option :)

Of course, no matter how we do this, it is still possible to revert back
to the old strategy by setting "maintenance.strategy=gc".

Thanks!

Patrick

---
Patrick Steinhardt (8):
      t: fix races caused by background maintenance
      t: disable maintenance where we verify object database structure
      t34xx: don't expire reflogs where it matters
      t5400: explicitly use "gc" strategy
      t5510: explicitly use "gc" strategy
      t6500: explicitly use "gc" strategy
      t7900: prepare for switch of the default strategy
      builtin/maintenance: use "geometric" strategy by default

 builtin/gc.c                            | 2 +-
 run-command.c                           | 2 +-
 t/t0081-find-pack.sh                    | 1 +
 t/t3404-rebase-interactive.sh           | 2 ++
 t/t3406-rebase-message.sh               | 3 +++
 t/t3431-rebase-fork-point.sh            | 2 ++
 t/t3432-rebase-fast-forward.sh          | 2 ++
 t/t5316-pack-delta-depth.sh             | 1 +
 t/t5319-multi-pack-index.sh             | 1 +
 t/t5326-multi-pack-bitmaps.sh           | 3 ++-
 t/t5327-multi-pack-bitmaps-rev.sh       | 3 ++-
 t/t5331-pack-objects-stdin.sh           | 2 ++
 t/t5332-multi-pack-reuse.sh             | 1 +
 t/t5334-incremental-multi-pack-index.sh | 1 +
 t/t5400-send-pack.sh                    | 1 +
 t/t5500-fetch-pack.sh                   | 3 ++-
 t/t5510-fetch.sh                        | 1 +
 t/t5616-partial-clone.sh                | 7 ++++---
 t/t6500-gc.sh                           | 1 +
 t/t7700-repack.sh                       | 3 +++
 t/t7900-maintenance.sh                  | 7 ++++++-
 t/test-lib.sh                           | 4 ++++
 22 files changed, 44 insertions(+), 9 deletions(-)


---
base-commit: 73fd77805fc6406f31c36212846d9e2541d19321
change-id: 20260218-b4-pks-maintenance-default-geometric-strategy-17fcedf92461


^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCH 1/8] t: fix races caused by background maintenance
  2026-02-20 10:15 [PATCH 0/8] builtin/maintenance: use "geometric" strategy by default Patrick Steinhardt
@ 2026-02-20 10:15 ` Patrick Steinhardt
  2026-02-23 16:01   ` Justin Tobler
  2026-08-10  4:43   ` Stefan Haller
  2026-02-20 10:15 ` [PATCH 2/8] t: disable maintenance where we verify object database structure Patrick Steinhardt
                   ` (8 subsequent siblings)
  9 siblings, 2 replies; 38+ messages in thread
From: Patrick Steinhardt @ 2026-02-20 10:15 UTC (permalink / raw)
  To: git; +Cc: Derrick Stolee, Taylor Blau

Many Git commands spawn git-maintenance(1) to optimize the repository in
the background. By default, performing the maintenance is for most of
the part asynchronous: we fork the executable and then continue with the
rest of our business logic.

This is working as expected for our users, but this behaviour is
somewhat problematic for our test suite as this is inherently racy. We
have many tests that verify the on-disk state of repositories, and those
tests may easily race with our background maintenance. In a similar
fashion, we may end up with processes that "leak" out of a current test
case.

Until now this tends to not be much of a problem. Our maintenance uses
git-gc(1) by default, which knows to bail out in case there aren't
either too many packfiles or too many loose objects. So even if other
data structures would need to be optimized, we won't do so unless the
object database also needs optimizations.

This is about to change though, as a subsequent commit will switch to
the "geometric" maintenance strategy as a default. The consequence is
that we will run required optimizations even if the object database is
well-optimized. And this uncovers races between our test suite and
background maintenance all over the place.

Disabling maintenance outright in our test suite is not really an
option, as it would result in significantly divergence from the "real
world" and reduce our test coverage. But we've got an alternative up our
sleeves: we can ensure that garbage collection runs synchronously by
overriding the "maintenance.autoDetach" configuration.

Of course that also diverges from the real world, as we now stop testing
that background maintenance interacts in a benign way with normal Git
commands. But on the other hand this ensures that the maintenance itself
does not for example lead to data loss in a more reproducible way.

Another concern is that this would make execution of the test suite much
slower. But a quick benchmark on my machine demonstrates that this does
not seem to be the case:

    Benchmark 1: meson test (revision = HEAD~)
      Time (mean ± σ):     131.182 s ±  1.293 s    [User: 853.737 s, System: 1160.479 s]
      Range (min … max):   130.001 s … 132.563 s    3 runs

    Benchmark 2: meson test (revision = HEAD)
      Time (mean ± σ):     129.554 s ±  0.507 s    [User: 849.040 s, System: 1152.664 s]
      Range (min … max):   129.000 s … 129.994 s    3 runs

    Summary
      meson test (revision = HEAD) ran
        1.01 ± 0.01 times faster than meson test (revision = HEAD~)

Funny enough, it even seems as if this speeds up test execution ever so
slightly, but that may just as well be noise.

Introduce a new `GIT_TEST_MAINT_AUTO_DETACH` environment variable that
allows us to override the auto-detach behaviour and set that varibale in
our tests.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 run-command.c            | 2 +-
 t/t5616-partial-clone.sh | 6 +++---
 t/t7900-maintenance.sh   | 1 +
 t/test-lib.sh            | 4 ++++
 4 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/run-command.c b/run-command.c
index e3e02475cc..438a290d30 100644
--- a/run-command.c
+++ b/run-command.c
@@ -1828,7 +1828,7 @@ int prepare_auto_maintenance(int quiet, struct child_process *maint)
 	 */
 	if (repo_config_get_bool(the_repository, "maintenance.autodetach", &auto_detach) &&
 	    repo_config_get_bool(the_repository, "gc.autodetach", &auto_detach))
-		auto_detach = 1;
+		auto_detach = git_env_bool("GIT_TEST_MAINT_AUTO_DETACH", true);
 
 	maint->git_cmd = 1;
 	maint->close_object_store = 1;
diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh
index 1e354e057f..d62760eb92 100755
--- a/t/t5616-partial-clone.sh
+++ b/t/t5616-partial-clone.sh
@@ -229,7 +229,7 @@ test_expect_success 'fetch --refetch triggers repacking' '
 
 	GIT_TRACE2_EVENT="$PWD/trace1.event" \
 	git -C pc1 fetch --refetch origin &&
-	test_subcommand git maintenance run --auto --no-quiet --detach <trace1.event &&
+	test_subcommand git maintenance run --auto --no-quiet --no-detach <trace1.event &&
 	grep \"param\":\"gc.autopacklimit\",\"value\":\"1\" trace1.event &&
 	grep \"param\":\"maintenance.incremental-repack.auto\",\"value\":\"-1\" trace1.event &&
 
@@ -238,7 +238,7 @@ test_expect_success 'fetch --refetch triggers repacking' '
 		-c gc.autoPackLimit=0 \
 		-c maintenance.incremental-repack.auto=1234 \
 		-C pc1 fetch --refetch origin &&
-	test_subcommand git maintenance run --auto --no-quiet --detach <trace2.event &&
+	test_subcommand git maintenance run --auto --no-quiet --no-detach <trace2.event &&
 	grep \"param\":\"gc.autopacklimit\",\"value\":\"0\" trace2.event &&
 	grep \"param\":\"maintenance.incremental-repack.auto\",\"value\":\"-1\" trace2.event &&
 
@@ -247,7 +247,7 @@ test_expect_success 'fetch --refetch triggers repacking' '
 		-c gc.autoPackLimit=1234 \
 		-c maintenance.incremental-repack.auto=0 \
 		-C pc1 fetch --refetch origin &&
-	test_subcommand git maintenance run --auto --no-quiet --detach <trace3.event &&
+	test_subcommand git maintenance run --auto --no-quiet --no-detach <trace3.event &&
 	grep \"param\":\"gc.autopacklimit\",\"value\":\"1\" trace3.event &&
 	grep \"param\":\"maintenance.incremental-repack.auto\",\"value\":\"0\" trace3.event
 '
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 7cc0ce57f8..d11d6f8f15 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -6,6 +6,7 @@ test_description='git maintenance builtin'
 
 GIT_TEST_COMMIT_GRAPH=0
 GIT_TEST_MULTI_PACK_INDEX=0
+sane_unset GIT_TEST_MAINT_AUTO_DETACH
 
 test_lazy_prereq XMLLINT '
 	xmllint --version
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 0fb76f7d11..aa805a01ce 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1947,6 +1947,10 @@ test_lazy_prereq COMPAT_HASH '
 GIT_TEST_MAINT_SCHEDULER="none:exit 1"
 export GIT_TEST_MAINT_SCHEDULER
 
+# Ensure that tests cannot race with background maintenance by default.
+GIT_TEST_MAINT_AUTO_DETACH="false"
+export GIT_TEST_MAINT_AUTO_DETACH
+
 # Does this platform support `git fsmonitor--daemon`
 #
 test_lazy_prereq FSMONITOR_DAEMON '

-- 
2.53.0.414.gf7e9f6c205.dirty


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 2/8] t: disable maintenance where we verify object database structure
  2026-02-20 10:15 [PATCH 0/8] builtin/maintenance: use "geometric" strategy by default Patrick Steinhardt
  2026-02-20 10:15 ` [PATCH 1/8] t: fix races caused by background maintenance Patrick Steinhardt
@ 2026-02-20 10:15 ` Patrick Steinhardt
  2026-02-23 16:07   ` Justin Tobler
  2026-02-20 10:15 ` [PATCH 3/8] t34xx: don't expire reflogs where it matters Patrick Steinhardt
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 38+ messages in thread
From: Patrick Steinhardt @ 2026-02-20 10:15 UTC (permalink / raw)
  To: git; +Cc: Derrick Stolee, Taylor Blau

We have a couple of tests that explicitly verify the structure of the
object database. Naturally, this structure is dependent on whether or
not we run repository maintenance: if it decides to optimize the object
database the expected structure is likely to not materialize.

Explicitly disable auto-maintenance in such tests so that we are not
dependent on decisions made by our maintenance.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t0081-find-pack.sh                    | 1 +
 t/t5316-pack-delta-depth.sh             | 1 +
 t/t5319-multi-pack-index.sh             | 1 +
 t/t5326-multi-pack-bitmaps.sh           | 3 ++-
 t/t5327-multi-pack-bitmaps-rev.sh       | 3 ++-
 t/t5331-pack-objects-stdin.sh           | 2 ++
 t/t5332-multi-pack-reuse.sh             | 1 +
 t/t5334-incremental-multi-pack-index.sh | 1 +
 t/t5500-fetch-pack.sh                   | 3 ++-
 t/t5616-partial-clone.sh                | 1 +
 t/t7700-repack.sh                       | 3 +++
 11 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/t/t0081-find-pack.sh b/t/t0081-find-pack.sh
index 5a628bf735..26f017422d 100755
--- a/t/t0081-find-pack.sh
+++ b/t/t0081-find-pack.sh
@@ -68,6 +68,7 @@ test_expect_success 'add more packfiles' '
 '
 
 test_expect_success 'add more commits (as loose objects)' '
+	test_config maintenance.auto false &&
 	test_commit six &&
 	test_commit seven &&
 
diff --git a/t/t5316-pack-delta-depth.sh b/t/t5316-pack-delta-depth.sh
index 03dfb7a61e..8a067a45cb 100755
--- a/t/t5316-pack-delta-depth.sh
+++ b/t/t5316-pack-delta-depth.sh
@@ -48,6 +48,7 @@ test_description='pack-objects breaks long cross-pack delta chains'
 # repeatedly-modified file to generate the delta chain).
 
 test_expect_success 'create series of packs' '
+	test_config maintenance.auto false &&
 	test-tool genrandom foo 4096 >content &&
 	prev= &&
 	for i in $(test_seq 1 10)
diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh
index faae98c7e7..7672d599d4 100755
--- a/t/t5319-multi-pack-index.sh
+++ b/t/t5319-multi-pack-index.sh
@@ -1315,6 +1315,7 @@ test_expect_success 'bitmapped packs are stored via the BTMP chunk' '
 	git init repo &&
 	(
 		cd repo &&
+		git config set maintenance.auto false &&
 
 		for i in 1 2 3 4 5
 		do
diff --git a/t/t5326-multi-pack-bitmaps.sh b/t/t5326-multi-pack-bitmaps.sh
index 892aeb09e4..62bd973d92 100755
--- a/t/t5326-multi-pack-bitmaps.sh
+++ b/t/t5326-multi-pack-bitmaps.sh
@@ -93,7 +93,8 @@ test_midx_bitmap_cases () {
 	test_expect_success 'setup test_repository' '
 		rm -rf * .git &&
 		git init &&
-		git config pack.writeBitmapLookupTable '"$writeLookupTable"'
+		git config pack.writeBitmapLookupTable '"$writeLookupTable"' &&
+		git config maintenance.auto false
 	'
 
 	midx_bitmap_core
diff --git a/t/t5327-multi-pack-bitmaps-rev.sh b/t/t5327-multi-pack-bitmaps-rev.sh
index 9cac03a94b..cfa12de2a8 100755
--- a/t/t5327-multi-pack-bitmaps-rev.sh
+++ b/t/t5327-multi-pack-bitmaps-rev.sh
@@ -30,7 +30,8 @@ test_midx_bitmap_rev () {
 	test_expect_success 'setup bitmap config' '
 		rm -rf * .git &&
 		git init &&
-		git config pack.writeBitmapLookupTable '"$writeLookupTable"'
+		git config pack.writeBitmapLookupTable '"$writeLookupTable"' &&
+		git config maintenance.auto false
 	'
 
 	midx_bitmap_core rev
diff --git a/t/t5331-pack-objects-stdin.sh b/t/t5331-pack-objects-stdin.sh
index cd949025b9..b03f6be164 100755
--- a/t/t5331-pack-objects-stdin.sh
+++ b/t/t5331-pack-objects-stdin.sh
@@ -14,6 +14,7 @@ packed_objects () {
 
 test_expect_success 'setup for --stdin-packs tests' '
 	git init stdin-packs &&
+	git -C stdin-packs config set maintenance.auto false &&
 	(
 		cd stdin-packs &&
 
@@ -255,6 +256,7 @@ test_expect_success '--stdin-packs=follow walks into unknown packs' '
 	git init repo &&
 	(
 		cd repo &&
+		git config set maintenance.auto false &&
 
 		for c in A B C D
 		do
diff --git a/t/t5332-multi-pack-reuse.sh b/t/t5332-multi-pack-reuse.sh
index 395d09444c..881ce668e1 100755
--- a/t/t5332-multi-pack-reuse.sh
+++ b/t/t5332-multi-pack-reuse.sh
@@ -59,6 +59,7 @@ test_pack_objects_reused () {
 
 test_expect_success 'preferred pack is reused for single-pack reuse' '
 	test_config pack.allowPackReuse single &&
+	git config set maintenance.auto false &&
 
 	for i in A B
 	do
diff --git a/t/t5334-incremental-multi-pack-index.sh b/t/t5334-incremental-multi-pack-index.sh
index d30d7253d6..99c7d44d8e 100755
--- a/t/t5334-incremental-multi-pack-index.sh
+++ b/t/t5334-incremental-multi-pack-index.sh
@@ -15,6 +15,7 @@ midx_chain=$midxdir/multi-pack-index-chain
 
 test_expect_success 'convert non-incremental MIDX to incremental' '
 	test_commit base &&
+	git config set maintenance.auto false &&
 	git repack -ad &&
 	git multi-pack-index write &&
 
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index 4bb56c167a..0c88d04d0a 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -154,7 +154,8 @@ test_expect_success 'clone shallow depth 1 with fsck' '
 '
 
 test_expect_success 'clone shallow' '
-	git clone --no-single-branch --depth 2 "file://$(pwd)/." shallow
+	git clone --no-single-branch --depth 2 "file://$(pwd)/." shallow &&
+	git -C shallow config set maintenance.auto false
 '
 
 test_expect_success 'clone shallow depth count' '
diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh
index d62760eb92..1c2805acca 100755
--- a/t/t5616-partial-clone.sh
+++ b/t/t5616-partial-clone.sh
@@ -585,6 +585,7 @@ test_expect_success 'verify fetch downloads only one pack when updating refs' '
 	git clone --filter=blob:none "file://$(pwd)/srv.bare" pack-test &&
 	ls pack-test/.git/objects/pack/*pack >pack-list &&
 	test_line_count = 2 pack-list &&
+	test_config -C pack-test maintenance.auto false &&
 	for i in A B C
 	do
 		test_commit -C src $i &&
diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh
index 73b78bdd88..acc2589f21 100755
--- a/t/t7700-repack.sh
+++ b/t/t7700-repack.sh
@@ -217,6 +217,7 @@ test_expect_success 'repack --keep-pack' '
 		cd keep-pack &&
 		# avoid producing different packs due to delta/base choices
 		git config pack.window 0 &&
+		git config maintenance.auto false &&
 		P1=$(commit_and_pack 1) &&
 		P2=$(commit_and_pack 2) &&
 		P3=$(commit_and_pack 3) &&
@@ -260,6 +261,7 @@ test_expect_success 'repacking fails when missing .pack actually means missing o
 
 		# Avoid producing different packs due to delta/base choices
 		git config pack.window 0 &&
+		git config maintenance.auto false &&
 		P1=$(commit_and_pack 1) &&
 		P2=$(commit_and_pack 2) &&
 		P3=$(commit_and_pack 3) &&
@@ -534,6 +536,7 @@ test_expect_success 'setup for --write-midx tests' '
 	(
 		cd midx &&
 		git config core.multiPackIndex true &&
+		git config maintenance.auto false &&
 
 		test_commit base
 	)

-- 
2.53.0.414.gf7e9f6c205.dirty


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 3/8] t34xx: don't expire reflogs where it matters
  2026-02-20 10:15 [PATCH 0/8] builtin/maintenance: use "geometric" strategy by default Patrick Steinhardt
  2026-02-20 10:15 ` [PATCH 1/8] t: fix races caused by background maintenance Patrick Steinhardt
  2026-02-20 10:15 ` [PATCH 2/8] t: disable maintenance where we verify object database structure Patrick Steinhardt
@ 2026-02-20 10:15 ` Patrick Steinhardt
  2026-02-23  0:48   ` Derrick Stolee
  2026-02-23 16:15   ` Justin Tobler
  2026-02-20 10:15 ` [PATCH 4/8] t5400: explicitly use "gc" strategy Patrick Steinhardt
                   ` (6 subsequent siblings)
  9 siblings, 2 replies; 38+ messages in thread
From: Patrick Steinhardt @ 2026-02-20 10:15 UTC (permalink / raw)
  To: git; +Cc: Derrick Stolee, Taylor Blau

We have a couple of tests in the t34xx range that rely on reflogs. This
never really used to be a problem, but in a subsequent commit we will
change the default maintenance strategy from "gc" to "geometric", and
this will cause us to drop all reflogs in these tests.

This may seem surprising and like a bug at first, but it's actually not.
The main difference between these two strategies is that the "gc"
strategy will skip all maintenance in case the object database is in a
well-optimized state. The "geometric" strategy has separate subtasks
though, and the conditions for each of these tasks is evaluated on a
case by case basis. This means that even if the object database is in
good shape, we may still decide to expire reflogs.

So why is that a problem? The issue is that Git's test suite hardcodes
the committer and author dates to a date in 2005. Interestingly though,
these hardcoded dates not only impact the commits, but also the reflog
entries. The consequence is that all newly written reflog entries are
immediately considered stale as our reflog expiration threshold is in
the range of weeks, only. It follows that executing `git reflog expire`
will thus immediately purge all reflog entries.

This hasn't been a problem in our test suite by pure chance, as the
repository shapes simply didn't cause us to perform actual garbage
collection. But with the upcoming "geometric" strategy we _will_ start
to execute `git reflog expire`, thus surfacing this issue.

Prepare for this by explicitly disabling reflog expiration in tests
impacted by this upcoming change.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t3404-rebase-interactive.sh  | 2 ++
 t/t3406-rebase-message.sh      | 3 +++
 t/t3431-rebase-fork-point.sh   | 2 ++
 t/t3432-rebase-fast-forward.sh | 2 ++
 4 files changed, 9 insertions(+)

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index e778dd8ae4..5e4623f7f1 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -31,6 +31,8 @@ Initial setup:
 . "$TEST_DIRECTORY"/lib-rebase.sh
 
 test_expect_success 'setup' '
+	git config set gc.reflogExpire never &&
+	git config set gc.reflogExpireUnreachable never &&
 	git switch -C primary &&
 	test_commit A file1 &&
 	test_commit B file1 &&
diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh
index a1d7fa7f7c..f89209c8d9 100755
--- a/t/t3406-rebase-message.sh
+++ b/t/t3406-rebase-message.sh
@@ -8,6 +8,9 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 . ./test-lib.sh
 
 test_expect_success 'setup' '
+	git config set gc.reflogExpire never &&
+	git config set gc.reflogExpireUnreachable never &&
+
 	test_commit O fileO &&
 	test_commit X fileX &&
 	git branch fast-forward &&
diff --git a/t/t3431-rebase-fork-point.sh b/t/t3431-rebase-fork-point.sh
index be09fc78c1..3a3c3a70a5 100755
--- a/t/t3431-rebase-fork-point.sh
+++ b/t/t3431-rebase-fork-point.sh
@@ -17,6 +17,8 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 # C was formerly part of main but main was rewound to remove C
 #
 test_expect_success setup '
+	git config set gc.reflogExpire never &&
+	git config set gc.reflogExpireUnreachable never &&
 	test_commit A &&
 	test_commit B &&
 	test_commit C &&
diff --git a/t/t3432-rebase-fast-forward.sh b/t/t3432-rebase-fast-forward.sh
index 5086e14c02..6e8de6c7aa 100755
--- a/t/t3432-rebase-fast-forward.sh
+++ b/t/t3432-rebase-fast-forward.sh
@@ -11,6 +11,8 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 . ./test-lib.sh
 
 test_expect_success setup '
+	git config set gc.reflogExpire never &&
+	git config set gc.reflogExpireUnreachable never &&
 	test_commit A &&
 	test_commit B &&
 	test_commit C &&

-- 
2.53.0.414.gf7e9f6c205.dirty


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 4/8] t5400: explicitly use "gc" strategy
  2026-02-20 10:15 [PATCH 0/8] builtin/maintenance: use "geometric" strategy by default Patrick Steinhardt
                   ` (2 preceding siblings ...)
  2026-02-20 10:15 ` [PATCH 3/8] t34xx: don't expire reflogs where it matters Patrick Steinhardt
@ 2026-02-20 10:15 ` Patrick Steinhardt
  2026-02-20 10:15 ` [PATCH 5/8] t5510: " Patrick Steinhardt
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 38+ messages in thread
From: Patrick Steinhardt @ 2026-02-20 10:15 UTC (permalink / raw)
  To: git; +Cc: Derrick Stolee, Taylor Blau

In t5400 we verify that git-receive-pack(1) runs automated repository
maintenance in the remote repository. The check is performed indirectly
by observing an effect that git-gc(1) would have, namely to prune a
temporary object from the object database. In a subsequent commit we're
about to switch to the "geometric" strategy by default though, and here
we stop observing that effect.

Adapt the test to explicitly use the "gc" strategy to prepare for that
upcoming change.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t5400-send-pack.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh
index 83b42ff073..b32a0a6aa7 100755
--- a/t/t5400-send-pack.sh
+++ b/t/t5400-send-pack.sh
@@ -187,6 +187,7 @@ test_expect_success 'receive-pack runs auto-gc in remote repo' '
 		cd child &&
 		git config gc.autopacklimit 1 &&
 		git config gc.autodetach false &&
+		git config maintenance.strategy gc &&
 		git branch test_auto_gc &&
 		# And create a file that follows the temporary object naming
 		# convention for the auto-gc to remove

-- 
2.53.0.414.gf7e9f6c205.dirty


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 5/8] t5510: explicitly use "gc" strategy
  2026-02-20 10:15 [PATCH 0/8] builtin/maintenance: use "geometric" strategy by default Patrick Steinhardt
                   ` (3 preceding siblings ...)
  2026-02-20 10:15 ` [PATCH 4/8] t5400: explicitly use "gc" strategy Patrick Steinhardt
@ 2026-02-20 10:15 ` Patrick Steinhardt
  2026-02-20 10:15 ` [PATCH 6/8] t6500: " Patrick Steinhardt
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 38+ messages in thread
From: Patrick Steinhardt @ 2026-02-20 10:15 UTC (permalink / raw)
  To: git; +Cc: Derrick Stolee, Taylor Blau

One of the tests in t5510 wants to verify that auto-gc does not lock up
when fetching into a repository. Adapt it to explicitly pick the "gc"
strategy for auto-maintenance.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t5510-fetch.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index c69afb5a60..5dcb4b51a4 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -1321,6 +1321,7 @@ test_expect_success 'fetching with auto-gc does not lock up' '
 		git config fetch.unpackLimit 1 &&
 		git config gc.autoPackLimit 1 &&
 		git config gc.autoDetach false &&
+		git config maintenance.strategy gc &&
 		GIT_ASK_YESNO="$TRASH_DIRECTORY/askyesno" git fetch --verbose >fetch.out 2>&1 &&
 		test_grep "Auto packing the repository" fetch.out &&
 		! grep "Should I try again" fetch.out

-- 
2.53.0.414.gf7e9f6c205.dirty


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 6/8] t6500: explicitly use "gc" strategy
  2026-02-20 10:15 [PATCH 0/8] builtin/maintenance: use "geometric" strategy by default Patrick Steinhardt
                   ` (4 preceding siblings ...)
  2026-02-20 10:15 ` [PATCH 5/8] t5510: " Patrick Steinhardt
@ 2026-02-20 10:15 ` Patrick Steinhardt
  2026-02-20 10:15 ` [PATCH 7/8] t7900: prepare for switch of the default strategy Patrick Steinhardt
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 38+ messages in thread
From: Patrick Steinhardt @ 2026-02-20 10:15 UTC (permalink / raw)
  To: git; +Cc: Derrick Stolee, Taylor Blau

The test in t6500 explicitly wants to exercise git-gc(1) and is thus
highly specific to the actual on-disk state of the repository and
specifically of the object database. An upcoming change modifies the
default maintenance strategy to be the "geometric" strategy though,
which breaks a couple of assumptions.

One fix would arguably be to disable auto-maintenance altogether, as we
do want to explicitly verify git-gc(1) anyway. But as the whole test
suite is about git-gc(1) in the first place it feels more sensible to
configure the default maintenance strategy to be "gc".

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t6500-gc.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh
index bef472cb8d..ea9aaad470 100755
--- a/t/t6500-gc.sh
+++ b/t/t6500-gc.sh
@@ -11,6 +11,7 @@ test_expect_success 'setup' '
 	# behavior, make sure we always pack everything to one pack by
 	# default
 	git config gc.bigPackThreshold 2g &&
+	git config set --global maintenance.strategy gc &&
 	test_oid_init
 '
 

-- 
2.53.0.414.gf7e9f6c205.dirty


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 7/8] t7900: prepare for switch of the default strategy
  2026-02-20 10:15 [PATCH 0/8] builtin/maintenance: use "geometric" strategy by default Patrick Steinhardt
                   ` (5 preceding siblings ...)
  2026-02-20 10:15 ` [PATCH 6/8] t6500: " Patrick Steinhardt
@ 2026-02-20 10:15 ` Patrick Steinhardt
  2026-02-20 10:15 ` [PATCH 8/8] builtin/maintenance: use "geometric" strategy by default Patrick Steinhardt
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 38+ messages in thread
From: Patrick Steinhardt @ 2026-02-20 10:15 UTC (permalink / raw)
  To: git; +Cc: Derrick Stolee, Taylor Blau

The t7900 test suite is exercising git-maintenance(1) and is thus of
course heavily reliant on the exact maintenance strategy. This reliance
comes in two flavors:

  - One test explicitly wants to verify that git-gc(1) is run as part of
    `git maintenance run`. This test is adapted by explicitly picking the
    "gc" strategy.

  - The other tests assume a specific shape of the object database,
    which is dependent on whether or not we run auto-maintenance before
    we come to the actual subject under test. These tests are adapted by
    disabling auto-maintenance.

With these changes t7900 passes with both "gc" and "geometric" default
strategies.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t7900-maintenance.sh | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index d11d6f8f15..63276dcc5f 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -43,7 +43,8 @@ test_expect_success 'help text' '
 	test_grep "usage: git maintenance" err
 '
 
-test_expect_success 'run [--auto|--quiet]' '
+test_expect_success 'run [--auto|--quiet] with gc strategy' '
+	test_config maintenance.strategy gc &&
 	GIT_TRACE2_EVENT="$(pwd)/run-no-auto.txt" \
 		git maintenance run 2>/dev/null &&
 	GIT_TRACE2_EVENT="$(pwd)/run-auto.txt" \
@@ -497,6 +498,7 @@ test_expect_success 'maintenance.incremental-repack.auto' '
 	(
 		cd incremental-repack-true &&
 		git config core.multiPackIndex true &&
+		git config maintenance.auto false &&
 		run_incremental_repack_and_verify
 	)
 '
@@ -507,6 +509,7 @@ test_expect_success 'maintenance.incremental-repack.auto (when config is unset)'
 	(
 		cd incremental-repack-unset &&
 		test_unconfig core.multiPackIndex &&
+		git config maintenance.auto false &&
 		run_incremental_repack_and_verify
 	)
 '
@@ -617,6 +620,7 @@ test_expect_success 'geometric repacking with --auto' '
 	git init repo &&
 	(
 		cd repo &&
+		git config set maintenance.auto false &&
 
 		# An empty repository does not need repacking, except when
 		# explicitly told to do it.

-- 
2.53.0.414.gf7e9f6c205.dirty


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 8/8] builtin/maintenance: use "geometric" strategy by default
  2026-02-20 10:15 [PATCH 0/8] builtin/maintenance: use "geometric" strategy by default Patrick Steinhardt
                   ` (6 preceding siblings ...)
  2026-02-20 10:15 ` [PATCH 7/8] t7900: prepare for switch of the default strategy Patrick Steinhardt
@ 2026-02-20 10:15 ` Patrick Steinhardt
  2026-02-23  0:52   ` Derrick Stolee
  2026-02-23 16:48   ` Justin Tobler
  2026-02-23  0:53 ` [PATCH 0/8] " Derrick Stolee
  2026-02-24  8:45 ` [PATCH v2 " Patrick Steinhardt
  9 siblings, 2 replies; 38+ messages in thread
From: Patrick Steinhardt @ 2026-02-20 10:15 UTC (permalink / raw)
  To: git; +Cc: Derrick Stolee, Taylor Blau

The git-gc(1) command has been introduced in the early days of Git in
30f610b7b0 (Create 'git gc' to perform common maintenance operations.,
2006-12-27) as the main repository maintenance utility. And while the
tool has of course evolved since then to cover new parts, the basic
strategy it uses has never really changed much.

It is safe to say that since 2006 the Git ecosystem has changed quite a
bit. Repositories tend to be much larger nowadays than they have been
almost 20 years ago, and large parts of the industry went crazy for
monorepos (for various wildly different definitions of "monorepo"). So
the maintenance strategy we used back then may not be the best fit
nowadays anymore.

Arguably, most of the maintenance tasks that git-gc(1) does are still
perfectly fine today: repacking references, expiring various data
structures and things like tend to not cause huge problems. But the big
exception is the way we repack objects.

git-gc(1) by default uses a split strategy: it performs incremental
repacks by default, and then whenever we have too many packs we perform
a large all-into-one repack. This all-into-one repack is what is causing
problems nowadays, as it is an operation that is quite expensive. While
it is wasteful in small- and medium-sized repositories, in large repos
it may even be prohibitively expensive.

We have eventually introduced git-maintenance(1) that was slated as a
replacement for git-gc(1). In contrast to git-gc(1), it was much more
flexible as it is structured around configurable tasks and strategies.
And while it knows about the "incremental" strategy that we may use for
scheduled maintenance when configured via Scalar, its default still is
to use git-gc(1) in the background.

The "incremental" strategy isn't really a full replacement for git-gc(1)
though, as it doesn't know to expire unused data structures. In Git 2.52
we have thus introduced a new "geometric" strategy that is a proper
replacement for the old git-gc(1).

In contrast to the incremental/all-into-one split used by git-gc(1), the
new "geometric" strategy maintains a geometric progression of packfiles,
which significantly reduces the number of all-into-one repacks that we
have to perform in large repositories. It is thus a much better fit for
large repositories than git-gc(1).

Note that the "geometric" strategy isn't perfect though: while we
perform way less all-into-one repacks compared to git-gc(1), we still
have to perform them eventually. But for the largest repositories out
there this may not be an option, as client machines might not be
powerful enough to perform such a repack in the first place. These cases
would thus still be covered by Scalar's "incremental" strategy.

Switch the default strategy away from "gc" to "geometric", but retain
the "incremental" strategy configured by Scalar.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/gc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index 4390eee6ec..fb329c2cff 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -1980,7 +1980,7 @@ static void initialize_task_config(struct maintenance_run_opts *opts,
 		strategy = none_strategy;
 		type = MAINTENANCE_TYPE_SCHEDULED;
 	} else {
-		strategy = gc_strategy;
+		strategy = geometric_strategy;
 		type = MAINTENANCE_TYPE_MANUAL;
 	}
 

-- 
2.53.0.414.gf7e9f6c205.dirty


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* Re: [PATCH 3/8] t34xx: don't expire reflogs where it matters
  2026-02-20 10:15 ` [PATCH 3/8] t34xx: don't expire reflogs where it matters Patrick Steinhardt
@ 2026-02-23  0:48   ` Derrick Stolee
  2026-02-23 16:15   ` Justin Tobler
  1 sibling, 0 replies; 38+ messages in thread
From: Derrick Stolee @ 2026-02-23  0:48 UTC (permalink / raw)
  To: Patrick Steinhardt, git; +Cc: Taylor Blau

On 2/20/26 5:15 AM, Patrick Steinhardt wrote:
> We have a couple of tests in the t34xx range that rely on reflogs. This
> never really used to be a problem, but in a subsequent commit we will
> change the default maintenance strategy from "gc" to "geometric", and
> this will cause us to drop all reflogs in these tests.
> 
> This may seem surprising and like a bug at first, but it's actually not.
> The main difference between these two strategies is that the "gc"
> strategy will skip all maintenance in case the object database is in a
> well-optimized state. The "geometric" strategy has separate subtasks
> though, and the conditions for each of these tasks is evaluated on a
> case by case basis. This means that even if the object database is in
> good shape, we may still decide to expire reflogs.
> 
> So why is that a problem? The issue is that Git's test suite hardcodes
> the committer and author dates to a date in 2005. Interestingly though,
> these hardcoded dates not only impact the commits, but also the reflog
> entries. The consequence is that all newly written reflog entries are
> immediately considered stale as our reflog expiration threshold is in
> the range of weeks, only. It follows that executing `git reflog expire`
> will thus immediately purge all reflog entries.

I found these two paragraphs very valuable in explaining this patch.

Thanks!
-Stolee


^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 8/8] builtin/maintenance: use "geometric" strategy by default
  2026-02-20 10:15 ` [PATCH 8/8] builtin/maintenance: use "geometric" strategy by default Patrick Steinhardt
@ 2026-02-23  0:52   ` Derrick Stolee
  2026-02-23  9:49     ` Patrick Steinhardt
  2026-02-23 16:48   ` Justin Tobler
  1 sibling, 1 reply; 38+ messages in thread
From: Derrick Stolee @ 2026-02-23  0:52 UTC (permalink / raw)
  To: Patrick Steinhardt, git; +Cc: Taylor Blau

On 2/20/26 5:15 AM, Patrick Steinhardt wrote:
> The git-gc(1) command has been introduced in the early days of Git in
> 30f610b7b0 (Create 'git gc' to perform common maintenance operations.,
> 2006-12-27) as the main repository maintenance utility. And while the
> tool has of course evolved since then to cover new parts, the basic
> strategy it uses has never really changed much.

I agree that the 'gc' strategy no longer serves users as a good default.
For those that want foreground commands to trigger maintenance (detached
on Unix, and as a blocking child on Windows) the 'geometric' strategy is
a good one.

> Switch the default strategy away from "gc" to "geometric", but retain
> the "incremental" strategy configured by Scalar.

Instead of "configured by Scalar" I'd say instead "configured when
initializing background maintenance with 'git maintenance start'" which
is how how Scalar sets this up indirectly.

Users could still opt-in to 'geometric' in the background, but it
would cause difficulties for the largest of repos that rely on the
'incremental' strategy's limit of the amount of data processed.

>   	} else {
> -		strategy = gc_strategy;
> +		strategy = geometric_strategy;
>   		type = MAINTENANCE_TYPE_MANUAL;
>   	}

Should this include some kind of documentation update in
Documentation/config/maintenance.adoc?

Thanks,
-Stolee



^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 0/8] builtin/maintenance: use "geometric" strategy by default
  2026-02-20 10:15 [PATCH 0/8] builtin/maintenance: use "geometric" strategy by default Patrick Steinhardt
                   ` (7 preceding siblings ...)
  2026-02-20 10:15 ` [PATCH 8/8] builtin/maintenance: use "geometric" strategy by default Patrick Steinhardt
@ 2026-02-23  0:53 ` Derrick Stolee
  2026-02-24  8:45 ` [PATCH v2 " Patrick Steinhardt
  9 siblings, 0 replies; 38+ messages in thread
From: Derrick Stolee @ 2026-02-23  0:53 UTC (permalink / raw)
  To: Patrick Steinhardt, git; +Cc: Taylor Blau

On 2/20/26 5:15 AM, Patrick Steinhardt wrote:
> Hi,
> 
> this series converts our default strategy used by git-maintenance(1)
> from "gc" to "geometric". The aim of this is twofold:
> 
>    - It completes the conversion to a more flexible infrastructure for
>      repository maintenance. git-maintenance(1) is structured around
>      tasks that can be toggled on/off as needed, and this is a lot easier
>      to extend going forward.
> 
>    - We start to use a more efficient repacking strategy by default,
>      which should especially help large repositories out there.
> 
> Out of these two, I think that the first point is actually the more
> important one.

I fully support this change as implemented, though I had a nit about
the final commit message and think there should be a documentation update
for this.

> Unfortunately, a lot of our tests are racy or will fail with the new
> strategy. This is mostly because the new strategy may decide to optimize
> data structures in cases where the old strategy didn't, and because the
> tasks we perform might be different. The majority of this patch series
> thus adapts our tests accordingly. The actual change is a one-line
> change in the final commit.

The patches that update these tests all look sensible. Thanks for the
extra care in explaining some tricky bits.

Thanks,
-Stolee


^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 8/8] builtin/maintenance: use "geometric" strategy by default
  2026-02-23  0:52   ` Derrick Stolee
@ 2026-02-23  9:49     ` Patrick Steinhardt
  0 siblings, 0 replies; 38+ messages in thread
From: Patrick Steinhardt @ 2026-02-23  9:49 UTC (permalink / raw)
  To: Derrick Stolee; +Cc: git, Taylor Blau

On Sun, Feb 22, 2026 at 07:52:40PM -0500, Derrick Stolee wrote:
> On 2/20/26 5:15 AM, Patrick Steinhardt wrote:
> > The git-gc(1) command has been introduced in the early days of Git in
> > 30f610b7b0 (Create 'git gc' to perform common maintenance operations.,
> > 2006-12-27) as the main repository maintenance utility. And while the
> > tool has of course evolved since then to cover new parts, the basic
> > strategy it uses has never really changed much.
> 
> I agree that the 'gc' strategy no longer serves users as a good default.
> For those that want foreground commands to trigger maintenance (detached
> on Unix, and as a blocking child on Windows) the 'geometric' strategy is
> a good one.
> 
> > Switch the default strategy away from "gc" to "geometric", but retain
> > the "incremental" strategy configured by Scalar.
> 
> Instead of "configured by Scalar" I'd say instead "configured when
> initializing background maintenance with 'git maintenance start'" which
> is how how Scalar sets this up indirectly.
> 
> Users could still opt-in to 'geometric' in the background, but it
> would cause difficulties for the largest of repos that rely on the
> 'incremental' strategy's limit of the amount of data processed.

Makes sense, will rephrase.

> >   	} else {
> > -		strategy = gc_strategy;
> > +		strategy = geometric_strategy;
> >   		type = MAINTENANCE_TYPE_MANUAL;
> >   	}
> 
> Should this include some kind of documentation update in
> Documentation/config/maintenance.adoc?

Oh, right, it definitely should!

Patrick

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 1/8] t: fix races caused by background maintenance
  2026-02-20 10:15 ` [PATCH 1/8] t: fix races caused by background maintenance Patrick Steinhardt
@ 2026-02-23 16:01   ` Justin Tobler
  2026-08-10  4:43   ` Stefan Haller
  1 sibling, 0 replies; 38+ messages in thread
From: Justin Tobler @ 2026-02-23 16:01 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git, Derrick Stolee, Taylor Blau

On 26/02/20 11:15AM, Patrick Steinhardt wrote:
> Many Git commands spawn git-maintenance(1) to optimize the repository in
> the background. By default, performing the maintenance is for most of
> the part asynchronous: we fork the executable and then continue with the
> rest of our business logic.
> 
> This is working as expected for our users, but this behaviour is
> somewhat problematic for our test suite as this is inherently racy. We
> have many tests that verify the on-disk state of repositories, and those
> tests may easily race with our background maintenance. In a similar
> fashion, we may end up with processes that "leak" out of a current test
> case.
> 
> Until now this tends to not be much of a problem. Our maintenance uses
> git-gc(1) by default, which knows to bail out in case there aren't
> either too many packfiles or too many loose objects. So even if other
> data structures would need to be optimized, we won't do so unless the
> object database also needs optimizations.
> 
> This is about to change though, as a subsequent commit will switch to
> the "geometric" maintenance strategy as a default. The consequence is
> that we will run required optimizations even if the object database is
> well-optimized. And this uncovers races between our test suite and
> background maintenance all over the place.
> 
> Disabling maintenance outright in our test suite is not really an
> option, as it would result in significantly divergence from the "real

s/significantly/significant/

> world" and reduce our test coverage. But we've got an alternative up our
> sleeves: we can ensure that garbage collection runs synchronously by
> overriding the "maintenance.autoDetach" configuration.
> 
> Of course that also diverges from the real world, as we now stop testing
> that background maintenance interacts in a benign way with normal Git
> commands. But on the other hand this ensures that the maintenance itself
> does not for example lead to data loss in a more reproducible way.
> 
> Another concern is that this would make execution of the test suite much
> slower. But a quick benchmark on my machine demonstrates that this does
> not seem to be the case:
> 
>     Benchmark 1: meson test (revision = HEAD~)
>       Time (mean ± σ):     131.182 s ±  1.293 s    [User: 853.737 s, System: 1160.479 s]
>       Range (min … max):   130.001 s … 132.563 s    3 runs
> 
>     Benchmark 2: meson test (revision = HEAD)
>       Time (mean ± σ):     129.554 s ±  0.507 s    [User: 849.040 s, System: 1152.664 s]
>       Range (min … max):   129.000 s … 129.994 s    3 runs
> 
>     Summary
>       meson test (revision = HEAD) ran
>         1.01 ± 0.01 times faster than meson test (revision = HEAD~)
> 
> Funny enough, it even seems as if this speeds up test execution ever so
> slightly, but that may just as well be noise.
> 
> Introduce a new `GIT_TEST_MAINT_AUTO_DETACH` environment variable that
> allows us to override the auto-detach behaviour and set that varibale in

s/varibale/variable/

> our tests.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  run-command.c            | 2 +-
>  t/t5616-partial-clone.sh | 6 +++---
>  t/t7900-maintenance.sh   | 1 +
>  t/test-lib.sh            | 4 ++++
>  4 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/run-command.c b/run-command.c
> index e3e02475cc..438a290d30 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -1828,7 +1828,7 @@ int prepare_auto_maintenance(int quiet, struct child_process *maint)
>  	 */
>  	if (repo_config_get_bool(the_repository, "maintenance.autodetach", &auto_detach) &&
>  	    repo_config_get_bool(the_repository, "gc.autodetach", &auto_detach))
> -		auto_detach = 1;
> +		auto_detach = git_env_bool("GIT_TEST_MAINT_AUTO_DETACH", true);

So now if "maintenance.autodetach" and "gc.autodetach" are both not set,
we then check for the "GIT_TEST_MAINT_AUTO_DETACH" env before defaulting
to true. Looks good.

>  
>  	maint->git_cmd = 1;
>  	maint->close_object_store = 1;
[snip]
> diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
> index 7cc0ce57f8..d11d6f8f15 100755
> --- a/t/t7900-maintenance.sh
> +++ b/t/t7900-maintenance.sh
> @@ -6,6 +6,7 @@ test_description='git maintenance builtin'
>  
>  GIT_TEST_COMMIT_GRAPH=0
>  GIT_TEST_MULTI_PACK_INDEX=0
> +sane_unset GIT_TEST_MAINT_AUTO_DETACH

I assume here we are unsetting the env for testing purposes. It might be
nice to leave some sort of breadcrumb comment here to explain to future
readers.

>  test_lazy_prereq XMLLINT '
>  	xmllint --version
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 0fb76f7d11..aa805a01ce 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -1947,6 +1947,10 @@ test_lazy_prereq COMPAT_HASH '
>  GIT_TEST_MAINT_SCHEDULER="none:exit 1"
>  export GIT_TEST_MAINT_SCHEDULER
>  
> +# Ensure that tests cannot race with background maintenance by default.
> +GIT_TEST_MAINT_AUTO_DETACH="false"
> +export GIT_TEST_MAINT_AUTO_DETACH

Looks good.

-Justin

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 2/8] t: disable maintenance where we verify object database structure
  2026-02-20 10:15 ` [PATCH 2/8] t: disable maintenance where we verify object database structure Patrick Steinhardt
@ 2026-02-23 16:07   ` Justin Tobler
  0 siblings, 0 replies; 38+ messages in thread
From: Justin Tobler @ 2026-02-23 16:07 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git, Derrick Stolee, Taylor Blau

On 26/02/20 11:15AM, Patrick Steinhardt wrote:
> We have a couple of tests that explicitly verify the structure of the
> object database. Naturally, this structure is dependent on whether or
> not we run repository maintenance: if it decides to optimize the object
> database the expected structure is likely to not materialize.
> 
> Explicitly disable auto-maintenance in such tests so that we are not
> dependent on decisions made by our maintenance.

I assume that these tests previously did not trigger maintenance to
begin with so now explicitly disabling maintenance does not change the
resulting structure. Changing the default to geometric repacking may
change this though.

-Justin

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 3/8] t34xx: don't expire reflogs where it matters
  2026-02-20 10:15 ` [PATCH 3/8] t34xx: don't expire reflogs where it matters Patrick Steinhardt
  2026-02-23  0:48   ` Derrick Stolee
@ 2026-02-23 16:15   ` Justin Tobler
  1 sibling, 0 replies; 38+ messages in thread
From: Justin Tobler @ 2026-02-23 16:15 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git, Derrick Stolee, Taylor Blau

On 26/02/20 11:15AM, Patrick Steinhardt wrote:
> We have a couple of tests in the t34xx range that rely on reflogs. This
> never really used to be a problem, but in a subsequent commit we will
> change the default maintenance strategy from "gc" to "geometric", and
> this will cause us to drop all reflogs in these tests.
> 
> This may seem surprising and like a bug at first, but it's actually not.
> The main difference between these two strategies is that the "gc"
> strategy will skip all maintenance in case the object database is in a
> well-optimized state. The "geometric" strategy has separate subtasks
> though, and the conditions for each of these tasks is evaluated on a
> case by case basis. This means that even if the object database is in
> good shape, we may still decide to expire reflogs.
> 
> So why is that a problem? The issue is that Git's test suite hardcodes
> the committer and author dates to a date in 2005. Interestingly though,
> these hardcoded dates not only impact the commits, but also the reflog
> entries. The consequence is that all newly written reflog entries are
> immediately considered stale as our reflog expiration threshold is in
> the range of weeks, only. It follows that executing `git reflog expire`
> will thus immediately purge all reflog entries.
> 
> This hasn't been a problem in our test suite by pure chance, as the
> repository shapes simply didn't cause us to perform actual garbage
> collection. But with the upcoming "geometric" strategy we _will_ start
> to execute `git reflog expire`, thus surfacing this issue.

Interesting find.

> Prepare for this by explicitly disabling reflog expiration in tests
> impacted by this upcoming change.

Makes sense.

> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  t/t3404-rebase-interactive.sh  | 2 ++
>  t/t3406-rebase-message.sh      | 3 +++
>  t/t3431-rebase-fork-point.sh   | 2 ++
>  t/t3432-rebase-fast-forward.sh | 2 ++
>  4 files changed, 9 insertions(+)
> 
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index e778dd8ae4..5e4623f7f1 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -31,6 +31,8 @@ Initial setup:
>  . "$TEST_DIRECTORY"/lib-rebase.sh
>  
>  test_expect_success 'setup' '
> +	git config set gc.reflogExpire never &&
> +	git config set gc.reflogExpireUnreachable never &&

As it may not be immediately obvious, it could be helpful for future
readers to explain in a comment that reflog dates are hardcoded to a
date that would be immediately expired and thus the need for this
configuration.

This patch looks good to me.

-Justin

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 8/8] builtin/maintenance: use "geometric" strategy by default
  2026-02-20 10:15 ` [PATCH 8/8] builtin/maintenance: use "geometric" strategy by default Patrick Steinhardt
  2026-02-23  0:52   ` Derrick Stolee
@ 2026-02-23 16:48   ` Justin Tobler
  2026-02-24  8:15     ` Patrick Steinhardt
  1 sibling, 1 reply; 38+ messages in thread
From: Justin Tobler @ 2026-02-23 16:48 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git, Derrick Stolee, Taylor Blau

On 26/02/20 11:15AM, Patrick Steinhardt wrote:
> The git-gc(1) command has been introduced in the early days of Git in
> 30f610b7b0 (Create 'git gc' to perform common maintenance operations.,
> 2006-12-27) as the main repository maintenance utility. And while the
> tool has of course evolved since then to cover new parts, the basic
> strategy it uses has never really changed much.
> 
> It is safe to say that since 2006 the Git ecosystem has changed quite a
> bit. Repositories tend to be much larger nowadays than they have been
> almost 20 years ago, and large parts of the industry went crazy for
> monorepos (for various wildly different definitions of "monorepo"). So
> the maintenance strategy we used back then may not be the best fit
> nowadays anymore.
> 
> Arguably, most of the maintenance tasks that git-gc(1) does are still
> perfectly fine today: repacking references, expiring various data
> structures and things like tend to not cause huge problems. But the big
> exception is the way we repack objects.
> 
> git-gc(1) by default uses a split strategy: it performs incremental
> repacks by default, and then whenever we have too many packs we perform
> a large all-into-one repack. This all-into-one repack is what is causing
> problems nowadays, as it is an operation that is quite expensive. While
> it is wasteful in small- and medium-sized repositories, in large repos
> it may even be prohibitively expensive.
> 
> We have eventually introduced git-maintenance(1) that was slated as a
> replacement for git-gc(1). In contrast to git-gc(1), it was much more
> flexible as it is structured around configurable tasks and strategies.
> And while it knows about the "incremental" strategy that we may use for
> scheduled maintenance when configured via Scalar, its default still is
> to use git-gc(1) in the background.

I'm a tad bit confused here. git-gc(1) by default uses an
"incremental/all-into-one" strategy and it is my understanding that this
is what git-maintenance(1) is currently using. Is there also another
"incremental" strategy for git-maintenance(1)?

> The "incremental" strategy isn't really a full replacement for git-gc(1)
> though, as it doesn't know to expire unused data structures. In Git 2.52
> we have thus introduced a new "geometric" strategy that is a proper
> replacement for the old git-gc(1).
> 
> In contrast to the incremental/all-into-one split used by git-gc(1), the
> new "geometric" strategy maintains a geometric progression of packfiles,
> which significantly reduces the number of all-into-one repacks that we
> have to perform in large repositories. It is thus a much better fit for
> large repositories than git-gc(1).
> 
> Note that the "geometric" strategy isn't perfect though: while we
> perform way less all-into-one repacks compared to git-gc(1), we still
> have to perform them eventually. But for the largest repositories out
> there this may not be an option, as client machines might not be
> powerful enough to perform such a repack in the first place. These cases
> would thus still be covered by Scalar's "incremental" strategy.

So the "problem" is the "all-into-one" repack. This ultimately occurs
for both the "gc" and "geometric" strategies so changing the default
strategy shouldn't make anything worse. Geometric repacking should in
fact delay the costly "all-into-one" repacks which is good.

> Switch the default strategy away from "gc" to "geometric", but retain
> the "incremental" strategy configured by Scalar.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  builtin/gc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/builtin/gc.c b/builtin/gc.c
> index 4390eee6ec..fb329c2cff 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -1980,7 +1980,7 @@ static void initialize_task_config(struct maintenance_run_opts *opts,
>  		strategy = none_strategy;
>  		type = MAINTENANCE_TYPE_SCHEDULED;
>  	} else {
> -		strategy = gc_strategy;
> +		strategy = geometric_strategy;

Looks good.

-Justin

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 8/8] builtin/maintenance: use "geometric" strategy by default
  2026-02-23 16:48   ` Justin Tobler
@ 2026-02-24  8:15     ` Patrick Steinhardt
  0 siblings, 0 replies; 38+ messages in thread
From: Patrick Steinhardt @ 2026-02-24  8:15 UTC (permalink / raw)
  To: Justin Tobler; +Cc: git, Derrick Stolee, Taylor Blau

On Mon, Feb 23, 2026 at 10:48:59AM -0600, Justin Tobler wrote:
> On 26/02/20 11:15AM, Patrick Steinhardt wrote:
> > The git-gc(1) command has been introduced in the early days of Git in
> > 30f610b7b0 (Create 'git gc' to perform common maintenance operations.,
> > 2006-12-27) as the main repository maintenance utility. And while the
> > tool has of course evolved since then to cover new parts, the basic
> > strategy it uses has never really changed much.
> > 
> > It is safe to say that since 2006 the Git ecosystem has changed quite a
> > bit. Repositories tend to be much larger nowadays than they have been
> > almost 20 years ago, and large parts of the industry went crazy for
> > monorepos (for various wildly different definitions of "monorepo"). So
> > the maintenance strategy we used back then may not be the best fit
> > nowadays anymore.
> > 
> > Arguably, most of the maintenance tasks that git-gc(1) does are still
> > perfectly fine today: repacking references, expiring various data
> > structures and things like tend to not cause huge problems. But the big
> > exception is the way we repack objects.
> > 
> > git-gc(1) by default uses a split strategy: it performs incremental
> > repacks by default, and then whenever we have too many packs we perform
> > a large all-into-one repack. This all-into-one repack is what is causing
> > problems nowadays, as it is an operation that is quite expensive. While
> > it is wasteful in small- and medium-sized repositories, in large repos
> > it may even be prohibitively expensive.
> > 
> > We have eventually introduced git-maintenance(1) that was slated as a
> > replacement for git-gc(1). In contrast to git-gc(1), it was much more
> > flexible as it is structured around configurable tasks and strategies.
> > And while it knows about the "incremental" strategy that we may use for
> > scheduled maintenance when configured via Scalar, its default still is
> > to use git-gc(1) in the background.
> 
> I'm a tad bit confused here. git-gc(1) by default uses an
> "incremental/all-into-one" strategy and it is my understanding that this
> is what git-maintenance(1) is currently using. Is there also another
> "incremental" strategy for git-maintenance(1)?

There are three strategies right now:

  - "gc", which is the current default strategy that uses git-gc(1) to
    perform the incremental packs followed by the all-into-one repacks.

  - "incremental", which is set up by `git maintenance start`. This
    strategy never performs the all-into-one repacks, and it instead
    creates packs of a fixed upper size.

  - "geometric", which is the strategy that'll become the new default.

I'll try to clarify this a bit.

Patrick

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCH v2 0/8] builtin/maintenance: use "geometric" strategy by default
  2026-02-20 10:15 [PATCH 0/8] builtin/maintenance: use "geometric" strategy by default Patrick Steinhardt
                   ` (8 preceding siblings ...)
  2026-02-23  0:53 ` [PATCH 0/8] " Derrick Stolee
@ 2026-02-24  8:45 ` Patrick Steinhardt
  2026-02-24  8:45   ` [PATCH v2 1/8] t: fix races caused by background maintenance Patrick Steinhardt
                     ` (8 more replies)
  9 siblings, 9 replies; 38+ messages in thread
From: Patrick Steinhardt @ 2026-02-24  8:45 UTC (permalink / raw)
  To: git; +Cc: Derrick Stolee, Taylor Blau, Justin Tobler

Hi,

this series converts our default strategy used by git-maintenance(1)
from "gc" to "geometric". The aim of this is twofold:

  - It completes the conversion to a more flexible infrastructure for
    repository maintenance. git-maintenance(1) is structured around
    tasks that can be toggled on/off as needed, and this is a lot easier
    to extend going forward.

  - We start to use a more efficient repacking strategy by default,
    which should especially help large repositories out there.

Out of these two, I think that the first point is actually the more
important one.

Unfortunately, a lot of our tests are racy or will fail with the new
strategy. This is mostly because the new strategy may decide to optimize
data structures in cases where the old strategy didn't, and because the
tasks we perform might be different. The majority of this patch series
thus adapts our tests accordingly. The actual change is a one-line
change in the final commit.

I was a bit torn initially whether or not I want to make the geometric
strategy the default right away, or whether we might first want to use
"feature.experimental" as an additional step. I'm quite happy to adapt
the series accordingly, but for the initial version I thought it might
invite more discussions if I pick the nuclear option :)

Of course, no matter how we do this, it is still possible to revert back
to the old strategy by setting "maintenance.strategy=gc".

Changes in v2:
  - Document the updated default strategy.
  - Clarify how this interacts with Scalar.
  - Explain the current landscape of strategies a bit better.
  - Leave some breadcrumbs in the tests.
  - Link to v1: https://lore.kernel.org/r/20260220-b4-pks-maintenance-default-geometric-strategy-v1-0-faeb321ad13b@pks.im

Thanks!

Patrick

---
Patrick Steinhardt (8):
      t: fix races caused by background maintenance
      t: disable maintenance where we verify object database structure
      t34xx: don't expire reflogs where it matters
      t5400: explicitly use "gc" strategy
      t5510: explicitly use "gc" strategy
      t6500: explicitly use "gc" strategy
      t7900: prepare for switch of the default strategy
      builtin/maintenance: use "geometric" strategy by default

 Documentation/config/maintenance.adoc   | 6 +++---
 builtin/gc.c                            | 2 +-
 run-command.c                           | 2 +-
 t/t0081-find-pack.sh                    | 1 +
 t/t3404-rebase-interactive.sh           | 6 ++++++
 t/t3406-rebase-message.sh               | 6 ++++++
 t/t3431-rebase-fork-point.sh            | 6 ++++++
 t/t3432-rebase-fast-forward.sh          | 6 ++++++
 t/t5316-pack-delta-depth.sh             | 1 +
 t/t5319-multi-pack-index.sh             | 1 +
 t/t5326-multi-pack-bitmaps.sh           | 3 ++-
 t/t5327-multi-pack-bitmaps-rev.sh       | 3 ++-
 t/t5331-pack-objects-stdin.sh           | 2 ++
 t/t5332-multi-pack-reuse.sh             | 1 +
 t/t5334-incremental-multi-pack-index.sh | 1 +
 t/t5400-send-pack.sh                    | 1 +
 t/t5500-fetch-pack.sh                   | 3 ++-
 t/t5510-fetch.sh                        | 1 +
 t/t5616-partial-clone.sh                | 7 ++++---
 t/t6500-gc.sh                           | 1 +
 t/t7700-repack.sh                       | 3 +++
 t/t7900-maintenance.sh                  | 9 ++++++++-
 t/test-lib.sh                           | 4 ++++
 23 files changed, 64 insertions(+), 12 deletions(-)

Range-diff versus v1:

1:  c5fadf42d0 ! 1:  9efc6d0a22 t: fix races caused by background maintenance
    @@ Commit message
         background maintenance all over the place.
     
         Disabling maintenance outright in our test suite is not really an
    -    option, as it would result in significantly divergence from the "real
    +    option, as it would result in significant divergence from the "real
         world" and reduce our test coverage. But we've got an alternative up our
         sleeves: we can ensure that garbage collection runs synchronously by
         overriding the "maintenance.autoDetach" configuration.
    @@ Commit message
         slightly, but that may just as well be noise.
     
         Introduce a new `GIT_TEST_MAINT_AUTO_DETACH` environment variable that
    -    allows us to override the auto-detach behaviour and set that varibale in
    +    allows us to override the auto-detach behaviour and set that variable in
         our tests.
     
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
    @@ t/t5616-partial-clone.sh: test_expect_success 'fetch --refetch triggers repackin
     
      ## t/t7900-maintenance.sh ##
     @@ t/t7900-maintenance.sh: test_description='git maintenance builtin'
    - 
      GIT_TEST_COMMIT_GRAPH=0
      GIT_TEST_MULTI_PACK_INDEX=0
    -+sane_unset GIT_TEST_MAINT_AUTO_DETACH
      
    ++# Ensure that auto-maintenance detaches as usual.
    ++sane_unset GIT_TEST_MAINT_AUTO_DETACH
    ++
      test_lazy_prereq XMLLINT '
      	xmllint --version
    + '
     
      ## t/test-lib.sh ##
     @@ t/test-lib.sh: test_lazy_prereq COMPAT_HASH '
2:  805417a4a7 = 2:  f80bde1353 t: disable maintenance where we verify object database structure
3:  8a579a768d ! 3:  7087a68815 t34xx: don't expire reflogs where it matters
    @@ t/t3404-rebase-interactive.sh: Initial setup:
      . "$TEST_DIRECTORY"/lib-rebase.sh
      
      test_expect_success 'setup' '
    ++	# Commit dates are hardcoded to 2005, and the reflog entries will have
    ++	# a matching timestamp. Maintenance may thus immediately expire
    ++	# reflogs if it was running.
     +	git config set gc.reflogExpire never &&
     +	git config set gc.reflogExpireUnreachable never &&
    ++
      	git switch -C primary &&
      	test_commit A file1 &&
      	test_commit B file1 &&
    @@ t/t3406-rebase-message.sh: export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
      . ./test-lib.sh
      
      test_expect_success 'setup' '
    ++	# Commit dates are hardcoded to 2005, and the reflog entries will have
    ++	# a matching timestamp. Maintenance may thus immediately expire
    ++	# reflogs if it was running.
     +	git config set gc.reflogExpire never &&
     +	git config set gc.reflogExpireUnreachable never &&
     +
    @@ t/t3431-rebase-fork-point.sh: export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
      # C was formerly part of main but main was rewound to remove C
      #
      test_expect_success setup '
    ++	# Commit dates are hardcoded to 2005, and the reflog entries will have
    ++	# a matching timestamp. Maintenance may thus immediately expire
    ++	# reflogs if it was running.
     +	git config set gc.reflogExpire never &&
     +	git config set gc.reflogExpireUnreachable never &&
    ++
      	test_commit A &&
      	test_commit B &&
      	test_commit C &&
    @@ t/t3432-rebase-fast-forward.sh: export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
      . ./test-lib.sh
      
      test_expect_success setup '
    ++	# Commit dates are hardcoded to 2005, and the reflog entries will have
    ++	# a matching timestamp. Maintenance may thus immediately expire
    ++	# reflogs if it was running.
     +	git config set gc.reflogExpire never &&
     +	git config set gc.reflogExpireUnreachable never &&
    ++
      	test_commit A &&
      	test_commit B &&
      	test_commit C &&
4:  283143c1d8 = 4:  d230055b22 t5400: explicitly use "gc" strategy
5:  410dc16eb0 = 5:  dba219391f t5510: explicitly use "gc" strategy
6:  c4c8c5a7e4 = 6:  61bc1add2a t6500: explicitly use "gc" strategy
7:  93893cfee3 = 7:  b89505178d t7900: prepare for switch of the default strategy
8:  9e7aa390a5 < -:  ---------- builtin/maintenance: use "geometric" strategy by default
-:  ---------- > 8:  647d46a239 builtin/maintenance: use "geometric" strategy by default

---
base-commit: 73fd77805fc6406f31c36212846d9e2541d19321
change-id: 20260218-b4-pks-maintenance-default-geometric-strategy-17fcedf92461


^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCH v2 1/8] t: fix races caused by background maintenance
  2026-02-24  8:45 ` [PATCH v2 " Patrick Steinhardt
@ 2026-02-24  8:45   ` Patrick Steinhardt
  2026-02-24  8:45   ` [PATCH v2 2/8] t: disable maintenance where we verify object database structure Patrick Steinhardt
                     ` (7 subsequent siblings)
  8 siblings, 0 replies; 38+ messages in thread
From: Patrick Steinhardt @ 2026-02-24  8:45 UTC (permalink / raw)
  To: git; +Cc: Derrick Stolee, Taylor Blau, Justin Tobler

Many Git commands spawn git-maintenance(1) to optimize the repository in
the background. By default, performing the maintenance is for most of
the part asynchronous: we fork the executable and then continue with the
rest of our business logic.

This is working as expected for our users, but this behaviour is
somewhat problematic for our test suite as this is inherently racy. We
have many tests that verify the on-disk state of repositories, and those
tests may easily race with our background maintenance. In a similar
fashion, we may end up with processes that "leak" out of a current test
case.

Until now this tends to not be much of a problem. Our maintenance uses
git-gc(1) by default, which knows to bail out in case there aren't
either too many packfiles or too many loose objects. So even if other
data structures would need to be optimized, we won't do so unless the
object database also needs optimizations.

This is about to change though, as a subsequent commit will switch to
the "geometric" maintenance strategy as a default. The consequence is
that we will run required optimizations even if the object database is
well-optimized. And this uncovers races between our test suite and
background maintenance all over the place.

Disabling maintenance outright in our test suite is not really an
option, as it would result in significant divergence from the "real
world" and reduce our test coverage. But we've got an alternative up our
sleeves: we can ensure that garbage collection runs synchronously by
overriding the "maintenance.autoDetach" configuration.

Of course that also diverges from the real world, as we now stop testing
that background maintenance interacts in a benign way with normal Git
commands. But on the other hand this ensures that the maintenance itself
does not for example lead to data loss in a more reproducible way.

Another concern is that this would make execution of the test suite much
slower. But a quick benchmark on my machine demonstrates that this does
not seem to be the case:

    Benchmark 1: meson test (revision = HEAD~)
      Time (mean ± σ):     131.182 s ±  1.293 s    [User: 853.737 s, System: 1160.479 s]
      Range (min … max):   130.001 s … 132.563 s    3 runs

    Benchmark 2: meson test (revision = HEAD)
      Time (mean ± σ):     129.554 s ±  0.507 s    [User: 849.040 s, System: 1152.664 s]
      Range (min … max):   129.000 s … 129.994 s    3 runs

    Summary
      meson test (revision = HEAD) ran
        1.01 ± 0.01 times faster than meson test (revision = HEAD~)

Funny enough, it even seems as if this speeds up test execution ever so
slightly, but that may just as well be noise.

Introduce a new `GIT_TEST_MAINT_AUTO_DETACH` environment variable that
allows us to override the auto-detach behaviour and set that variable in
our tests.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 run-command.c            | 2 +-
 t/t5616-partial-clone.sh | 6 +++---
 t/t7900-maintenance.sh   | 3 +++
 t/test-lib.sh            | 4 ++++
 4 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/run-command.c b/run-command.c
index e3e02475cc..438a290d30 100644
--- a/run-command.c
+++ b/run-command.c
@@ -1828,7 +1828,7 @@ int prepare_auto_maintenance(int quiet, struct child_process *maint)
 	 */
 	if (repo_config_get_bool(the_repository, "maintenance.autodetach", &auto_detach) &&
 	    repo_config_get_bool(the_repository, "gc.autodetach", &auto_detach))
-		auto_detach = 1;
+		auto_detach = git_env_bool("GIT_TEST_MAINT_AUTO_DETACH", true);
 
 	maint->git_cmd = 1;
 	maint->close_object_store = 1;
diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh
index 1e354e057f..d62760eb92 100755
--- a/t/t5616-partial-clone.sh
+++ b/t/t5616-partial-clone.sh
@@ -229,7 +229,7 @@ test_expect_success 'fetch --refetch triggers repacking' '
 
 	GIT_TRACE2_EVENT="$PWD/trace1.event" \
 	git -C pc1 fetch --refetch origin &&
-	test_subcommand git maintenance run --auto --no-quiet --detach <trace1.event &&
+	test_subcommand git maintenance run --auto --no-quiet --no-detach <trace1.event &&
 	grep \"param\":\"gc.autopacklimit\",\"value\":\"1\" trace1.event &&
 	grep \"param\":\"maintenance.incremental-repack.auto\",\"value\":\"-1\" trace1.event &&
 
@@ -238,7 +238,7 @@ test_expect_success 'fetch --refetch triggers repacking' '
 		-c gc.autoPackLimit=0 \
 		-c maintenance.incremental-repack.auto=1234 \
 		-C pc1 fetch --refetch origin &&
-	test_subcommand git maintenance run --auto --no-quiet --detach <trace2.event &&
+	test_subcommand git maintenance run --auto --no-quiet --no-detach <trace2.event &&
 	grep \"param\":\"gc.autopacklimit\",\"value\":\"0\" trace2.event &&
 	grep \"param\":\"maintenance.incremental-repack.auto\",\"value\":\"-1\" trace2.event &&
 
@@ -247,7 +247,7 @@ test_expect_success 'fetch --refetch triggers repacking' '
 		-c gc.autoPackLimit=1234 \
 		-c maintenance.incremental-repack.auto=0 \
 		-C pc1 fetch --refetch origin &&
-	test_subcommand git maintenance run --auto --no-quiet --detach <trace3.event &&
+	test_subcommand git maintenance run --auto --no-quiet --no-detach <trace3.event &&
 	grep \"param\":\"gc.autopacklimit\",\"value\":\"1\" trace3.event &&
 	grep \"param\":\"maintenance.incremental-repack.auto\",\"value\":\"0\" trace3.event
 '
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 7cc0ce57f8..fe344f47ee 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -7,6 +7,9 @@ test_description='git maintenance builtin'
 GIT_TEST_COMMIT_GRAPH=0
 GIT_TEST_MULTI_PACK_INDEX=0
 
+# Ensure that auto-maintenance detaches as usual.
+sane_unset GIT_TEST_MAINT_AUTO_DETACH
+
 test_lazy_prereq XMLLINT '
 	xmllint --version
 '
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 0fb76f7d11..aa805a01ce 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1947,6 +1947,10 @@ test_lazy_prereq COMPAT_HASH '
 GIT_TEST_MAINT_SCHEDULER="none:exit 1"
 export GIT_TEST_MAINT_SCHEDULER
 
+# Ensure that tests cannot race with background maintenance by default.
+GIT_TEST_MAINT_AUTO_DETACH="false"
+export GIT_TEST_MAINT_AUTO_DETACH
+
 # Does this platform support `git fsmonitor--daemon`
 #
 test_lazy_prereq FSMONITOR_DAEMON '

-- 
2.53.0.536.g309c995771.dirty


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v2 2/8] t: disable maintenance where we verify object database structure
  2026-02-24  8:45 ` [PATCH v2 " Patrick Steinhardt
  2026-02-24  8:45   ` [PATCH v2 1/8] t: fix races caused by background maintenance Patrick Steinhardt
@ 2026-02-24  8:45   ` Patrick Steinhardt
  2026-02-24  8:45   ` [PATCH v2 3/8] t34xx: don't expire reflogs where it matters Patrick Steinhardt
                     ` (6 subsequent siblings)
  8 siblings, 0 replies; 38+ messages in thread
From: Patrick Steinhardt @ 2026-02-24  8:45 UTC (permalink / raw)
  To: git; +Cc: Derrick Stolee, Taylor Blau, Justin Tobler

We have a couple of tests that explicitly verify the structure of the
object database. Naturally, this structure is dependent on whether or
not we run repository maintenance: if it decides to optimize the object
database the expected structure is likely to not materialize.

Explicitly disable auto-maintenance in such tests so that we are not
dependent on decisions made by our maintenance.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t0081-find-pack.sh                    | 1 +
 t/t5316-pack-delta-depth.sh             | 1 +
 t/t5319-multi-pack-index.sh             | 1 +
 t/t5326-multi-pack-bitmaps.sh           | 3 ++-
 t/t5327-multi-pack-bitmaps-rev.sh       | 3 ++-
 t/t5331-pack-objects-stdin.sh           | 2 ++
 t/t5332-multi-pack-reuse.sh             | 1 +
 t/t5334-incremental-multi-pack-index.sh | 1 +
 t/t5500-fetch-pack.sh                   | 3 ++-
 t/t5616-partial-clone.sh                | 1 +
 t/t7700-repack.sh                       | 3 +++
 11 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/t/t0081-find-pack.sh b/t/t0081-find-pack.sh
index 5a628bf735..26f017422d 100755
--- a/t/t0081-find-pack.sh
+++ b/t/t0081-find-pack.sh
@@ -68,6 +68,7 @@ test_expect_success 'add more packfiles' '
 '
 
 test_expect_success 'add more commits (as loose objects)' '
+	test_config maintenance.auto false &&
 	test_commit six &&
 	test_commit seven &&
 
diff --git a/t/t5316-pack-delta-depth.sh b/t/t5316-pack-delta-depth.sh
index 03dfb7a61e..8a067a45cb 100755
--- a/t/t5316-pack-delta-depth.sh
+++ b/t/t5316-pack-delta-depth.sh
@@ -48,6 +48,7 @@ test_description='pack-objects breaks long cross-pack delta chains'
 # repeatedly-modified file to generate the delta chain).
 
 test_expect_success 'create series of packs' '
+	test_config maintenance.auto false &&
 	test-tool genrandom foo 4096 >content &&
 	prev= &&
 	for i in $(test_seq 1 10)
diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh
index faae98c7e7..7672d599d4 100755
--- a/t/t5319-multi-pack-index.sh
+++ b/t/t5319-multi-pack-index.sh
@@ -1315,6 +1315,7 @@ test_expect_success 'bitmapped packs are stored via the BTMP chunk' '
 	git init repo &&
 	(
 		cd repo &&
+		git config set maintenance.auto false &&
 
 		for i in 1 2 3 4 5
 		do
diff --git a/t/t5326-multi-pack-bitmaps.sh b/t/t5326-multi-pack-bitmaps.sh
index 892aeb09e4..62bd973d92 100755
--- a/t/t5326-multi-pack-bitmaps.sh
+++ b/t/t5326-multi-pack-bitmaps.sh
@@ -93,7 +93,8 @@ test_midx_bitmap_cases () {
 	test_expect_success 'setup test_repository' '
 		rm -rf * .git &&
 		git init &&
-		git config pack.writeBitmapLookupTable '"$writeLookupTable"'
+		git config pack.writeBitmapLookupTable '"$writeLookupTable"' &&
+		git config maintenance.auto false
 	'
 
 	midx_bitmap_core
diff --git a/t/t5327-multi-pack-bitmaps-rev.sh b/t/t5327-multi-pack-bitmaps-rev.sh
index 9cac03a94b..cfa12de2a8 100755
--- a/t/t5327-multi-pack-bitmaps-rev.sh
+++ b/t/t5327-multi-pack-bitmaps-rev.sh
@@ -30,7 +30,8 @@ test_midx_bitmap_rev () {
 	test_expect_success 'setup bitmap config' '
 		rm -rf * .git &&
 		git init &&
-		git config pack.writeBitmapLookupTable '"$writeLookupTable"'
+		git config pack.writeBitmapLookupTable '"$writeLookupTable"' &&
+		git config maintenance.auto false
 	'
 
 	midx_bitmap_core rev
diff --git a/t/t5331-pack-objects-stdin.sh b/t/t5331-pack-objects-stdin.sh
index cd949025b9..b03f6be164 100755
--- a/t/t5331-pack-objects-stdin.sh
+++ b/t/t5331-pack-objects-stdin.sh
@@ -14,6 +14,7 @@ packed_objects () {
 
 test_expect_success 'setup for --stdin-packs tests' '
 	git init stdin-packs &&
+	git -C stdin-packs config set maintenance.auto false &&
 	(
 		cd stdin-packs &&
 
@@ -255,6 +256,7 @@ test_expect_success '--stdin-packs=follow walks into unknown packs' '
 	git init repo &&
 	(
 		cd repo &&
+		git config set maintenance.auto false &&
 
 		for c in A B C D
 		do
diff --git a/t/t5332-multi-pack-reuse.sh b/t/t5332-multi-pack-reuse.sh
index 395d09444c..881ce668e1 100755
--- a/t/t5332-multi-pack-reuse.sh
+++ b/t/t5332-multi-pack-reuse.sh
@@ -59,6 +59,7 @@ test_pack_objects_reused () {
 
 test_expect_success 'preferred pack is reused for single-pack reuse' '
 	test_config pack.allowPackReuse single &&
+	git config set maintenance.auto false &&
 
 	for i in A B
 	do
diff --git a/t/t5334-incremental-multi-pack-index.sh b/t/t5334-incremental-multi-pack-index.sh
index d30d7253d6..99c7d44d8e 100755
--- a/t/t5334-incremental-multi-pack-index.sh
+++ b/t/t5334-incremental-multi-pack-index.sh
@@ -15,6 +15,7 @@ midx_chain=$midxdir/multi-pack-index-chain
 
 test_expect_success 'convert non-incremental MIDX to incremental' '
 	test_commit base &&
+	git config set maintenance.auto false &&
 	git repack -ad &&
 	git multi-pack-index write &&
 
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index 4bb56c167a..0c88d04d0a 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -154,7 +154,8 @@ test_expect_success 'clone shallow depth 1 with fsck' '
 '
 
 test_expect_success 'clone shallow' '
-	git clone --no-single-branch --depth 2 "file://$(pwd)/." shallow
+	git clone --no-single-branch --depth 2 "file://$(pwd)/." shallow &&
+	git -C shallow config set maintenance.auto false
 '
 
 test_expect_success 'clone shallow depth count' '
diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh
index d62760eb92..1c2805acca 100755
--- a/t/t5616-partial-clone.sh
+++ b/t/t5616-partial-clone.sh
@@ -585,6 +585,7 @@ test_expect_success 'verify fetch downloads only one pack when updating refs' '
 	git clone --filter=blob:none "file://$(pwd)/srv.bare" pack-test &&
 	ls pack-test/.git/objects/pack/*pack >pack-list &&
 	test_line_count = 2 pack-list &&
+	test_config -C pack-test maintenance.auto false &&
 	for i in A B C
 	do
 		test_commit -C src $i &&
diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh
index 73b78bdd88..acc2589f21 100755
--- a/t/t7700-repack.sh
+++ b/t/t7700-repack.sh
@@ -217,6 +217,7 @@ test_expect_success 'repack --keep-pack' '
 		cd keep-pack &&
 		# avoid producing different packs due to delta/base choices
 		git config pack.window 0 &&
+		git config maintenance.auto false &&
 		P1=$(commit_and_pack 1) &&
 		P2=$(commit_and_pack 2) &&
 		P3=$(commit_and_pack 3) &&
@@ -260,6 +261,7 @@ test_expect_success 'repacking fails when missing .pack actually means missing o
 
 		# Avoid producing different packs due to delta/base choices
 		git config pack.window 0 &&
+		git config maintenance.auto false &&
 		P1=$(commit_and_pack 1) &&
 		P2=$(commit_and_pack 2) &&
 		P3=$(commit_and_pack 3) &&
@@ -534,6 +536,7 @@ test_expect_success 'setup for --write-midx tests' '
 	(
 		cd midx &&
 		git config core.multiPackIndex true &&
+		git config maintenance.auto false &&
 
 		test_commit base
 	)

-- 
2.53.0.536.g309c995771.dirty


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v2 3/8] t34xx: don't expire reflogs where it matters
  2026-02-24  8:45 ` [PATCH v2 " Patrick Steinhardt
  2026-02-24  8:45   ` [PATCH v2 1/8] t: fix races caused by background maintenance Patrick Steinhardt
  2026-02-24  8:45   ` [PATCH v2 2/8] t: disable maintenance where we verify object database structure Patrick Steinhardt
@ 2026-02-24  8:45   ` Patrick Steinhardt
  2026-02-24  8:45   ` [PATCH v2 4/8] t5400: explicitly use "gc" strategy Patrick Steinhardt
                     ` (5 subsequent siblings)
  8 siblings, 0 replies; 38+ messages in thread
From: Patrick Steinhardt @ 2026-02-24  8:45 UTC (permalink / raw)
  To: git; +Cc: Derrick Stolee, Taylor Blau, Justin Tobler

We have a couple of tests in the t34xx range that rely on reflogs. This
never really used to be a problem, but in a subsequent commit we will
change the default maintenance strategy from "gc" to "geometric", and
this will cause us to drop all reflogs in these tests.

This may seem surprising and like a bug at first, but it's actually not.
The main difference between these two strategies is that the "gc"
strategy will skip all maintenance in case the object database is in a
well-optimized state. The "geometric" strategy has separate subtasks
though, and the conditions for each of these tasks is evaluated on a
case by case basis. This means that even if the object database is in
good shape, we may still decide to expire reflogs.

So why is that a problem? The issue is that Git's test suite hardcodes
the committer and author dates to a date in 2005. Interestingly though,
these hardcoded dates not only impact the commits, but also the reflog
entries. The consequence is that all newly written reflog entries are
immediately considered stale as our reflog expiration threshold is in
the range of weeks, only. It follows that executing `git reflog expire`
will thus immediately purge all reflog entries.

This hasn't been a problem in our test suite by pure chance, as the
repository shapes simply didn't cause us to perform actual garbage
collection. But with the upcoming "geometric" strategy we _will_ start
to execute `git reflog expire`, thus surfacing this issue.

Prepare for this by explicitly disabling reflog expiration in tests
impacted by this upcoming change.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t3404-rebase-interactive.sh  | 6 ++++++
 t/t3406-rebase-message.sh      | 6 ++++++
 t/t3431-rebase-fork-point.sh   | 6 ++++++
 t/t3432-rebase-fast-forward.sh | 6 ++++++
 4 files changed, 24 insertions(+)

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index e778dd8ae4..3e44562afa 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -31,6 +31,12 @@ Initial setup:
 . "$TEST_DIRECTORY"/lib-rebase.sh
 
 test_expect_success 'setup' '
+	# Commit dates are hardcoded to 2005, and the reflog entries will have
+	# a matching timestamp. Maintenance may thus immediately expire
+	# reflogs if it was running.
+	git config set gc.reflogExpire never &&
+	git config set gc.reflogExpireUnreachable never &&
+
 	git switch -C primary &&
 	test_commit A file1 &&
 	test_commit B file1 &&
diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh
index a1d7fa7f7c..bc51a9d3a7 100755
--- a/t/t3406-rebase-message.sh
+++ b/t/t3406-rebase-message.sh
@@ -8,6 +8,12 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 . ./test-lib.sh
 
 test_expect_success 'setup' '
+	# Commit dates are hardcoded to 2005, and the reflog entries will have
+	# a matching timestamp. Maintenance may thus immediately expire
+	# reflogs if it was running.
+	git config set gc.reflogExpire never &&
+	git config set gc.reflogExpireUnreachable never &&
+
 	test_commit O fileO &&
 	test_commit X fileX &&
 	git branch fast-forward &&
diff --git a/t/t3431-rebase-fork-point.sh b/t/t3431-rebase-fork-point.sh
index be09fc78c1..4336f417c2 100755
--- a/t/t3431-rebase-fork-point.sh
+++ b/t/t3431-rebase-fork-point.sh
@@ -17,6 +17,12 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 # C was formerly part of main but main was rewound to remove C
 #
 test_expect_success setup '
+	# Commit dates are hardcoded to 2005, and the reflog entries will have
+	# a matching timestamp. Maintenance may thus immediately expire
+	# reflogs if it was running.
+	git config set gc.reflogExpire never &&
+	git config set gc.reflogExpireUnreachable never &&
+
 	test_commit A &&
 	test_commit B &&
 	test_commit C &&
diff --git a/t/t3432-rebase-fast-forward.sh b/t/t3432-rebase-fast-forward.sh
index 5086e14c02..181d19dcc1 100755
--- a/t/t3432-rebase-fast-forward.sh
+++ b/t/t3432-rebase-fast-forward.sh
@@ -11,6 +11,12 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 . ./test-lib.sh
 
 test_expect_success setup '
+	# Commit dates are hardcoded to 2005, and the reflog entries will have
+	# a matching timestamp. Maintenance may thus immediately expire
+	# reflogs if it was running.
+	git config set gc.reflogExpire never &&
+	git config set gc.reflogExpireUnreachable never &&
+
 	test_commit A &&
 	test_commit B &&
 	test_commit C &&

-- 
2.53.0.536.g309c995771.dirty


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v2 4/8] t5400: explicitly use "gc" strategy
  2026-02-24  8:45 ` [PATCH v2 " Patrick Steinhardt
                     ` (2 preceding siblings ...)
  2026-02-24  8:45   ` [PATCH v2 3/8] t34xx: don't expire reflogs where it matters Patrick Steinhardt
@ 2026-02-24  8:45   ` Patrick Steinhardt
  2026-02-24  8:45   ` [PATCH v2 5/8] t5510: " Patrick Steinhardt
                     ` (4 subsequent siblings)
  8 siblings, 0 replies; 38+ messages in thread
From: Patrick Steinhardt @ 2026-02-24  8:45 UTC (permalink / raw)
  To: git; +Cc: Derrick Stolee, Taylor Blau, Justin Tobler

In t5400 we verify that git-receive-pack(1) runs automated repository
maintenance in the remote repository. The check is performed indirectly
by observing an effect that git-gc(1) would have, namely to prune a
temporary object from the object database. In a subsequent commit we're
about to switch to the "geometric" strategy by default though, and here
we stop observing that effect.

Adapt the test to explicitly use the "gc" strategy to prepare for that
upcoming change.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t5400-send-pack.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh
index 83b42ff073..b32a0a6aa7 100755
--- a/t/t5400-send-pack.sh
+++ b/t/t5400-send-pack.sh
@@ -187,6 +187,7 @@ test_expect_success 'receive-pack runs auto-gc in remote repo' '
 		cd child &&
 		git config gc.autopacklimit 1 &&
 		git config gc.autodetach false &&
+		git config maintenance.strategy gc &&
 		git branch test_auto_gc &&
 		# And create a file that follows the temporary object naming
 		# convention for the auto-gc to remove

-- 
2.53.0.536.g309c995771.dirty


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v2 5/8] t5510: explicitly use "gc" strategy
  2026-02-24  8:45 ` [PATCH v2 " Patrick Steinhardt
                     ` (3 preceding siblings ...)
  2026-02-24  8:45   ` [PATCH v2 4/8] t5400: explicitly use "gc" strategy Patrick Steinhardt
@ 2026-02-24  8:45   ` Patrick Steinhardt
  2026-02-24  8:45   ` [PATCH v2 6/8] t6500: " Patrick Steinhardt
                     ` (3 subsequent siblings)
  8 siblings, 0 replies; 38+ messages in thread
From: Patrick Steinhardt @ 2026-02-24  8:45 UTC (permalink / raw)
  To: git; +Cc: Derrick Stolee, Taylor Blau, Justin Tobler

One of the tests in t5510 wants to verify that auto-gc does not lock up
when fetching into a repository. Adapt it to explicitly pick the "gc"
strategy for auto-maintenance.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t5510-fetch.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index c69afb5a60..5dcb4b51a4 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -1321,6 +1321,7 @@ test_expect_success 'fetching with auto-gc does not lock up' '
 		git config fetch.unpackLimit 1 &&
 		git config gc.autoPackLimit 1 &&
 		git config gc.autoDetach false &&
+		git config maintenance.strategy gc &&
 		GIT_ASK_YESNO="$TRASH_DIRECTORY/askyesno" git fetch --verbose >fetch.out 2>&1 &&
 		test_grep "Auto packing the repository" fetch.out &&
 		! grep "Should I try again" fetch.out

-- 
2.53.0.536.g309c995771.dirty


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v2 6/8] t6500: explicitly use "gc" strategy
  2026-02-24  8:45 ` [PATCH v2 " Patrick Steinhardt
                     ` (4 preceding siblings ...)
  2026-02-24  8:45   ` [PATCH v2 5/8] t5510: " Patrick Steinhardt
@ 2026-02-24  8:45   ` Patrick Steinhardt
  2026-02-25 10:13     ` Toon Claes
  2026-02-24  8:45   ` [PATCH v2 7/8] t7900: prepare for switch of the default strategy Patrick Steinhardt
                     ` (2 subsequent siblings)
  8 siblings, 1 reply; 38+ messages in thread
From: Patrick Steinhardt @ 2026-02-24  8:45 UTC (permalink / raw)
  To: git; +Cc: Derrick Stolee, Taylor Blau, Justin Tobler

The test in t6500 explicitly wants to exercise git-gc(1) and is thus
highly specific to the actual on-disk state of the repository and
specifically of the object database. An upcoming change modifies the
default maintenance strategy to be the "geometric" strategy though,
which breaks a couple of assumptions.

One fix would arguably be to disable auto-maintenance altogether, as we
do want to explicitly verify git-gc(1) anyway. But as the whole test
suite is about git-gc(1) in the first place it feels more sensible to
configure the default maintenance strategy to be "gc".

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t6500-gc.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh
index bef472cb8d..ea9aaad470 100755
--- a/t/t6500-gc.sh
+++ b/t/t6500-gc.sh
@@ -11,6 +11,7 @@ test_expect_success 'setup' '
 	# behavior, make sure we always pack everything to one pack by
 	# default
 	git config gc.bigPackThreshold 2g &&
+	git config set --global maintenance.strategy gc &&
 	test_oid_init
 '
 

-- 
2.53.0.536.g309c995771.dirty


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v2 7/8] t7900: prepare for switch of the default strategy
  2026-02-24  8:45 ` [PATCH v2 " Patrick Steinhardt
                     ` (5 preceding siblings ...)
  2026-02-24  8:45   ` [PATCH v2 6/8] t6500: " Patrick Steinhardt
@ 2026-02-24  8:45   ` Patrick Steinhardt
  2026-02-24  8:45   ` [PATCH v2 8/8] builtin/maintenance: use "geometric" strategy by default Patrick Steinhardt
  2026-02-24 18:54   ` [PATCH v2 0/8] " Justin Tobler
  8 siblings, 0 replies; 38+ messages in thread
From: Patrick Steinhardt @ 2026-02-24  8:45 UTC (permalink / raw)
  To: git; +Cc: Derrick Stolee, Taylor Blau, Justin Tobler

The t7900 test suite is exercising git-maintenance(1) and is thus of
course heavily reliant on the exact maintenance strategy. This reliance
comes in two flavors:

  - One test explicitly wants to verify that git-gc(1) is run as part of
    `git maintenance run`. This test is adapted by explicitly picking the
    "gc" strategy.

  - The other tests assume a specific shape of the object database,
    which is dependent on whether or not we run auto-maintenance before
    we come to the actual subject under test. These tests are adapted by
    disabling auto-maintenance.

With these changes t7900 passes with both "gc" and "geometric" default
strategies.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t7900-maintenance.sh | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index fe344f47ee..4700beacc1 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -45,7 +45,8 @@ test_expect_success 'help text' '
 	test_grep "usage: git maintenance" err
 '
 
-test_expect_success 'run [--auto|--quiet]' '
+test_expect_success 'run [--auto|--quiet] with gc strategy' '
+	test_config maintenance.strategy gc &&
 	GIT_TRACE2_EVENT="$(pwd)/run-no-auto.txt" \
 		git maintenance run 2>/dev/null &&
 	GIT_TRACE2_EVENT="$(pwd)/run-auto.txt" \
@@ -499,6 +500,7 @@ test_expect_success 'maintenance.incremental-repack.auto' '
 	(
 		cd incremental-repack-true &&
 		git config core.multiPackIndex true &&
+		git config maintenance.auto false &&
 		run_incremental_repack_and_verify
 	)
 '
@@ -509,6 +511,7 @@ test_expect_success 'maintenance.incremental-repack.auto (when config is unset)'
 	(
 		cd incremental-repack-unset &&
 		test_unconfig core.multiPackIndex &&
+		git config maintenance.auto false &&
 		run_incremental_repack_and_verify
 	)
 '
@@ -619,6 +622,7 @@ test_expect_success 'geometric repacking with --auto' '
 	git init repo &&
 	(
 		cd repo &&
+		git config set maintenance.auto false &&
 
 		# An empty repository does not need repacking, except when
 		# explicitly told to do it.

-- 
2.53.0.536.g309c995771.dirty


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v2 8/8] builtin/maintenance: use "geometric" strategy by default
  2026-02-24  8:45 ` [PATCH v2 " Patrick Steinhardt
                     ` (6 preceding siblings ...)
  2026-02-24  8:45   ` [PATCH v2 7/8] t7900: prepare for switch of the default strategy Patrick Steinhardt
@ 2026-02-24  8:45   ` Patrick Steinhardt
  2026-02-24 12:12     ` Derrick Stolee
  2026-02-24 18:54   ` [PATCH v2 0/8] " Justin Tobler
  8 siblings, 1 reply; 38+ messages in thread
From: Patrick Steinhardt @ 2026-02-24  8:45 UTC (permalink / raw)
  To: git; +Cc: Derrick Stolee, Taylor Blau, Justin Tobler

The git-gc(1) command has been introduced in the early days of Git in
30f610b7b0 (Create 'git gc' to perform common maintenance operations.,
2006-12-27) as the main repository maintenance utility. And while the
tool has of course evolved since then to cover new parts, the basic
strategy it uses has never really changed much.

It is safe to say that since 2006 the Git ecosystem has changed quite a
bit. Repositories tend to be much larger nowadays than they have been
almost 20 years ago, and large parts of the industry went crazy for
monorepos (for various wildly different definitions of "monorepo"). So
the maintenance strategy we used back then may not be the best fit
nowadays anymore.

Arguably, most of the maintenance tasks that git-gc(1) does are still
perfectly fine today: repacking references, expiring various data
structures and things like tend to not cause huge problems. But the big
exception is the way we repack objects.

git-gc(1) by default uses a split strategy: it performs incremental
repacks by default, and then whenever we have too many packs we perform
a large all-into-one repack. This all-into-one repack is what is causing
problems nowadays, as it is an operation that is quite expensive. While
it is wasteful in small- and medium-sized repositories, in large repos
it may even be prohibitively expensive.

We have eventually introduced git-maintenance(1) that was slated as a
replacement for git-gc(1). In contrast to git-gc(1), it is much more
flexible as it is structured around configurable tasks and strategies.
So while its default "gc" strategy still uses git-gc(1) under the hood,
it allows us to iterate.

A second strategy it knows about is the "incremental" strategy, which we
configure when registering a repository for scheduled maintenance. This
strategy isn't really a full replacement for git-gc(1) though, as it
doesn't know to expire unused data structures. In Git 2.52 we have thus
introduced a new "geometric" strategy that is a proper replacement for
the old git-gc(1).

In contrast to the incremental/all-into-one split used by git-gc(1), the
new "geometric" strategy maintains a geometric progression of packfiles,
which significantly reduces the number of all-into-one repacks that we
have to perform in large repositories. It is thus a much better fit for
large repositories than git-gc(1).

Note that the "geometric" strategy isn't perfect though: while we
perform way less all-into-one repacks compared to git-gc(1), we still
have to perform them eventually. But for the largest repositories out
there this may not be an option either, as client machines might not be
powerful enough to perform such a repack in the first place. These cases
would thus still be covered by the "incremental" strategy.

Switch the default strategy away from "gc" to "geometric", but retain
the "incremental" strategy configured when registering background
maintenance with `git maintenance register`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 Documentation/config/maintenance.adoc | 6 +++---
 builtin/gc.c                          | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/config/maintenance.adoc b/Documentation/config/maintenance.adoc
index d0c38f03fa..b578856dde 100644
--- a/Documentation/config/maintenance.adoc
+++ b/Documentation/config/maintenance.adoc
@@ -30,8 +30,7 @@ The possible strategies are:
 +
 * `none`: This strategy implies no tasks are run at all. This is the default
   strategy for scheduled maintenance.
-* `gc`: This strategy runs the `gc` task. This is the default strategy for
-  manual maintenance.
+* `gc`: This strategy runs the `gc` task.
 * `geometric`: This strategy performs geometric repacking of packfiles and
   keeps auxiliary data structures up-to-date. The strategy expires data in the
   reflog and removes worktrees that cannot be located anymore. When the
@@ -40,7 +39,8 @@ The possible strategies are:
   are already part of a cruft pack will be expired.
 +
 This repacking strategy is a full replacement for the `gc` strategy and is
-recommended for large repositories.
+recommended for large repositories. This is the default strategy for manual
+maintenance.
 * `incremental`: This setting optimizes for performing small maintenance
   activities that do not delete any data. This does not schedule the `gc`
   task, but runs the `prefetch` and `commit-graph` tasks hourly, the
diff --git a/builtin/gc.c b/builtin/gc.c
index 4390eee6ec..fb329c2cff 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -1980,7 +1980,7 @@ static void initialize_task_config(struct maintenance_run_opts *opts,
 		strategy = none_strategy;
 		type = MAINTENANCE_TYPE_SCHEDULED;
 	} else {
-		strategy = gc_strategy;
+		strategy = geometric_strategy;
 		type = MAINTENANCE_TYPE_MANUAL;
 	}
 

-- 
2.53.0.536.g309c995771.dirty


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* Re: [PATCH v2 8/8] builtin/maintenance: use "geometric" strategy by default
  2026-02-24  8:45   ` [PATCH v2 8/8] builtin/maintenance: use "geometric" strategy by default Patrick Steinhardt
@ 2026-02-24 12:12     ` Derrick Stolee
  2026-02-25 10:33       ` Toon Claes
  0 siblings, 1 reply; 38+ messages in thread
From: Derrick Stolee @ 2026-02-24 12:12 UTC (permalink / raw)
  To: Patrick Steinhardt, git; +Cc: Taylor Blau, Justin Tobler

On 2/24/26 3:45 AM, Patrick Steinhardt wrote:

> @@ -30,8 +30,7 @@ The possible strategies are:
>   +
>   * `none`: This strategy implies no tasks are run at all. This is the default
>     strategy for scheduled maintenance.
> -* `gc`: This strategy runs the `gc` task. This is the default strategy for
> -  manual maintenance.
> +* `gc`: This strategy runs the `gc` task.
>   * `geometric`: This strategy performs geometric repacking of packfiles and
>     keeps auxiliary data structures up-to-date. The strategy expires data in the
>     reflog and removes worktrees that cannot be located anymore. When the
> @@ -40,7 +39,8 @@ The possible strategies are:
>     are already part of a cruft pack will be expired.
>   +
>   This repacking strategy is a full replacement for the `gc` strategy and is
> -recommended for large repositories.
> +recommended for large repositories. This is the default strategy for manual
> +maintenance.

Thanks for these updates.

With this, v2 looks good to me.

-Stolee

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v2 0/8] builtin/maintenance: use "geometric" strategy by default
  2026-02-24  8:45 ` [PATCH v2 " Patrick Steinhardt
                     ` (7 preceding siblings ...)
  2026-02-24  8:45   ` [PATCH v2 8/8] builtin/maintenance: use "geometric" strategy by default Patrick Steinhardt
@ 2026-02-24 18:54   ` Justin Tobler
  8 siblings, 0 replies; 38+ messages in thread
From: Justin Tobler @ 2026-02-24 18:54 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git, Derrick Stolee, Taylor Blau

On 26/02/24 09:45AM, Patrick Steinhardt wrote:
> Changes in v2:
>   - Document the updated default strategy.
>   - Clarify how this interacts with Scalar.
>   - Explain the current landscape of strategies a bit better.
>   - Leave some breadcrumbs in the tests.
>   - Link to v1: https://lore.kernel.org/r/20260220-b4-pks-maintenance-default-geometric-strategy-v1-0-faeb321ad13b@pks.im

Thanks Patrick. This version addresses all my previous comments and
looks good to me.

-Justin

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v2 6/8] t6500: explicitly use "gc" strategy
  2026-02-24  8:45   ` [PATCH v2 6/8] t6500: " Patrick Steinhardt
@ 2026-02-25 10:13     ` Toon Claes
  0 siblings, 0 replies; 38+ messages in thread
From: Toon Claes @ 2026-02-25 10:13 UTC (permalink / raw)
  To: Patrick Steinhardt, git; +Cc: Derrick Stolee, Taylor Blau, Justin Tobler

Patrick Steinhardt <ps@pks.im> writes:

> The test in t6500 explicitly wants to exercise git-gc(1) and is thus
> highly specific to the actual on-disk state of the repository and
> specifically of the object database. An upcoming change modifies the
> default maintenance strategy to be the "geometric" strategy though,
> which breaks a couple of assumptions.
>
> One fix would arguably be to disable auto-maintenance altogether, as we
> do want to explicitly verify git-gc(1) anyway. But as the whole test
> suite is about git-gc(1) in the first place it feels more sensible to
> configure the default maintenance strategy to be "gc".
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  t/t6500-gc.sh | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh
> index bef472cb8d..ea9aaad470 100755
> --- a/t/t6500-gc.sh
> +++ b/t/t6500-gc.sh
> @@ -11,6 +11,7 @@ test_expect_success 'setup' '
>  	# behavior, make sure we always pack everything to one pack by
>  	# default
>  	git config gc.bigPackThreshold 2g &&
> +	git config set --global maintenance.strategy gc &&

I wasn't sure (no more) what effect setting globally would have. But
because each test file operates in it's own $TRASH_DIRECTORY, a global
setting only affects that file.

Makes sense.

-- 
Cheers,
Toon

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v2 8/8] builtin/maintenance: use "geometric" strategy by default
  2026-02-24 12:12     ` Derrick Stolee
@ 2026-02-25 10:33       ` Toon Claes
  0 siblings, 0 replies; 38+ messages in thread
From: Toon Claes @ 2026-02-25 10:33 UTC (permalink / raw)
  To: Derrick Stolee, Patrick Steinhardt, git; +Cc: Taylor Blau, Justin Tobler

Derrick Stolee <stolee@gmail.com> writes:

> With this, v2 looks good to me.
>
> -Stolee
>

Also did a review on v2, and I've got no comments about it. Approving.

-- 
Cheers,
Toon


^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 1/8] t: fix races caused by background maintenance
  2026-02-20 10:15 ` [PATCH 1/8] t: fix races caused by background maintenance Patrick Steinhardt
  2026-02-23 16:01   ` Justin Tobler
@ 2026-08-10  4:43   ` Stefan Haller
  2026-08-10  5:20     ` Patrick Steinhardt
  1 sibling, 1 reply; 38+ messages in thread
From: Stefan Haller @ 2026-08-10  4:43 UTC (permalink / raw)
  To: Patrick Steinhardt, git; +Cc: Derrick Stolee, Taylor Blau

On 20.02.26 11:15, Patrick Steinhardt wrote:
> Introduce a new `GIT_TEST_MAINT_AUTO_DETACH` environment variable that
> allows us to override the auto-detach behaviour and set that varibale in
> our tests.
I have just run into this problem with the lazygit test suite, and I
worked around it there by turning off auto maintenance altogether. Some
more details of how this affected us and why can be found in [1].

I'm fine with that solution, but I do wonder why we think this doesn't
also affect ordinary usage. Lazygit's integration test suite doesn't do
anything special, it simply executes git commands like a normal user
would. Maybe a bit faster than a normal user would type them, but for
scripts that create a bunch of files, stage them, and commit them, I see
no reason why they shouldn't run into the same problem. Or am I missing
something?

Thanks,
Stefan


[1] <https://github.com/jesseduffield/lazygit/pull/5898/
      changes/4ec91a0bf58e07ce040f08600cd0c6b64f996e07>

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 1/8] t: fix races caused by background maintenance
  2026-08-10  4:43   ` Stefan Haller
@ 2026-08-10  5:20     ` Patrick Steinhardt
  2026-08-10  7:37       ` Stefan Haller
  0 siblings, 1 reply; 38+ messages in thread
From: Patrick Steinhardt @ 2026-08-10  5:20 UTC (permalink / raw)
  To: Stefan Haller; +Cc: git, Derrick Stolee, Taylor Blau

On Mon, Aug 10, 2026 at 06:43:45AM +0200, Stefan Haller wrote:
> On 20.02.26 11:15, Patrick Steinhardt wrote:
> > Introduce a new `GIT_TEST_MAINT_AUTO_DETACH` environment variable that
> > allows us to override the auto-detach behaviour and set that varibale in
> > our tests.
> I have just run into this problem with the lazygit test suite, and I
> worked around it there by turning off auto maintenance altogether. Some
> more details of how this affected us and why can be found in [1].
> 
> I'm fine with that solution, but I do wonder why we think this doesn't
> also affect ordinary usage. Lazygit's integration test suite doesn't do
> anything special, it simply executes git commands like a normal user
> would. Maybe a bit faster than a normal user would type them, but for
> scripts that create a bunch of files, stage them, and commit them, I see
> no reason why they shouldn't run into the same problem. Or am I missing
> something?

It does affect ordinary usage, but the expectation is that a "normal"
user should not care about repository maintenance running in parallel to
us. It should be completely transparent to them in the best case. Git
commands should just work with concurrent maintenance, and if they don't
then it's worth to have a deeper look at why it doesn't.

The reason why it's not fine for the Git test suite is that in lots of
cases we assume a lot about the on-disk state of the repository. We are
often reaching into internals to verify that it looks as expected, and
that is of course racing with concurrent maintenance. And hence we have
to be more careful than users, as they are not supposed to reach into
repository internals without Git or an implementation thereof.

Patrick

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 1/8] t: fix races caused by background maintenance
  2026-08-10  5:20     ` Patrick Steinhardt
@ 2026-08-10  7:37       ` Stefan Haller
  2026-08-10  8:35         ` Patrick Steinhardt
  0 siblings, 1 reply; 38+ messages in thread
From: Stefan Haller @ 2026-08-10  7:37 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git, Derrick Stolee, Taylor Blau

On 10.08.26 07:20, Patrick Steinhardt wrote:

> Git commands should just work with concurrent maintenance, and if they
> don't then it's worth to have a deeper look at why it doesn't.

That was my point; in lazygit's test suite I was getting errors when
executing simple commands such as "create a bunch of files, git add,
git commit". I had cases where the commit fails with

  error: invalid object 100644 50d5612... for 'file09.txt'
  error: Error building trees

> The reason why it's not fine for the Git test suite is that in lots of
> cases we assume a lot about the on-disk state of the repository. We are
> often reaching into internals to verify that it looks as expected, and
> that is of course racing with concurrent maintenance.

Yes, I understand why git's test suite has reasons to disable concurrent
maintenance. My point was that lazygit doesn't have any such reasons,
and shouldn't have to disable maintenance just so that the commands it
invokes don't error.

Stefan

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 1/8] t: fix races caused by background maintenance
  2026-08-10  7:37       ` Stefan Haller
@ 2026-08-10  8:35         ` Patrick Steinhardt
  2026-08-10 10:45           ` Stefan Haller
  0 siblings, 1 reply; 38+ messages in thread
From: Patrick Steinhardt @ 2026-08-10  8:35 UTC (permalink / raw)
  To: Stefan Haller; +Cc: git, Derrick Stolee, Taylor Blau

On Mon, Aug 10, 2026 at 09:37:01AM +0200, Stefan Haller wrote:
> On 10.08.26 07:20, Patrick Steinhardt wrote:
> 
> > Git commands should just work with concurrent maintenance, and if they
> > don't then it's worth to have a deeper look at why it doesn't.
> 
> That was my point; in lazygit's test suite I was getting errors when
> executing simple commands such as "create a bunch of files, git add,
> git commit". I had cases where the commit fails with
> 
>   error: invalid object 100644 50d5612... for 'file09.txt'
>   error: Error building trees

That's a bug then that we ought to fix. Do you maybe have a reproducer
for this?

Also, which version of Git are you testing this with? We recently had an
issue with multiple concurrent git-maintenance(1) processes running at
the same time, which is something that shouldn't ever happen. That was
fixed already, but IIRC Git 2.54 was still prone to this race.

Patrick

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 1/8] t: fix races caused by background maintenance
  2026-08-10  8:35         ` Patrick Steinhardt
@ 2026-08-10 10:45           ` Stefan Haller
  2026-08-10 13:55             ` Patrick Steinhardt
  0 siblings, 1 reply; 38+ messages in thread
From: Stefan Haller @ 2026-08-10 10:45 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git, Derrick Stolee, Taylor Blau

On 10.08.26 10:35, Patrick Steinhardt wrote:

> On Mon, Aug 10, 2026 at 09:37:01AM +0200, Stefan Haller wrote:
>
>> On 10.08.26 07:20, Patrick Steinhardt wrote:
>>
>> I had cases where the commit fails with
>>
>>   error: invalid object 100644 50d5612... for 'file09.txt'
>>   error: Error building trees
> 
> That's a bug then that we ought to fix. Do you maybe have a reproducer
> for this?

I had my friendly AI agent dig into this more, and below is what it came up
with. Disclaimer: I only skimmed it, and didn't check it for accuracy; I'm way
too unfamiliar with any of this stuff. I hope this is of some use.

----------

Subject: [BUG] geometric-repack --auto fires on tiny repos, and the
 resulting background repacks race concurrent commands

Since v2.54.0, `git commit` in an ordinary small repository can leave a
detached `git repack` running behind it, and it does so far more often
than intended: the auto condition that is documented as "at least 100
loose objects" is in practice satisfied by a repository containing four
objects.  The background repacks that result break concurrent git
commands at a rate of roughly one in a thousand commits in my testing.

There are two separate issues here.  The first is a plain bug; the
second is its fallout.

1. The geometric-repack auto condition triggers ~256x too eagerly
----------------------------------------------------------------

Documentation/config/maintenance.adoc says:

    maintenance.geometric-repack.auto::
            [...] a positive value implies the command should run either
            when there are packfiles that need to be merged together to
            retain the geometric progression, or when there are at least
            this many loose objects that would be written into a new
            packfile. The default value is 100.

Reproducer, on a repository with four objects (v2.54.0 and v2.55.0):

    git init repro && cd repro
    git config maintenance.auto false   # so that the setup itself does
                                        # not fork any maintenance
    echo 263 >a.txt                     # blob 175b6c5d...
    echo 410 >b.txt                     # blob 17e344e7...
    git add . && git commit -m initial

    $ find .git/objects -type f
    .git/objects/17/5b6c5dfd7f9bf6e2b2c4e2dcf3e2341298575d
    .git/objects/17/e344e7c08441fa81d5b56c21008dc0feeeaa20
    .git/objects/70/a9a72c46fc85c654e626f54e77c4946950da52
    .git/objects/7f/174478a536fc641c735e6b6e1a1944c868ba0c

    $ git maintenance run --auto --no-detach
    $ ls .git/objects/pack/
    multi-pack-index
    pack-f826a2dd....idx
    pack-f826a2dd....pack
    pack-f826a2dd....rev

(only the two blob IDs are fixed; the tree and commit depend on the
commit timestamp.)  Committing only a.txt -- three objects, one of them
in objects/17 -- does not trigger the repack.  Adding the second blob
whose object ID starts with "17" does.  That is the whole condition.

geometric_repack_auto_condition() (builtin/gc.c) passes its threshold to
too_many_loose_objects(), which does not count loose objects: it counts
the entries of .git/objects/17 and scales by 256.  In v2.54.0:

    int auto_threshold = DIV_ROUND_UP(limit, 256);
    [...]
            if (++num_loose > auto_threshold) {

and equivalently after the rewrite in v2.55.0:

    /*
     * This is weird, but stems from legacy behaviour: [...]
     */
    int auto_threshold = DIV_ROUND_UP(limit, 256) * 256;
    [...]
    return loose_count > auto_threshold;

with loose_count coming from ODB_COUNT_OBJECTS_APPROXIMATE, i.e. the
same one-directory estimate.  Either way, any limit <= 256 collapses to
"two or more objects share the objects/17 directory".

That estimator is fine for gc.auto, whose default of 6700 needs 27
entries in that directory -- a number you only reach with thousands of
objects, and whose documentation says "approximately".  It falls apart
for a threshold below 256, where the smallest representable estimate
step exceeds the threshold itself and a single fanout collision decides
the outcome.  For a repository with n objects the condition is satisfied
with probability ~1-(1-p)^n-np(1-p)^(n-1), p=1/256: about 5% at 90
objects, and much higher for repositories that accumulate objects over
time.

By contrast maintenance.loose-objects.auto, documented in the same terms,
is implemented as an exact count in loose_object_auto_condition().

Since 452b12c2e0 (builtin/maintenance: use "geometric" strategy by
default, 2026-02-24) this condition governs what `git maintenance run
--auto` does, and run_auto_maintenance() invokes that from commit, am,
fetch, merge and rebase, with --detach unless gc.autoDetach says
otherwise.  So the practical effect is: once any two objects in a
repository collide in objects/17 -- and the condition only becomes
easier to satisfy as the repository grows -- every subsequent commit
forks a detached repack.

Measured on a fixture repository built by 40 commits of "write a file,
git add, git commit": a background repack fired in 4 of 25 runs (the
prediction from the 40 nondeterministic commit object IDs alone is
1-(255/256)^40 = 14%).

2. The resulting background repacks break concurrent commands
-------------------------------------------------------------

`git repack -d` installs the new pack, removes the redundant ones and
then calls prune_packed_objects(), which unlinks the loose copies of
objects that are now packed and rmdir()s the fanout directories it
empties.  Doing that concurrently with unrelated git processes in the
same repository is new exposure: before v2.54.0 the same repositories
never reached the gc.auto threshold and no such repack ever ran.

Stress test: 24 concurrent workers, each repeatedly creating a fresh
repository and running 40 iterations of "write a file, git add, git
commit", with maintenance.geometric-repack.auto=-1 so that the condition
fires on every commit rather than in ~15% of repositories.  Nothing else
touches the repositories; the only concurrency is git's own detached
auto-maintenance.

109280 commits produced 97 failing commands, i.e. one per ~1100 commits.
Message lines across those 97 failures (one failure often emits two of
them, e.g. a temporary-file error followed by a fatal):

      71  error: packfile .../pack-....pack index unavailable
      50  fatal: failed to write commit object
      37  error: unable to create temporary file: Invalid argument
      23  error: unable to create temporary file: No such file or directory
      10  error: unable to index file '...'
      10  fatal: adding files failed

At least the ENOENT variant has a straightforward explanation:

  - The writer, in create_tmpfile() (object-file.c), calls
    mkstemp(".git/objects/XX/tmp_obj_XXXXXX").  On ENOENT it mkdir()s
    the fanout directory and retries -- exactly once.

  - The repack, in prune_subdir() (prune-packed.c), rmdir()s each fanout
    directory after pruning its contents.

An rmdir() landing between the writer's mkdir() and its retried
mkstemp() defeats the single retry, and the command aborts.  The writer
retries a bounded number of times against interference that is not
bounded.

(For anyone hitting this meanwhile: setting maintenance.auto=false makes
it go away, since it stops the fork at the source.  We have done that in
lazygit's test configuration.)

3. Where this came from
-----------------------

lazygit's integration tests started failing intermittently on CI, always
and only in the job running the newest git (2.54.0 on GitHub Actions
ubuntu-latest), never in the jobs pinned to 2.32.0/2.38.2/2.44.0, and
always during test fixture setup, which is a plain loop of "write a
file, git add, git commit":

    error: invalid object 100644 50d561270fcfcdc9afc85f6136f937c529accaaa for 'file09.txt'
    error: invalid object 100644 50d561270fcfcdc9afc85f6136f937c529accaaa for 'file09.txt'
    error: Error building trees

(the first message really is printed twice), i.e. the cache-tree
existence check in update_one() (cache-tree.c) failing to find a blob
that a previous iteration had successfully committed.  The hash is the
correct blob for the file's content, so `git add` had staged it fine.

Those fixture repositories are exactly the case in section 1: they
contain one object in objects/17 by construction, so a single commit
object hashing into the same directory tips them over, and every
following commit forks a repack.  The affected tests were, without
exception, the ones whose fixtures build six or more commits.

I could not reproduce this specific failure outside Linux, and I have
not isolated the window that produces it.  A fresh process is robust
(7200 lookups against a repack loop, no misses), and so is a `git
commit` whose repack completes mid-flight -- the flag the cache-tree
check passes, ODB_HAS_OBJECT_RECHECK_PACKED, makes a missing lookup
reprepare the pack list and retry, and that recovery does work in
everything I could construct deliberately.  So I am reporting this as
the symptom that led to the above rather than as a diagnosed bug, in
case it is familiar to someone.

One observation from reading that retry path, which may or may not be
related: packfile_store_reprepare() refreshes the pack list but never
reloads the multi-pack-index, because prepare_multi_pack_index_one()
(midx.c) returns early when one is already loaded.  A long-lived process
therefore keeps a midx that may reference packs a concurrent repack has
since deleted, for its entire lifetime.

Environment
-----------

Original failures: git 2.54.0, GitHub Actions ubuntu-latest.
Reproducers and measurements above: git 2.55.0 (built from source),
macOS 26, APFS.  Code references are to v2.54.0 unless stated; I checked
that the behaviour is unchanged in v2.55.0.

Thanks,
Stefan


^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 1/8] t: fix races caused by background maintenance
  2026-08-10 10:45           ` Stefan Haller
@ 2026-08-10 13:55             ` Patrick Steinhardt
  2026-08-10 15:12               ` Stefan Haller
  0 siblings, 1 reply; 38+ messages in thread
From: Patrick Steinhardt @ 2026-08-10 13:55 UTC (permalink / raw)
  To: Stefan Haller; +Cc: git, Derrick Stolee, Taylor Blau

On Mon, Aug 10, 2026 at 12:45:57PM +0200, Stefan Haller wrote:
> On 10.08.26 10:35, Patrick Steinhardt wrote:
> > On Mon, Aug 10, 2026 at 09:37:01AM +0200, Stefan Haller wrote:
[snip]
> geometric_repack_auto_condition() (builtin/gc.c) passes its threshold to
> too_many_loose_objects(), which does not count loose objects: it counts
> the entries of .git/objects/17 and scales by 256.  In v2.54.0:
> 
>     int auto_threshold = DIV_ROUND_UP(limit, 256);
>     [...]
>             if (++num_loose > auto_threshold) {
> 
> and equivalently after the rewrite in v2.55.0:
> 
>     /*
>      * This is weird, but stems from legacy behaviour: [...]
>      */
>     int auto_threshold = DIV_ROUND_UP(limit, 256) * 256;
>     [...]
>     return loose_count > auto_threshold;
> 
> with loose_count coming from ODB_COUNT_OBJECTS_APPROXIMATE, i.e. the
> same one-directory estimate.  Either way, any limit <= 256 collapses to
> "two or more objects share the objects/17 directory".

That's by design, and is also true for git-gc(1).

> That estimator is fine for gc.auto, whose default of 6700 needs 27
> entries in that directory -- a number you only reach with thousands of
> objects, and whose documentation says "approximately".  It falls apart
> for a threshold below 256, where the smallest representable estimate
> step exceeds the threshold itself and a single fanout collision decides
> the outcome.  For a repository with n objects the condition is satisfied
> with probability ~1-(1-p)^n-np(1-p)^(n-1), p=1/256: about 5% at 90
> objects, and much higher for repositories that accumulate objects over
> time.

But I tend to agree that the default value here is too low. That's an
easy-enough change to make:

diff --git a/Documentation/config/maintenance.adoc b/Documentation/config/maintenance.adoc
index b578856dde..da8be9f812 100644
--- a/Documentation/config/maintenance.adoc
+++ b/Documentation/config/maintenance.adoc
@@ -101,7 +101,7 @@ maintenance.geometric-repack.auto::
 	there are packfiles that need to be merged together to retain the
 	geometric progression, or when there are at least this many loose
 	objects that would be written into a new packfile. The default value is
-	100.
+	6700.
 
 maintenance.geometric-repack.splitFactor::
 	This integer config option controls the factor used for the geometric
diff --git a/odb/source-files.c b/odb/source-files.c
index 5a68af7d84..555e466145 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -521,7 +521,7 @@ bool odb_source_files_optimize_required(struct odb_source *source,
 		};
 		struct existing_packs existing_packs = EXISTING_PACKS_INIT;
 		struct string_list kept_packs = STRING_LIST_INIT_DUP;
-		int auto_value = 100;
+		int auto_value = 6700;
 		bool ret;
 
 		repo_config_get_int(repo, "maintenance.geometric-repack.auto",

> 2. The resulting background repacks break concurrent commands
> -------------------------------------------------------------
> 
> `git repack -d` installs the new pack, removes the redundant ones and
> then calls prune_packed_objects(), which unlinks the loose copies of
> objects that are now packed and rmdir()s the fanout directories it
> empties.  Doing that concurrently with unrelated git processes in the
> same repository is new exposure: before v2.54.0 the same repositories
> never reached the gc.auto threshold and no such repack ever ran.

Okay, so the issue is basically preexistent, but because we now repack a
lot more aggressively it's surfacing more often. Ideally, we'd fix that,
but it's also clear that repacking too often will make us race a lot
more, so we should avoid doing that too aggressively.

Does the issue go away if you set `maintenance.geometric-repack.auto=6700`?
If yes, I'd propose to simply change that default to be in line with
what git-gc(1) uses.

Thanks!

Patrick

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* Re: [PATCH 1/8] t: fix races caused by background maintenance
  2026-08-10 13:55             ` Patrick Steinhardt
@ 2026-08-10 15:12               ` Stefan Haller
  0 siblings, 0 replies; 38+ messages in thread
From: Stefan Haller @ 2026-08-10 15:12 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git, Derrick Stolee, Taylor Blau

On 10.08.26 15:55, Patrick Steinhardt wrote:

> Does the issue go away if you set `maintenance.geometric-repack.auto=6700`?
> If yes, I'd propose to simply change that default to be in line with
> what git-gc(1) uses.
I'm afraid it would take me too long to find out; the issue occurred
sporadically on CI, so I'm not sure how many CI runs I'd have to trigger
on a branch to be reasonably sure it doesn't reoccur with that setting.
(And I'd prefer not to make that change on main, I'm quite happy with my
current solution that's guaranteed stable.)

Stefan

^ permalink raw reply	[flat|nested] 38+ messages in thread

end of thread, other threads:[~2026-08-10 15:12 UTC | newest]

Thread overview: 38+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-20 10:15 [PATCH 0/8] builtin/maintenance: use "geometric" strategy by default Patrick Steinhardt
2026-02-20 10:15 ` [PATCH 1/8] t: fix races caused by background maintenance Patrick Steinhardt
2026-02-23 16:01   ` Justin Tobler
2026-08-10  4:43   ` Stefan Haller
2026-08-10  5:20     ` Patrick Steinhardt
2026-08-10  7:37       ` Stefan Haller
2026-08-10  8:35         ` Patrick Steinhardt
2026-08-10 10:45           ` Stefan Haller
2026-08-10 13:55             ` Patrick Steinhardt
2026-08-10 15:12               ` Stefan Haller
2026-02-20 10:15 ` [PATCH 2/8] t: disable maintenance where we verify object database structure Patrick Steinhardt
2026-02-23 16:07   ` Justin Tobler
2026-02-20 10:15 ` [PATCH 3/8] t34xx: don't expire reflogs where it matters Patrick Steinhardt
2026-02-23  0:48   ` Derrick Stolee
2026-02-23 16:15   ` Justin Tobler
2026-02-20 10:15 ` [PATCH 4/8] t5400: explicitly use "gc" strategy Patrick Steinhardt
2026-02-20 10:15 ` [PATCH 5/8] t5510: " Patrick Steinhardt
2026-02-20 10:15 ` [PATCH 6/8] t6500: " Patrick Steinhardt
2026-02-20 10:15 ` [PATCH 7/8] t7900: prepare for switch of the default strategy Patrick Steinhardt
2026-02-20 10:15 ` [PATCH 8/8] builtin/maintenance: use "geometric" strategy by default Patrick Steinhardt
2026-02-23  0:52   ` Derrick Stolee
2026-02-23  9:49     ` Patrick Steinhardt
2026-02-23 16:48   ` Justin Tobler
2026-02-24  8:15     ` Patrick Steinhardt
2026-02-23  0:53 ` [PATCH 0/8] " Derrick Stolee
2026-02-24  8:45 ` [PATCH v2 " Patrick Steinhardt
2026-02-24  8:45   ` [PATCH v2 1/8] t: fix races caused by background maintenance Patrick Steinhardt
2026-02-24  8:45   ` [PATCH v2 2/8] t: disable maintenance where we verify object database structure Patrick Steinhardt
2026-02-24  8:45   ` [PATCH v2 3/8] t34xx: don't expire reflogs where it matters Patrick Steinhardt
2026-02-24  8:45   ` [PATCH v2 4/8] t5400: explicitly use "gc" strategy Patrick Steinhardt
2026-02-24  8:45   ` [PATCH v2 5/8] t5510: " Patrick Steinhardt
2026-02-24  8:45   ` [PATCH v2 6/8] t6500: " Patrick Steinhardt
2026-02-25 10:13     ` Toon Claes
2026-02-24  8:45   ` [PATCH v2 7/8] t7900: prepare for switch of the default strategy Patrick Steinhardt
2026-02-24  8:45   ` [PATCH v2 8/8] builtin/maintenance: use "geometric" strategy by default Patrick Steinhardt
2026-02-24 12:12     ` Derrick Stolee
2026-02-25 10:33       ` Toon Claes
2026-02-24 18:54   ` [PATCH v2 0/8] " Justin Tobler

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.