[PATCH 3/6] fetch-pack: in protocol v2, enqueue commons first
- From
- Jonathan Tan <jonathantanmy@google.com>
- Date
- Jun 4, 2018, 17:29 UTC
- Message-ID
- <8265d9485a2bfe0e81d68495afd3733879add992.1527894919.git.jonathantanmy@google.com>
- In-Reply-To
- <cover.1527894919.git.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