[PATCH v2 8/8] repack: disable writing bitmaps when doing a local geometric repack

From: Patrick Steinhardt <ps@pks.im>
Date: Apr 12, 2023, 10:23 UTC
Message-ID: <b74d0a037b07706d07fad7b438fa0cb211726575.1681294715.git.ps@pks.im>
In-Reply-To: <cover.1681294715.git.ps@pks.im>

In order to write a bitmap, we need to have full coverage of all objects that are about to be packed. In the traditional non-multi-pack-index world this meant we need to do a full repack of all objects into a single packfile. But in the new multi-pack-index world we can get away with writing bitmaps when we have multiple packfiles as long as the multi-pack-index covers all objects.

This is not always the case though. When writing multi-pack-indices in a repository that is connected to an alternate object directory we may end up writing a multi-pack-index that only has partial coverage of objects. The end result is that writing the bitmap will fail:

    $ git multi-pack-index write --stdin-packs --bitmap <packfiles
    warning: Failed to write bitmap index. Packfile doesn't have full closure (object 1529341d78cf45377407369acb0f4ff2b5cdae42 is missing)
    error: could not write multi-pack bitmap

Now there are two different ways to fix this. The first one would be to amend git-multi-pack-index(1) to disable writing bitmaps when we notice that we don't have full object coverage. But we ain't really got enough information there, and seeing that it is a low-level plumbing command it does not feel like the right place to fix this.

We can easily fix it at a higher level in git-repack(1) though. When all of the following conditions are true:

    - We are asked to write a multi-pack index with bitmaps.

    - We are asked to only include local objects via `-l`.

    - We are connected to an alternate repository that has packfiles.

Then we will override the user's ask and disable writing bitmaps with a warning. This is similar to what we do in git-pack-objects(1), where we also disable writing bitmaps in case we omit an object from the pack.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/repack.c            | 20 ++++++++++++++++++++
 t/t7703-repack-geometric.sh | 27 +++++++++++++++++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/builtin/repack.c b/builtin/repack.c
index f57869f14a..07d92fdf87 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -881,6 +881,26 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
 	if (pack_kept_objects < 0)
 		pack_kept_objects = write_bitmaps > 0 && !write_midx;
 
+	if (write_midx && write_bitmaps && geometric_factor && po_args.local) {
+		struct packed_git *p;
+
+		for (p = get_all_packs(the_repository); p; p = p->next) {
+			if (p->pack_local)
+				continue;
+
+			/*
+			 * When asked to do a local repack, but we have
+			 * packfiles that are inherited from an alternate, then
+			 * we cannot guarantee that the multi-pack-index would
+			 * have full coverage of all objects. We thus disable
+			 * writing bitmaps in that case.
+			 */
+			warning(_("disabling bitmap writing, as some objects are not being packed"));
+			write_bitmaps = 0;
+			break;
+		}
+	}
+
 	if (write_bitmaps && !(pack_everything & ALL_INTO_ONE) && !write_midx)
 		die(_(incremental_bitmap_conflict_error));
 
diff --git a/t/t7703-repack-geometric.sh b/t/t7703-repack-geometric.sh
index 96c8d4cdfa..0aaec9f853 100755
--- a/t/t7703-repack-geometric.sh
+++ b/t/t7703-repack-geometric.sh
@@ -420,4 +420,31 @@ test_expect_success '--geometric -l with non-intact geometric sequence across OD
 	test_cmp expected-objects actual-objects.sorted
 '
 
+test_expect_success '--geometric -l disables writing bitmaps with non-local packfiles' '
+	test_when_finished "rm -fr shared member" &&
+
+	git init shared &&
+	test_commit_bulk -C shared --start=1 1 &&
+
+	git clone --shared shared member &&
+	test_commit_bulk -C member --start=2 1 &&
+
+	# When performing a geometric repack with `-l` while connecting 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.
+	git -C member repack -l --geometric=2 --write-midx --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 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_done

-- 
2.40.0