From a87679339c0abb67b0be8b8b1cc10483f28df038 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Thu, 6 Feb 2014 22:10:34 +0700 Subject: [PATCH 1/6] test: rename http fetch and push test files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make clear which one is for dumb protocol, which one is for smart from their file name. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- t/{t5540-http-push.sh => t5540-http-push-webdav.sh} | 0 t/{t5541-http-push.sh => t5541-http-push-smart.sh} | 0 t/{t5550-http-fetch.sh => t5550-http-fetch-dumb.sh} | 0 t/{t5551-http-fetch.sh => t5551-http-fetch-smart.sh} | 0 4 files changed, 0 insertions(+), 0 deletions(-) rename t/{t5540-http-push.sh => t5540-http-push-webdav.sh} (100%) rename t/{t5541-http-push.sh => t5541-http-push-smart.sh} (100%) rename t/{t5550-http-fetch.sh => t5550-http-fetch-dumb.sh} (100%) rename t/{t5551-http-fetch.sh => t5551-http-fetch-smart.sh} (100%) diff --git a/t/t5540-http-push.sh b/t/t5540-http-push-webdav.sh similarity index 100% rename from t/t5540-http-push.sh rename to t/t5540-http-push-webdav.sh diff --git a/t/t5541-http-push.sh b/t/t5541-http-push-smart.sh similarity index 100% rename from t/t5541-http-push.sh rename to t/t5541-http-push-smart.sh diff --git a/t/t5550-http-fetch.sh b/t/t5550-http-fetch-dumb.sh similarity index 100% rename from t/t5550-http-fetch.sh rename to t/t5550-http-fetch-dumb.sh diff --git a/t/t5551-http-fetch.sh b/t/t5551-http-fetch-smart.sh similarity index 100% rename from t/t5551-http-fetch.sh rename to t/t5551-http-fetch-smart.sh From 32752e966d3370f6b411b42fe3a301bbd46d3eae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Thu, 6 Feb 2014 22:10:36 +0700 Subject: [PATCH 2/6] pack-protocol.txt: clarify 'obj-id' in the last ACK after 'done' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It's introduced in 1bd8c8f (git-upload-pack: Support the multi_ack protocol - 2005-10-28) but probably better documented in the commit message of 78affc4 (Add multi_ack_detailed capability to fetch-pack/upload-pack - 2009-10-30). Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- Documentation/technical/pack-protocol.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt index c73b62f5e..39c64105a 100644 --- a/Documentation/technical/pack-protocol.txt +++ b/Documentation/technical/pack-protocol.txt @@ -338,7 +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'. 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. From 087e347f2672b9a950d6da56179ca44eb7ded834 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Thu, 6 Feb 2014 22:10:37 +0700 Subject: [PATCH 3/6] protocol-capabilities.txt: refer multi_ack_detailed back to pack-protocol.txt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pack-protocol.txt explains in detail how multi_ack_detailed works and what's the difference between no multi_ack, multi_ack and multi_ack_detailed. No need to repeat here. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- Documentation/technical/protocol-capabilities.txt | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Documentation/technical/protocol-capabilities.txt b/Documentation/technical/protocol-capabilities.txt index e3e792476..cb40ebbd8 100644 --- a/Documentation/technical/protocol-capabilities.txt +++ b/Documentation/technical/protocol-capabilities.txt @@ -69,6 +69,12 @@ ends. Without multi_ack the client would have sent that c-b-a chain anyway, interleaved with S-R-Q. +multi_ack_detailed +------------------ +This is an extension of multi_ack that permits client to better +understand the server's in-memory state. See pack-protocol.txt, +section "Packfile Negotiation" for more information. + thin-pack --------- From c9cd60f6fa064595e4c1a8a84b9a15cc016a09e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Thu, 6 Feb 2014 22:10:38 +0700 Subject: [PATCH 4/6] protocol-capabilities.txt: document no-done MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See 3e63b21 (upload-pack: Implement no-done capability - 2011-03-14) and 761ecf0 (fetch-pack: Implement no-done capability - 2011-03-14) for more information. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- Documentation/technical/protocol-capabilities.txt | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/Documentation/technical/protocol-capabilities.txt b/Documentation/technical/protocol-capabilities.txt index cb40ebbd8..e17434384 100644 --- a/Documentation/technical/protocol-capabilities.txt +++ b/Documentation/technical/protocol-capabilities.txt @@ -75,6 +75,18 @@ This is an extension of multi_ack that permits client to better understand the server's in-memory state. See pack-protocol.txt, section "Packfile Negotiation" for more information. +no-done +------- +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 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 --------- From ff62eca7d1f9716e36550adedc6e8edc35ff9a15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Thu, 6 Feb 2014 22:10:39 +0700 Subject: [PATCH 5/6] fetch-pack: fix deepen shallow over smart http with no-done cap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- fetch-pack.c | 3 ++- t/t5537-fetch-shallow.sh | 30 ++++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/fetch-pack.c b/fetch-pack.c index 90fdd4982..f061f1fe8 100644 --- 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 adf215a19..098f220bb 100755 --- 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 From 0232852b06cb000a3b1f5f48676c8b4d084f18ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Thu, 13 Feb 2014 20:21:14 +0700 Subject: [PATCH 6/6] t5537: move http tests out to t5539 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit start_httpd is supposed to be at the beginning of the test file, not the middle of it. The "test_seq" line in "no shallow lines.." test is updated to compensate missing refs that are there in t5537, but not in the new t5539. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- t/t5537-fetch-shallow.sh | 57 ------------------------ t/t5539-fetch-http-shallow.sh | 82 +++++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 57 deletions(-) create mode 100755 t/t5539-fetch-http-shallow.sh diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh index 098f220bb..3ae9092f5 100755 --- a/t/t5537-fetch-shallow.sh +++ b/t/t5537-fetch-shallow.sh @@ -173,61 +173,4 @@ EOF ) ' -if test -n "$NO_CURL" -o -z "$GIT_TEST_HTTPD"; then - say 'skipping remaining tests, git built without http support' - test_done -fi - -. "$TEST_DIRECTORY"/lib-httpd.sh -start_httpd - -test_expect_success 'clone http repository' ' - git clone --bare --no-local shallow "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" && - git clone $HTTPD_URL/smart/repo.git clone && - ( - cd clone && - git fsck && - git log --format=%s origin/master >actual && - cat <expect && -7 -6 -5 -4 -3 -EOF - test_cmp expect actual - ) -' - -# 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 diff --git a/t/t5539-fetch-http-shallow.sh b/t/t5539-fetch-http-shallow.sh new file mode 100755 index 000000000..94553e103 --- /dev/null +++ b/t/t5539-fetch-http-shallow.sh @@ -0,0 +1,82 @@ +#!/bin/sh + +test_description='fetch/clone from a shallow clone over http' + +. ./test-lib.sh + +if test -n "$NO_CURL"; then + skip_all='skipping test, git built without http support' + test_done +fi + +. "$TEST_DIRECTORY"/lib-httpd.sh +start_httpd + +commit() { + echo "$1" >tracked && + git add tracked && + git commit -m "$1" +} + +test_expect_success 'setup shallow clone' ' + commit 1 && + commit 2 && + commit 3 && + commit 4 && + commit 5 && + commit 6 && + commit 7 && + git clone --no-local --depth=5 .git shallow && + git config --global transfer.fsckObjects true +' + +test_expect_success 'clone http repository' ' + git clone --bare --no-local shallow "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" && + git clone $HTTPD_URL/smart/repo.git clone && + ( + cd clone && + git fsck && + git log --format=%s origin/master >actual && + cat <expect && +7 +6 +5 +4 +3 +EOF + test_cmp expect actual + ) +' + +# 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 15) + 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