[PATCH v2 1/2] repack: respect kept objects with '--write-midx -b'

From: Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>
Date: Dec 20, 2021, 14:48 UTC
Message-ID: <747328a4dd69f2325892b4d89e3e62a0bcfbba59.1640011691.git.gitgitgadget@gmail.com>
In-Reply-To: <pull.1098.v2.git.1640011691.gitgitgadget@gmail.com>

From: Derrick Stolee <dstolee@microsoft.com>

Historically, we needed a single packfile in order to have reachability bitmaps. This introduced logic that when 'git repack' had a '-b' option that we should stop sending the '--honor-pack-keep' option to the 'git pack-objects' child process, ensuring that we create a packfile containing all reachable objects.

In the world of multi-pack-index bitmaps, we no longer need to repack all objects into a single pack to have valid bitmaps. Thus, we should continue sending the '--honor-pack-keep' flag to 'git pack-objects'.

The fix is very simple: only disable the flag when writing bitmaps but also _not_ writing the multi-pack-index.

This opens the door to new repacking strategies that might want to keep some historical set of objects in a stable pack-file while only repacking more recent objects.

To test, create a new 'test_subcommand_inexact' helper that is more flexible than 'test_subcommand'. This allows us to look for the --honor-pack-keep flag without over-indexing on the exact set of arguments.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 builtin/repack.c        |  2 +-
 t/t7700-repack.sh       |  6 ++++++
 t/test-lib-functions.sh | 34 ++++++++++++++++++++++++++++++++++
 3 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/builtin/repack.c b/builtin/repack.c
index 9b0be6a6ab3..1f128b7c90b 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -693,7 +693,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
 		write_bitmaps = 0;
 	}
 	if (pack_kept_objects < 0)
-		pack_kept_objects = write_bitmaps > 0;
+		pack_kept_objects = write_bitmaps > 0 && !write_midx;
 
 	if (write_bitmaps && !(pack_everything & ALL_INTO_ONE) && !write_midx)
 		die(_(incremental_bitmap_conflict_error));
diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh
index 0260ad6f0e0..63c9a247f57 100755
--- a/t/t7700-repack.sh
+++ b/t/t7700-repack.sh
@@ -372,4 +372,10 @@ test_expect_success '--write-midx with preferred bitmap tips' '
 	)
 '
 
+test_expect_success '--write-midx -b packs non-kept objects' '
+	GIT_TRACE2_EVENT="$(pwd)/trace.txt" \
+		git repack --write-midx -a -b &&
+	test_subcommand_inexact git pack-objects --honor-pack-keep <trace.txt
+'
+
 test_done
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 389153e5916..c3d38aaccbd 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -1759,6 +1759,40 @@ test_subcommand () {
 	fi
 }
 
+# Check that the given command was invoked as part of the
+# trace2-format trace on stdin, but without an exact set of
+# arguments.
+#
+#	test_subcommand [!] <command> <args>... < <trace>
+#
+# For example, to look for an invocation of "git pack-objects"
+# with the "--honor-pack-keep" argument, use
+#
+#	GIT_TRACE2_EVENT=event.log git repack ... &&
+#	test_subcommand git pack-objects --honor-pack-keep <event.log
+#
+# If the first parameter passed is !, this instead checks that
+# the given command was not called.
+#
+test_subcommand_inexact () {
+	local negate=
+	if test "$1" = "!"
+	then
+		negate=t
+		shift
+	fi
+
+	local expr=$(printf '"%s".*' "$@")
+	expr="${expr%,}"
+
+	if test -n "$negate"
+	then
+		! grep "\"event\":\"child_start\".*\[$expr\]"
+	else
+		grep "\"event\":\"child_start\".*\[$expr\]"
+	fi
+}
+
 # Check that the given command was invoked as part of the
 # trace2-format trace on stdin.
 #

-- 
gitgitgadget