diff mbox series

[1/1] revision: don't set parents as uninteresting if exclude promisor objects

Message ID 20240719093435.69238-2-hanyang.tony@bytedance.com (mailing list archive)
State New, archived
Headers show
Series Fix bug in revision walk if --exclude-promisor-objects is set | expand

Commit Message

Han Young July 19, 2024, 9:34 a.m. UTC
In revision.c, `process_parents()` recursively marks commit parents 
as UNINTERESTING if the commit itself is UNINTERESTING.
`--exclude-promisor-objects` is implemented as 
"iterate all objects in promisor packfiles, mark them as UNINTERESTING".
So when we find a commit object in a promisor packfile, we also set its ancestors 
as UNINTERESTING, whether the ancestor is a promisor object or not.
This causes normal objects to be falsely marked as promisor objects 
and removed by `git repack`.

Stop setting the parents of uninteresting commits' to UNINTERESTING 
when we exclude promisor objects, and add a test to prevent regression.

Note that this change would cause rev-list to report incorrect results if 
`--exclude-promisor-objects` is used with other revision walk filters. But 
`--exclude-promisor-objects` is for internal use only, so we don't have to worry
about users using other filters with `--exclude-promisor-objects`.

Signed-off-by: Han Young <hanyang.tony@bytedance.com>
Helped-by: C O Xing Xin <xingxin.xx@bytedance.com>
---
 revision.c               |  2 +-
 t/t0410-partial-clone.sh | 22 +++++++++++++++++++++-
 2 files changed, 22 insertions(+), 2 deletions(-)
diff mbox series

Patch

diff --git a/revision.c b/revision.c
index 1c0192f522..eacb0c909d 100644
--- a/revision.c
+++ b/revision.c
@@ -1164,7 +1164,7 @@  static int process_parents(struct rev_info *revs, struct commit *commit,
 	 * wasn't uninteresting), in which case we need
 	 * to mark its parents recursively too..
 	 */
-	if (commit->object.flags & UNINTERESTING) {
+	if (!revs->exclude_promisor_objects && commit->object.flags & UNINTERESTING) {
 		while (parent) {
 			struct commit *p = parent->item;
 			parent = parent->next;
diff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh
index 2c30c86e7b..4ee3437685 100755
--- a/t/t0410-partial-clone.sh
+++ b/t/t0410-partial-clone.sh
@@ -22,6 +22,17 @@  pack_as_from_promisor () {
 	echo $HASH
 }
 
+pack_commit() {
+	HASH=$(echo $1 | git -C repo pack-objects .git/objects/pack/pack --missing=allow-any) &&
+	delete_object repo $1 &&
+	echo $HASH
+}
+
+pack_commit_as_promisor() {
+	HASH=$(pack_commit $1) &&
+	>repo/.git/objects/pack/pack-$HASH.promisor
+}
+
 promise_and_delete () {
 	HASH=$(git -C repo rev-parse "$1") &&
 	git -C repo tag -a -m message my_annotated_tag "$HASH" &&
@@ -369,7 +380,16 @@  test_expect_success 'missing tree objects with --missing=allow-promisor and --ex
 	git -C repo rev-list --exclude-promisor-objects --objects HEAD >objs 2>rev_list_err &&
 	test_must_be_empty rev_list_err &&
 	# 3 commits, no blobs or trees
-	test_line_count = 3 objs
+	test_line_count = 3 objs &&
+
+	# Pack all three commits into individual packs, and mark the last commit pack as promisor
+	pack_commit_as_promisor $(git -C repo rev-parse baz) &&
+	pack_commit $(git -C repo rev-parse bar) &&
+	pack_commit $(git -C repo rev-parse foo) &&
+	git -C repo rev-list --exclude-promisor-objects --objects HEAD >objs 2>rev_list_err &&
+	test_must_be_empty rev_list_err &&
+	# commits foo and bar should remain
+	test_line_count = 2 objs
 '
 
 test_expect_success 'missing non-root tree object and rev-list' '