[PATCH 2/3] pack-objects: inspect selected haves for shared objects

["Marc Strapetz via GitGitGadget" <gitgitgadget@gmail.com>]

From: Marc Strapetz <marc.strapetz@syntevo.com>

A receiver-known commit can be excluded from a revision walk without
becoming a boundary commit. Its tree then need not be inspected, so
objects shared with the outgoing history can still be packed.

Accept --inspect-have <commit> lines in --revs input and carry the
selected tips through the object database pack-generation options.
Compare their root trees with the outgoing tips using the existing
sparse tree marking. This identifies additional receiver-known
objects without walking the selected commits' histories and avoids
descending into identical or receiver-only subtrees.

Resolve the selected commits in pack-objects, where replacement refs
are already disabled, so local replacements cannot substitute trees
the receiver does not have. Keep successful bitmap enumeration on its
existing path and skip the optional inspection in partial clones.

Cover identical root trees, shared blobs under different roots,
retained changed objects, subtree pruning, non-commit input, partial
clone fallback and shared objects in earlier outgoing commits.

Assisted-by: GPT-6 Astra, GPT-6.1 Sol
Signed-off-by: Marc Strapetz <marc.strapetz@syntevo.com>
---
 Documentation/git-pack-objects.adoc |   7 ++
 builtin/pack-objects.c              |  81 ++++++++++++++-
 odb.c                               |   1 +
 odb.h                               |   8 ++
 odb/source-files.c                  |   3 +
 t/t5322-pack-objects-sparse.sh      | 148 ++++++++++++++++++++++++++++
 6 files changed, 247 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-pack-objects.adoc b/Documentation/git-pack-objects.adoc
index ccad938c5b..a98d4286fa 100644
--- a/Documentation/git-pack-objects.adoc
+++ b/Documentation/git-pack-objects.adoc
@@ -70,6 +70,13 @@ base-name::
 	outputs.  The objects on the resulting list are packed.
 	Besides revisions, `--not` or `--shallow <SHA-1>` lines are
 	also accepted.
++
+A `--inspect-have <commit>` line supplies the object ID of a commit known
+to the receiver. Before non-bitmap enumeration, its root tree is compared
+with the outgoing tips' root trees to identify additional objects that
+need not be packed, without walking its commit history. Identical
+subtrees and paths found only in the receiver-known trees are skipped.
+This optional inspection is skipped in partial clones.
 
 --unpacked::
 	This implies `--revs`.  When processing the list of
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index ca3a891dfb..9abb4b9fe3 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -4892,12 +4892,76 @@ static int get_object_list_path_walk(struct rev_info *revs)
 	return result;
 }
 
+static void add_have_to_inspect(struct oidset *trees, const char *arg)
+{
+	struct object_id oid;
+	struct commit *commit;
+	struct tree *tree;
+	const char *end;
+
+	if (parse_oid_hex(arg, &oid, &end) || *end)
+		die(_("not an object name '%s'"), arg);
+	if (!odb_has_object(the_repository->objects, &oid, 0))
+		return;
+	commit = lookup_commit_reference_gently(the_repository, &oid, 1);
+	if (!commit)
+		die(_("object %s is not a commit"), arg);
+	tree = repo_get_commit_tree(the_repository, commit);
+	if (tree)
+		oidset_insert(trees, &tree->object.oid);
+}
+
+static void inspect_haves(struct rev_info *revs, struct oidset *trees)
+{
+	struct oidset_iter iter;
+	struct object_id *oid;
+
+	/* Avoid fetching missing trees for this optional optimization. */
+	if (!oidset_size(trees) || repo_has_promisor_remote(revs->repo))
+		return;
+
+	/* Mark receiver-known root trees as uninteresting. */
+	oidset_iter_init(trees, &iter);
+	while ((oid = oidset_iter_next(&iter))) {
+		struct tree *tree = lookup_tree(revs->repo, oid);
+
+		if (tree)
+			tree->object.flags |= UNINTERESTING;
+	}
+
+	/*
+	 * Add outgoing root trees for the same sparse tree marking used by
+	 * mark_edges_uninteresting(). A receiver-known tip on the other side of
+	 * a divergence can be a negative revision without its tree being visited
+	 * by the ordinary walk. Combining its tree with the outgoing trees lets
+	 * us exclude shared objects without walking that tip's commit history.
+	 */
+	for (size_t i = 0; i < revs->pending.nr; i++) {
+		struct object *obj = revs->pending.objects[i].item;
+
+		if (obj->flags & UNINTERESTING)
+			continue;
+		if (obj->type == OBJ_COMMIT) {
+			struct tree *tree = repo_get_commit_tree(revs->repo,
+							(struct commit *)obj);
+
+			if (tree)
+				oidset_insert(trees, &tree->object.oid);
+		} else if (obj->type == OBJ_TREE) {
+			oidset_insert(trees, &obj->oid);
+		}
+	}
+
+	mark_trees_uninteresting_sparse(revs->repo, trees);
+}
+
 static void get_object_list(struct rev_info *revs, struct strvec *argv)
 {
 	struct setup_revision_opt s_r_opt = {
 		.allow_exclude_promisor_objects = 1,
 	};
 	struct repo_config_values *cfg = repo_config_values(the_repository);
+	struct oidset have_trees = OIDSET_INIT;
 	char line[1000];
 	int flags = 0;
 	int save_warning;
@@ -4931,6 +4995,11 @@ static void get_object_list(struct rev_info *revs, struct strvec *argv)
 				use_bitmap_index = 0;
 				continue;
 			}
+			if (starts_with(line, "--inspect-have ")) {
+				add_have_to_inspect(&have_trees, line + 15);
+				write_bitmap_index = 0;
+				continue;
+			}
 			die(_("not a rev '%s'"), line);
 		}
 		if (handle_revision_arg(line, revs, flags, REVARG_CANNOT_BE_FILENAME))
@@ -4939,8 +5008,18 @@ static void get_object_list(struct rev_info *revs, struct strvec *argv)
 
 	cfg->warn_on_object_refname_ambiguity = save_warning;
 
-	if (use_bitmap_index && !get_object_list_from_bitmap(revs))
+	if (use_bitmap_index && !get_object_list_from_bitmap(revs)) {
+		oidset_clear(&have_trees);
 		return;
+	}
+
+	/*
+	 * Limit this inspection to non-bitmap enumeration to avoid extra tree
+	 * reads on the bitmap fast path. Boundary bitmap traversal may still
+	 * include receiver-known objects; that tradeoff is intentional.
+	 */
+	inspect_haves(revs, &have_trees);
+	oidset_clear(&have_trees);
 
 	if (use_delta_islands)
 		load_delta_islands(the_repository, progress);
diff --git a/odb.c b/odb.c
index 98c5c862f5..2bb27848ac 100644
--- a/odb.c
+++ b/odb.c
@@ -731,6 +731,7 @@ void odb_generate_pack_options_release(struct odb_generate_pack_options *opts)
 {
 	oid_array_clear(&opts->wants);
 	oid_array_clear(&opts->haves);
+	oid_array_clear(&opts->haves_to_inspect);
 	oid_array_clear(&opts->shallows);
 }
 
diff --git a/odb.h b/odb.h
index 0ccc47ee73..10629b2044 100644
--- a/odb.h
+++ b/odb.h
@@ -667,6 +667,13 @@ struct odb_generate_pack_options {
 	 */
 	struct oid_array haves;
 
+	/*
+	 * Receiver-known commit tips whose root trees should be examined
+	 * to identify additional objects that need not be packed. This
+	 * optional inspection does not walk their commit histories.
+	 */
+	struct oid_array haves_to_inspect;
+
 	/*
 	 * The shallow boundary that shall be used when computing object
 	 * reachability. When set, any shallow information of the repository
@@ -751,6 +758,7 @@ struct odb_generate_pack_options {
 #define ODB_GENERATE_PACK_OPTIONS_INIT { \
 	.wants = OID_ARRAY_INIT, \
 	.haves = OID_ARRAY_INIT, \
+	.haves_to_inspect = OID_ARRAY_INIT, \
 	.shallows = OID_ARRAY_INIT, \
 	.pack_fd = -1, \
 }
diff --git a/odb/source-files.c b/odb/source-files.c
index 455f409db0..bad9ecde4c 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -1176,6 +1176,9 @@ static int odb_source_files_generate_pack(struct odb_source *source UNUSED,
 	fprintf(in, "--not\n");
 	for (size_t i = 0; i < opts->haves.nr; i++)
 		fprintf(in, "%s\n", oid_to_hex(&opts->haves.oid[i]));
+	for (size_t i = 0; i < opts->haves_to_inspect.nr; i++)
+		fprintf(in, "--inspect-have %s\n",
+			oid_to_hex(&opts->haves_to_inspect.oid[i]));
 	fprintf(in, "\n");
 	fflush(in);
 	if (ferror(in)) {
diff --git a/t/t5322-pack-objects-sparse.sh b/t/t5322-pack-objects-sparse.sh
index d39958c066..0cf48513eb 100755
--- a/t/t5322-pack-objects-sparse.sh
+++ b/t/t5322-pack-objects-sparse.sh
@@ -138,4 +138,152 @@ test_expect_success 'pack.useSparse overridden' '
 	test_cmp required_objects.txt sparse_objects.txt
 '
 
+test_expect_success 'setup inspect-have receiver and outgoing commits' '
+	git checkout -b receiver main &&
+	echo large >large.bin &&
+	echo receiver >f2/f2/data.txt &&
+	git add large.bin f2/f2/data.txt &&
+	test_commit --no-tag receiver file.txt &&
+	git checkout -b outgoing main &&
+	git checkout receiver -- large.bin f2 &&
+	git rm -r f3 &&
+	test_commit --no-tag outgoing file.txt &&
+	receiver_tip=$(git rev-parse receiver) &&
+	cat >packinput.txt <<-EOF
+	outgoing
+	^main
+	^receiver
+	--inspect-have $receiver_tip
+	EOF
+'
+
+test_expect_success 'inspect-have excludes an identical root tree' '
+	tree=$(git rev-parse receiver^{tree}) &&
+	same_tip=$(git commit-tree "$tree" -p main -m "Same tree") &&
+	cat >same-tree-input.txt <<-EOF &&
+	$same_tip
+	^main
+	^receiver
+	--inspect-have $receiver_tip
+	EOF
+	echo "$same_tip" >expect_objects.txt &&
+	git pack-objects --stdout --revs <same-tree-input.txt >inspected.pack &&
+	git index-pack -o inspected.idx inspected.pack &&
+	git show-index <inspected.idx >index &&
+	awk "{print \$2}" index >inspected_objects.txt &&
+	test_cmp_sorted expect_objects.txt inspected_objects.txt
+'
+
+test_expect_success 'inspect-have excludes shared large blob from different root trees' '
+	test_cmp_rev ! receiver^{tree} outgoing^{tree} &&
+	test_cmp_rev receiver:large.bin outgoing:large.bin &&
+	blob=$(git rev-parse receiver:large.bin) &&
+	sed "/^--inspect-have /d" packinput.txt >ordinary-input.txt &&
+	git pack-objects --stdout --revs <ordinary-input.txt >ordinary.pack &&
+	git index-pack -o ordinary.idx ordinary.pack &&
+	git show-index <ordinary.idx >index &&
+	awk "{print \$2}" index >ordinary_objects.txt &&
+	test_grep "$blob" ordinary_objects.txt &&
+	git rev-parse outgoing outgoing^{tree} outgoing:file.txt >expect_objects.txt &&
+	git pack-objects --stdout --revs <packinput.txt >inspected.pack &&
+	git index-pack -o inspected.idx inspected.pack &&
+	git show-index <inspected.idx >index &&
+	awk "{print \$2}" index >inspected_objects.txt &&
+	test_cmp_sorted expect_objects.txt inspected_objects.txt
+'
+
+test_expect_success 'inspect-have keeps changed blobs and trees' '
+	git checkout -b outgoing-changed outgoing &&
+	test_commit changed f2/f2/data.txt &&
+	sed "s/^outgoing$/outgoing-changed/" packinput.txt >changed-input.txt &&
+	git rev-parse \
+		outgoing \
+		outgoing^{tree} \
+		outgoing:file.txt \
+		outgoing-changed \
+		outgoing-changed^{tree} \
+		outgoing-changed:f2 \
+		outgoing-changed:f2/f2 \
+		outgoing-changed:f2/f2/data.txt >changed_objects.txt &&
+	git pack-objects --stdout --revs <changed-input.txt >inspected.pack &&
+	git index-pack -o inspected.idx inspected.pack &&
+	git show-index <inspected.idx >index &&
+	awk "{print \$2}" index >inspected_objects.txt &&
+	test_cmp_sorted changed_objects.txt inspected_objects.txt
+'
+
+test_expect_success 'inspect-have skips unchanged and receiver-only subtrees' '
+	git rev-parse receiver:f2 receiver:f3 >trees &&
+	git pack-objects --stdout --window=0 <trees >trees.pack &&
+	git index-pack --stdin <trees.pack &&
+	while read oid
+	do
+		path=$(test_oid_to_path "$oid") &&
+		rm ".git/objects/$path" || return 1
+	done <trees &&
+	>pack-access &&
+	GIT_TRACE_PACK_ACCESS="$(pwd)/pack-access" \
+	git pack-objects --stdout --revs <packinput.txt >inspected.pack &&
+	test_must_be_empty pack-access &&
+	git rev-parse outgoing outgoing^{tree} outgoing:file.txt >expect_objects.txt &&
+	git index-pack -o inspected.idx inspected.pack &&
+	git show-index <inspected.idx >index &&
+	awk "{print \$2}" index >inspected_objects.txt &&
+	test_cmp_sorted expect_objects.txt inspected_objects.txt
+'
+
+test_expect_success 'inspect-have rejects non-commit objects' '
+	blob=$(git rev-parse receiver:f2/f2/data.txt) &&
+	echo "--inspect-have $blob" >invalid-input.txt &&
+	test_must_fail git pack-objects --stdout --revs \
+		<invalid-input.txt >invalid.pack 2>err &&
+	test_grep "is not a commit" err
+'
+
+test_expect_success 'inspect-have is ignored in partial clones' '
+	test_config remote.missing.url "$(pwd)/missing" &&
+	test_config remote.missing.promisor true &&
+	sed "/^--inspect-have /d" packinput.txt >partial-input.txt &&
+	git pack-objects --stdout --revs --thin --window=0 \
+		<partial-input.txt >ordinary.pack 2>err &&
+	test_must_be_empty err &&
+	git pack-objects --stdout --revs --thin --window=0 \
+		<packinput.txt >inspected.pack 2>err &&
+	test_must_be_empty err &&
+	git index-pack -o ordinary.idx ordinary.pack &&
+	git show-index <ordinary.idx >index &&
+	awk "{print \$2}" index >ordinary_objects.txt &&
+	git index-pack -o inspected.idx inspected.pack &&
+	git show-index <inspected.idx >index &&
+	awk "{print \$2}" index >inspected_objects.txt &&
+	test_cmp ordinary_objects.txt inspected_objects.txt
+'
+
+test_expect_success 'inspect-have excludes shared blobs from earlier outgoing commits' '
+	git checkout -b receiver-extra receiver &&
+	test_commit --no-tag extra f2/f2/small &&
+	git checkout -b outgoing-extra outgoing &&
+	test_commit --no-tag extra f2/f2/small &&
+	receiver_extra_tip=$(git rev-parse receiver-extra) &&
+	cat >tip-input.txt <<-EOF &&
+	outgoing-extra
+	^main
+	^receiver-extra
+	--inspect-have $receiver_extra_tip
+	EOF
+	git rev-parse \
+		outgoing \
+		outgoing^{tree} \
+		outgoing:f2 \
+		outgoing:f2/f2 \
+		outgoing:file.txt \
+		outgoing-extra \
+		outgoing-extra^{tree} >expect_objects.txt &&
+	git pack-objects --stdout --revs <tip-input.txt >inspected.pack &&
+	git index-pack -o inspected.idx inspected.pack &&
+	git show-index <inspected.idx >index &&
+	awk "{print \$2}" index >inspected_objects.txt &&
+	test_cmp_sorted expect_objects.txt inspected_objects.txt
+'
+
 test_done
-- 
gitgitgadget