Re: [PATCH v3 03/10] builtin/maintenance: introduce "geometric-repack" task
From: Jeff King <hidden>
Date: 2025-10-25 19:15:57
Subsystem:
the rest · Maintainer:
Linus Torvalds
On Fri, Oct 24, 2025 at 08:57:16AM +0200, Patrick Steinhardt wrote:
+ # Repacking should now cause a no-op geometric repack because + # no packfiles need to be combined. + ls -l .git/objects/pack >before && + run_and_verify_geometric_pack 1 && + ls -l .git/objects/pack >after && + test_cmp before after &&
I got a CI failure from this test like this:
+ diff -u before after
--- before 2025-10-25 17:51:59.985025237 +0000
+++ after 2025-10-25 17:52:00.304026445 +0000
@@ -1,5 +1,5 @@
total 16
--rw-rw-r-- 1 builder builder 1252 Oct 25 17:51 multi-pack-index
+-rw-rw-r-- 1 builder builder 1252 Oct 25 17:52 multi-pack-index
-r--r--r-- 1 builder builder 1156 Oct 25 17:51 pack-68c20c4590a622a21395b4480621d55494112a83.idx
-r--r--r-- 1 builder builder 226 Oct 25 17:51 pack-68c20c4590a622a21395b4480621d55494112a83.pack
-r--r--r-- 1 builder builder 64 Oct 25 17:51 pack-68c20c4590a622a21395b4480621d55494112a83.rev
I'm not sure if this is a bug or a race condition in the test. If
"no-op" means "do not generate a new pack, but do generate a new midx"
then it's a race condition (the regenerated midx might move across the
minute boundary). If it means "do not even generate a new midx", then
there is a bug. ;)
You can generate the race at will like this:
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 0d76693fee..2b5141196f 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh@@ -501,6 +501,7 @@ test_expect_success 'geometric repacking task' ' # Repacking should now cause a no-op geometric repack because # no packfiles need to be combined. ls -l .git/objects/pack >before && + sleep 60 && run_and_verify_geometric_pack 1 && ls -l .git/objects/pack >after && test_cmp before after &&
though if we are going to be picky about timestamps, it probably makes sense to use a higher resolution. Sadly I don't think there's a portable way to do that with "ls", and "stat" is probably likewise something we can't assume. I'd turn to perl, but I know you've been trying to avoid depending on it. You can hack around it with: test-tool chmtime -v +0 .git/objects/pack/* for this case, I'd think. -Peff