[PATCH 3/6] fetch-pack: in protocol v2, enqueue commons first

[Jonathan Tan <jonathantanmy@google.com>]

In do_fetch_pack_v2(), rev_list_insert_ref_oid() is invoked before
everything_local(). This means that if we have a commit that is both our
ref and their ref, it would be enqueued by rev_list_insert_ref_oid() as
SEEN, and since it is thus already SEEN, everything_local() would not
enqueue it.

If everything_local() were invoked first, as it is in do_fetch_pack()
for protocol v0, then everything_local() would enqueue it with
COMMON_REF | SEEN. The addition of COMMON_REF ensures that its parents
are not sent as "have" lines.

Change the order in do_fetch_pack_v2() to be consistent with
do_fetch_pack(), and to avoid sending unnecessary "have" lines.

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
 fetch-pack.c          |  6 +++---
 t/t5500-fetch-pack.sh | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/fetch-pack.c b/fetch-pack.c
index 2d090f612..192771a8f 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1372,14 +1372,14 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
 				for_each_ref(clear_marks, NULL);
 			marked = 1;
 
-			for_each_ref(rev_list_insert_ref_oid, NULL);
-			for_each_cached_alternate(insert_one_alternate_object);
-
 			/* Filter 'ref' by 'sought' and those that aren't local */
 			if (everything_local(args, &ref, sought, nr_sought))
 				state = FETCH_DONE;
 			else
 				state = FETCH_SEND_REQUEST;
+
+			for_each_ref(rev_list_insert_ref_oid, NULL);
+			for_each_cached_alternate(insert_one_alternate_object);
 			break;
 		case FETCH_SEND_REQUEST:
 			if (send_fetch_request(fd[1], args, ref, &common,
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index 0680dec80..ad6a50ad6 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -808,6 +808,41 @@ test_expect_success 'fetch with --filter=blob:limit=0' '
 	fetch_filter_blob_limit_zero server server
 '
 
+test_expect_success 'use ref advertisement to prune "have" lines sent' '
+	rm -rf server client &&
+	git init server &&
+	test_commit -C server aref_both_1 &&
+	git -C server tag -d aref_both_1 &&
+	test_commit -C server aref_both_2 &&
+
+	# The ref name that only the server has must be a prefix of all the
+	# others, to ensure that the client has the same information regardless
+	# of whether protocol v0 (which does not have ref prefix filtering) or
+	# protocol v2 (which does) is used.
+	git clone server client &&
+	test_commit -C server aref &&
+	test_commit -C client aref_client &&
+
+	# In both protocol v0 and v2, ensure that the parent of aref_both_2 is
+	# not sent as a "have" line.
+
+	rm -f trace &&
+	cp -r client clientv0 &&
+	GIT_TRACE_PACKET="$(pwd)/trace" git -C clientv0 \
+		fetch origin aref &&
+	grep "have $(git -C client rev-parse aref_client)" trace &&
+	grep "have $(git -C client rev-parse aref_both_2)" trace &&
+	! grep "have $(git -C client rev-parse aref_both_2^)" trace &&
+
+	rm -f trace &&
+	cp -r client clientv2 &&
+	GIT_TRACE_PACKET="$(pwd)/trace" git -C clientv2 -c protocol.version=2 \
+		fetch origin aref &&
+	grep "have $(git -C client rev-parse aref_client)" trace &&
+	grep "have $(git -C client rev-parse aref_both_2)" trace &&
+	! grep "have $(git -C client rev-parse aref_both_2^)" trace
+'
+
 . "$TEST_DIRECTORY"/lib-httpd.sh
 start_httpd
 
-- 
2.17.0.768.g1526ddbba1.dirty