[PATCH v4 00/10] repack: fix geometric repacking with gitalternates

From: Patrick Steinhardt <ps@pks.im>
Date: Apr 14, 2023, 06:01 UTC
Message-ID: <cover.1681452028.git.ps@pks.im>
In-Reply-To: <a07ed50feeec4bfc3e9736bf493b9876896bcdd2.1680606445.git.ps@pks.im>

Hi,

this is the fourth version of my patch series to fix geometric repacking with repositories connected to an alternate object database.

This version only addresses some issues with the tests, the actual logic remains untouched:

    - The test added in t7700-repack.sh that verifies that `--local`
      causes us to disable generation of the bitmap index was failing in
      the linux-TEST-vars CI job. This was because it sets
      GIT_TEST_MULTI_PACK_INDEX=1, which causes us to disable the bitmap
      logic in git-repack(1). I've fixed this failure by explicitly
      overriding the environment variable like other tests in the same
      file do.

    - I've converted path checks to use `test_path_is_missing` and
      `test_path_is_file` instead of `test ! -f` and `test -f`.

    - I've fixed a typo in t7703-repack-geometric.sh and shifted code
      around a bit to make the test more readable, following Derrick's
      suggestion.

Thanks!

Patrick

Patrick Steinhardt (10):
  midx: fix segfault with no packs and invalid preferred pack
  repack: fix trying to use preferred pack in alternates
  repack: fix generating multi-pack-index with only non-local packs
  pack-objects: split out `--stdin-packs` tests into separate file
  pack-objects: fix error when packing same pack twice
  pack-objects: fix error when same packfile is included and excluded
  pack-objects: extend test coverage of `--stdin-packs` with alternates
  t/helper: allow chmtime to print verbosely without modifying mtime
  repack: honor `-l` when calculating pack geometry
  repack: disable writing bitmaps when doing a local repack

 builtin/pack-objects.c        |  10 +-
 builtin/repack.c              |  62 ++++++++-
 midx.c                        |   6 +-
 object-file.c                 |   6 +
 object-store.h                |   1 +
 t/helper/test-chmtime.c       |   2 +-
 t/t5300-pack-object.sh        | 135 -------------------
 t/t5319-multi-pack-index.sh   |  12 ++
 t/t5331-pack-objects-stdin.sh | 240 ++++++++++++++++++++++++++++++++++
 t/t7700-repack.sh             |  17 +++
 t/t7703-repack-geometric.sh   | 164 +++++++++++++++++++++++
 11 files changed, 504 insertions(+), 151 deletions(-)
 create mode 100755 t/t5331-pack-objects-stdin.sh

Diff against v3:

diff --git a/t/t5331-pack-objects-stdin.sh b/t/t5331-pack-objects-stdin.sh
index 45e24fa94a..acab31667a 100755
--- a/t/t5331-pack-objects-stdin.sh
+++ b/t/t5331-pack-objects-stdin.sh
@@ -7,7 +7,7 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
-packed_objects() {
+packed_objects () {
 	git show-index <"$1" >tmp-object-list &&
 	cut -d' ' -f2 tmp-object-list | sort &&
 	rm tmp-object-list
diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh
index 93c5763e7d..faa739eeb9 100755
--- a/t/t7700-repack.sh
+++ b/t/t7700-repack.sh
@@ -114,12 +114,12 @@ test_expect_success '--local disables writing bitmaps when connected to alternat
 	(
 		cd member &&
 		test_commit "object" &&
-		git repack -Adl --write-bitmap-index 2>err &&
+		GIT_TEST_MULTI_PACK_INDEX=0 git repack -Adl --write-bitmap-index 2>err &&
 		cat >expect <<-EOF &&
 		warning: disabling bitmap writing, as some objects are not being packed
 		EOF
 		test_cmp expect err &&
-		test ! -f .git/objects/pack-*.bitmap
+		test_path_is_missing .git/objects/pack-*.bitmap
 	)
 '
 
diff --git a/t/t7703-repack-geometric.sh b/t/t7703-repack-geometric.sh
index 33d7977fca..00f28fb558 100755
--- a/t/t7703-repack-geometric.sh
+++ b/t/t7703-repack-geometric.sh
@@ -10,9 +10,9 @@ objdir=.git/objects
 packdir=$objdir/pack
 midx=$objdir/pack/multi-pack-index
 
-packed_objects() {
+packed_objects () {
 	git show-index <"$1" >tmp-object-list &&
-	cut -d' ' -f2 tmp-object-list &&
+	cut -d' ' -f2 tmp-object-list | sort &&
 	rm tmp-object-list
  }
 
@@ -312,7 +312,7 @@ test_expect_success '--geometric --write-midx with packfiles in main and alterna
 
 	# We should also see a multi-pack-index. This multi-pack-index should
 	# never refer to any packfiles in the alternate object database.
-	test -f member/.git/objects/pack/multi-pack-index &&
+	test_path_is_file member/.git/objects/pack/multi-pack-index &&
 	test-tool read-midx member/.git/objects >packs.midx &&
 	grep "^pack-.*\.idx$" packs.midx | sort >actual &&
 	basename member/.git/objects/pack/pack-*.idx >expect &&
@@ -378,11 +378,11 @@ test_expect_success '--geometric -l with non-intact geometric sequence across OD
 
 	# Verify that our assumptions actually hold: both generated packfiles
 	# should have three objects and should be non-equal.
-	packed_objects shared/.git/objects/pack/pack-*.idx >packed-objects &&
-	test_line_count = 3 packed-objects &&
-	packed_objects member/.git/objects/pack/pack-*.idx >packed-objetcs &&
-	test_line_count = 3 packed-objects &&
-	test "$(basename member/.git/objects/pack/pack-*.pack)" != "$(basename shared/.git/objects/pack/pack-*.pack)" &&
+	packed_objects shared/.git/objects/pack/pack-*.idx >shared-objects &&
+	packed_objects member/.git/objects/pack/pack-*.idx >member-objects &&
+	test_line_count = 3 shared-objects &&
+	test_line_count = 3 member-objects &&
+	! test_cmp shared-objects member-objects &&
 
 	# Perform the geometric repack. With `-l`, we should only see the local
 	# packfile and thus arrive at the conclusion that the geometric
@@ -415,8 +415,7 @@ test_expect_success '--geometric -l with non-intact geometric sequence across OD
 	test_line_count = 1 actual &&
 	packed_objects member/.git/objects/pack/pack-*.idx >actual-objects &&
 	test_line_count = 6 actual-objects &&
-	sort <actual-objects >actual-objects.sorted &&
-	test_cmp expected-objects actual-objects.sorted
+	test_cmp expected-objects actual-objects
 '
 
 test_expect_success '--geometric -l disables writing bitmaps with non-local packfiles' '
@@ -428,7 +427,7 @@ test_expect_success '--geometric -l disables writing bitmaps with non-local pack
 	git clone --shared shared member &&
 	test_commit_bulk -C member --start=2 1 &&
 
-	# When performing a geometric repack with `-l` while connecting to an
+	# When performing a geometric repack with `-l` while connected to an
 	# alternate object database that has a packfile we do not have full
 	# coverage of objects. As a result, we expect that writing the bitmap
 	# will be disabled.
@@ -437,13 +436,13 @@ test_expect_success '--geometric -l disables writing bitmaps with non-local pack
 	warning: disabling bitmap writing, as some objects are not being packed
 	EOF
 	test_cmp expect err &&
-	test ! -f member/.git/objects/pack/multi-pack-index-*.bitmap &&
+	test_path_is_missing member/.git/objects/pack/multi-pack-index-*.bitmap &&
 
 	# On the other hand, when we repack without `-l`, we should see that
 	# the bitmap gets created.
 	git -C member repack --geometric=2 --write-midx --write-bitmap-index 2>err &&
 	test_must_be_empty err &&
-	test -f member/.git/objects/pack/multi-pack-index-*.bitmap
+	test_path_is_file member/.git/objects/pack/multi-pack-index-*.bitmap
 '
 
 test_done

-- 
2.40.0