Thread (62 messages) 62 messages, 4 authors, 2025-10-27

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help