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

[Junio C Hamano <gitster@pobox.com>]

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> 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

In addition to two problems Eric and Peff noticed, && chain is
broken between these two pushes.  I initially didn't notice it but
it became obvious after reformatting to fix the indentation.

Here is the difference between the posted series and what I queued
after applying the changes suggested during the review.

Thanks.

 Documentation/technical/pack-protocol.txt         |  4 +--
 Documentation/technical/protocol-capabilities.txt | 10 +++----
 t/t5537-fetch-shallow.sh                          | 34 +++++++++++++----------
 3 files changed, 26 insertions(+), 22 deletions(-)

diff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt
index eb0b8a1..39c6410 100644
--- a/Documentation/technical/pack-protocol.txt
+++ b/Documentation/technical/pack-protocol.txt
@@ -338,8 +338,8 @@ during a prior round.  This helps to ensure that at least one common
 ancestor is found before we give up entirely.
 
 Once the 'done' line is read from the client, the server will either
-send a final 'ACK obj-id' or it will send a 'NAK'. 'obj-id' is the
-last SHA-1 determined to be common. The server only sends
+send a final 'ACK obj-id' or it will send a 'NAK'. 'obj-id' is the object
+name of the last commit determined to be common. The server only sends
 ACK after 'done' if there is at least one common base and multi_ack or
 multi_ack_detailed is enabled. The server always sends NAK after 'done'
 if there is no common base found.
diff --git a/Documentation/technical/protocol-capabilities.txt b/Documentation/technical/protocol-capabilities.txt
index cb2f5eb..e174343 100644
--- a/Documentation/technical/protocol-capabilities.txt
+++ b/Documentation/technical/protocol-capabilities.txt
@@ -77,15 +77,15 @@ section "Packfile Negotiation" for more information.
 
 no-done
 -------
-This capability should be only used with smart HTTP protocol. If
+This capability should only be used with the smart HTTP protocol. If
 multi_ack_detailed and no-done are both present, then the sender is
 free to immediately send a pack following its first "ACK obj-id ready"
 message.
 
-Without no-done in smart HTTP protocol, the server session would end
-and the client has to make another trip to send "done" and the server
-can send the pack. no-done removes the last round and thus slightly
-reduces latency.
+Without no-done in the smart HTTP protocol, the server session would
+end and the client has to make another trip to send "done" before
+the server can send the pack. no-done removes the last round and
+thus slightly reduces latency.
 
 thin-pack
 ---------
diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh
index fb11073..1413caf 100755
--- a/t/t5537-fetch-shallow.sh
+++ b/t/t5537-fetch-shallow.sh
@@ -201,26 +201,30 @@ 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
+# 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 '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 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 fetch --depth=2
+		cd clone &&
+		git checkout --orphan newnew &&
+		test_commit new-too &&
+		git fetch --depth=2
 	)
 '