Skip to content

Commit ff62eca

Browse files
pcloudsgitster
authored andcommitted
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 <[email protected]> Signed-off-by: Nguyễn Thái Ngọc Duy <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent c9cd60f commit ff62eca

File tree

2 files changed

+32
-1
lines changed

2 files changed

+32
-1
lines changed

fetch-pack.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -439,7 +439,8 @@ static int find_common(struct fetch_pack_args *args,
439439
}
440440
strbuf_release(&req_buf);
441441

442-
consume_shallow_list(args, fd[0]);
442+
if (!got_ready || !no_done)
443+
consume_shallow_list(args, fd[0]);
443444
while (flushes || multi_ack) {
444445
int ack = get_ack(fd[0], result_sha1);
445446
if (ack) {

t/t5537-fetch-shallow.sh

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,5 +199,35 @@ EOF
199199
)
200200
'
201201

202+
# This test is tricky. We need large enough "have"s that fetch-pack
203+
# will put pkt-flush in between. Then we need a "have" the server
204+
# does not have, it'll send "ACK %s ready"
205+
test_expect_success 'no shallow lines after receiving ACK ready' '
206+
(
207+
cd shallow &&
208+
for i in $(test_seq 10)
209+
do
210+
git checkout --orphan unrelated$i &&
211+
test_commit unrelated$i &&
212+
git push -q "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" \
213+
refs/heads/unrelated$i:refs/heads/unrelated$i &&
214+
git push -q ../clone/.git \
215+
refs/heads/unrelated$i:refs/heads/unrelated$i ||
216+
exit 1
217+
done &&
218+
git checkout master &&
219+
test_commit new &&
220+
git push "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" master
221+
) &&
222+
(
223+
cd clone &&
224+
git checkout --orphan newnew &&
225+
test_commit new-too &&
226+
GIT_TRACE_PACKET="$TRASH_DIRECTORY/trace" git fetch --depth=2 &&
227+
grep "fetch-pack< ACK .* ready" ../trace &&
228+
! grep "fetch-pack> done" ../trace
229+
)
230+
'
231+
202232
stop_httpd
203233
test_done

0 commit comments

Comments
 (0)