[PATCH 2/6] pack-objects: keep --keep-pack open when following

["Qin ShiCheng via GitGitGadget" <gitgitgadget@gmail.com>]

From: Qin ShiCheng <qeesung@live.com>

"--stdin-packs=follow" distinguishes excluded packs that are closed
under reachability ("^") from those that are not ("!"). The traversal
stops at objects in the former, and goes on through the latter to
rescue whatever they depend on that would otherwise be left out.

A pack named with "--keep-pack" gets the same in-core flag as a "^"
pack, so the traversal stops at it too. Nothing warrants that: the
caller said not to repack it, not that it is self-contained. When it
holds a commit but not that commit's tree, the tree is never rescued,
and writing a bitmap over the result fails for lack of closure.

In follow mode, mark such a pack as kept-open instead, the way repack
already lists the packs it cannot vouch for as "!" on stdin. Its
objects stay out of the result, and the traversal can go through it.

This matters more once repack names its ".keep" packs this way instead
of passing "--honor-pack-keep": on-disk kept packs never were a
boundary, and they should not become one.

Signed-off-by: Qin ShiCheng <qeesung@live.com>
---
 builtin/pack-objects.c        | 20 +++++++++++++----
 t/t5331-pack-objects-stdin.sh | 41 +++++++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+), 4 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 708b719f40..6f579173b0 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -4999,7 +4999,8 @@ static void get_object_list(struct rev_info *revs, struct strvec *argv)
 	oid_array_clear(&recent_objects);
 }
 
-static void add_extra_kept_packs(const struct string_list *names)
+static void add_extra_kept_packs(const struct string_list *names,
+				 enum stdin_packs_mode stdin_packs)
 {
 	struct packed_git *p;
 
@@ -5018,8 +5019,19 @@ static void add_extra_kept_packs(const struct string_list *names)
 				break;
 
 		if (i < names->nr) {
-			p->pack_keep_in_core = 1;
-			ignore_packed_keep_in_core = 1;
+			/*
+			 * When following, treat the pack like a "!" pack, not
+			 * a "^" one: nobody said it is closed under
+			 * reachability, so the traversal must be able to go
+			 * through it.
+			 */
+			if (stdin_packs == STDIN_PACKS_MODE_FOLLOW) {
+				p->pack_keep_in_core_open = 1;
+				ignore_packed_keep_in_core_open = 1;
+			} else {
+				p->pack_keep_in_core = 1;
+				ignore_packed_keep_in_core = 1;
+			}
 			continue;
 		}
 	}
@@ -5443,7 +5455,7 @@ int cmd_pack_objects(int argc,
 	if (progress && all_progress_implied)
 		progress = 2;
 
-	add_extra_kept_packs(&keep_pack_list);
+	add_extra_kept_packs(&keep_pack_list, stdin_packs);
 	if (ignore_packed_keep_on_disk) {
 		struct packed_git *p;
 
diff --git a/t/t5331-pack-objects-stdin.sh b/t/t5331-pack-objects-stdin.sh
index c74b5861af..4e1fde1b08 100755
--- a/t/t5331-pack-objects-stdin.sh
+++ b/t/t5331-pack-objects-stdin.sh
@@ -483,6 +483,47 @@ test_expect_success '--stdin-packs=follow with open-excluded packs' '
 	)
 '
 
+test_expect_success '--stdin-packs=follow walks through a --keep-pack pack' '
+	test_when_finished "rm -fr repo" &&
+
+	git init repo &&
+	(
+		cd repo &&
+		git config set maintenance.auto false &&
+
+		test_commit A &&
+		test_commit B &&
+		test_commit C &&
+
+		A="$(echo A | git pack-objects --revs $packdir/pack)" &&
+		B="$(echo A..B | git pack-objects --revs $packdir/pack)" &&
+		C="$(echo B..C | git pack-objects --revs $packdir/pack)" &&
+		B_ONLY="$(git rev-parse B | git pack-objects $packdir/pack)" &&
+		git prune-packed &&
+
+		# Pack C is included and pack A is excluded and closed. The
+		# commit B is in the kept pack B_ONLY, but its tree and blob
+		# are only in pack B, which pack-objects is not told about.
+		# The kept pack keeps B out of the result, and the walk has
+		# to go through it to rescue the tree and the blob.
+		P=$(git pack-objects --stdin-packs=follow \
+			--keep-pack=pack-$B_ONLY.pack $packdir/pack <<-EOF
+		pack-$C.pack
+		^pack-$A.pack
+		EOF
+		) &&
+
+		{
+			objects_in_packs $C &&
+			git rev-parse "B^{tree}" B:B.t
+		} >expect.raw &&
+		sort expect.raw >expect &&
+
+		objects_in_packs $P >actual &&
+		test_cmp expect actual
+	)
+'
+
 test_expect_success '--stdin-packs with !-delimited pack without follow' '
 	test_when_finished "rm -fr repo" &&
 
-- 
gitgitgadget