fetch-pack: fix deepen shallow over smart http with no-done cap

In smart http, upload-pack adds new shallow lines at the beginning of
each rpc response. Only shallow lines from the first rpc call are
useful. After that they are thrown away. It's designed this way
because upload-pack is stateless and has no idea when its shallow
lines are helpful or not.

So after refs are negotiated with multi_ack_detailed and the server
thinks it learned enough, it sends "ACK obj-id ready", terminates the
rpc call and waits for the final rpc round. The client sends "done".
The server sends another response, which also has shallow lines at
the beginning, and the last "ACK obj-id" line.

When no-done is active, the last round is cut out, the server sends
"ACK obj-id ready" and "ACK obj-id" in the same rpc
response. fetch-pack is updated to recognize this and not send
"done". However it still tries to consume shallow lines, which are
never sent.

Update the code, make sure to skip consuming shallow lines when
no-done is enabled.

Reported-by: Jeff King <peff@peff.net>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/fetch-pack.c b/fetch-pack.c
index 90fdd49821a1d6d3e104124444e37ac4b9b03fa2..f061f1fe85ea20549f860c4bf37980941f480833 100644 (file)
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -439,7 +439,8 @@ static int find_common(struct fetch_pack_args *args,
        }
        strbuf_release(&req_buf);
 
-       consume_shallow_list(args, fd[0]);
+       if (!got_ready || !no_done)
+               consume_shallow_list(args, fd[0]);
        while (flushes || multi_ack) {
                int ack = get_ack(fd[0], result_sha1);
                if (ack) {

diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh
index adf215a1937cf2ab2049e3e942497860e49d67cb..098f220bbee8905989862ef57a5af4391b02d58c 100755 (executable)
--- a/t/t5537-fetch-shallow.sh
+++ b/t/t5537-fetch-shallow.sh
@@ -199,5 +199,35 @@ EOF
        )
 '
 
+# This test is tricky. We need large enough "have"s that fetch-pack
+# will put pkt-flush in between. Then we need a "have" the server
+# does not have, it'll send "ACK %s ready"
+test_expect_success 'no shallow lines after receiving ACK ready' '
+       (
+               cd shallow &&
+               for i in $(test_seq 10)
+               do
+                       git checkout --orphan unrelated$i &&
+                       test_commit unrelated$i &&
+                       git push -q "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" \
+                               refs/heads/unrelated$i:refs/heads/unrelated$i &&
+                       git push -q ../clone/.git \
+                               refs/heads/unrelated$i:refs/heads/unrelated$i ||
+                       exit 1
+               done &&
+               git checkout master &&
+               test_commit new &&
+               git push  "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" master
+       ) &&
+       (
+               cd clone &&
+               git checkout --orphan newnew &&
+               test_commit new-too &&
+               GIT_TRACE_PACKET="$TRASH_DIRECTORY/trace" git fetch --depth=2 &&
+               grep "fetch-pack< ACK .* ready" ../trace &&
+               ! grep "fetch-pack> done" ../trace
+       )
+'
+
 stop_httpd
 test_done