git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Junio C Hamano <gitster@pobox.com>
Date
Feb 7, 2014, 18:01 UTC
Message-ID
<xmqqppmyvm3f.fsf@gitster.dls.corp.google.com>
In-Reply-To
<1391699439-22781-7-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
Show 19 quoted lines
> 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
 	)
 '
Previous: Jeff KingNext: Duy Nguyen
Message 37 of 44 in “Fix the shallow deepen bug with no-done”
  1. 0/6 Fix the shallow deepen bug with no-doneNguyễn Thái Ngọc Duy, Feb 6, 2014
  2. 1/6 test: rename http fetch and push test filesNguyễn Thái Ngọc Duy, Feb 6, 2014
  3. Jeff KingFeb 6, 2014
  4. 2/6 t5538: fix default http portNguyễn Thái Ngọc Duy, Feb 6, 2014
  5. Jeff KingFeb 6, 2014
  6. Jeff KingFeb 7, 2014
  7. Duy NguyenFeb 8, 2014
  8. Jeff KingFeb 10, 2014
  9. Junio C HamanoFeb 10, 2014
  10. Jeff KingFeb 10, 2014
  11. Jeff KingFeb 10, 2014
  12. tests: turn on network daemon tests by defaultJeff King, Feb 10, 2014
  13. Junio C HamanoFeb 11, 2014
  14. Jeff KingFeb 11, 2014
  15. Junio C HamanoFeb 11, 2014
  16. Jeff KingFeb 12, 2014
  17. Junio C HamanoFeb 12, 2014
  18. Junio C HamanoFeb 13, 2014
  19. Jeff KingFeb 14, 2014
  20. Junio C HamanoFeb 14, 2014
  21. Junio C HamanoFeb 12, 2014
  22. Jeff KingFeb 12, 2014
  23. Duy NguyenFeb 13, 2014
  24. t5537: move http tests out to t5539Nguyễn Thái Ngọc Duy, Feb 13, 2014
  25. Junio C HamanoFeb 13, 2014
  26. 3/6 pack-protocol.txt: clarify 'obj-id' in the last ACK after 'done'Nguyễn Thái Ngọc Duy, Feb 6, 2014
  27. Junio C HamanoFeb 6, 2014
  28. 4/6 protocol-capabilities.txt: refer multi_ack_detailed back to pack-protocol.txtNguyễn Thái Ngọc Duy, Feb 6, 2014
  29. 5/6 protocol-capabilities.txt: document no-doneNguyễn Thái Ngọc Duy, Feb 6, 2014
  30. Junio C HamanoFeb 6, 2014
  31. Jeff KingFeb 6, 2014
  32. 6/6 fetch-pack: fix deepen shallow over smart http with no-done capNguyễn Thái Ngọc Duy, Feb 6, 2014
  33. Junio C HamanoFeb 6, 2014
  34. Duy NguyenFeb 7, 2014
  35. Eric SunshineFeb 6, 2014
  36. Jeff KingFeb 6, 2014
  37. Junio C HamanoFeb 7, 2014
  38. Duy NguyenFeb 7, 2014
  39. Junio C HamanoFeb 10, 2014
  40. Junio C HamanoFeb 6, 2014
  41. Jeff KingFeb 6, 2014
  42. Duy NguyenFeb 7, 2014
  43. Jonathan NiederFeb 7, 2014
  44. Junio C HamanoFeb 7, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.