[PATCH 6/6] fetch-pack: fix deepen shallow over smart http with no-done cap

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 both sides
happy. The server 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@xxxxxxxx>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
---
 fetch-pack.c             |  3 ++-
 t/t5537-fetch-shallow.sh | 24 ++++++++++++++++++++++++
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/fetch-pack.c b/fetch-pack.c
index 90fdd49..f061f1f 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -439,7 +439,8 @@ done:
 	}
 	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 b0fa738..fb11073 100755
--- a/t/t5537-fetch-shallow.sh
+++ b/t/t5537-fetch-shallow.sh
@@ -200,5 +200,29 @@ 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 the server
+# does not have, it'll send "ACK %s ready"
+test_expect_success 'add more commits' '
+	(
+	cd shallow &&
+	for i in $(seq 10); do
+	git checkout --orphan unrelated$i &&
+	test_commit unrelated$i >/dev/null &&
+	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
+	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 fetch --depth=2
+	)
+'
+
 stop_httpd
 test_done
-- 
1.8.5.2.240.g8478abd

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]