Skip to content

Commit 2de3478

Browse files
gitstergitster
committed
Merge branch 'nd/http-fetch-shallow-fix'
Attempting to deepen a shallow repository by fetching over smart HTTP transport failed in the protocol exchange, when no-done extension was used. The fetching side waited for the list of shallow boundary commits after the sending end stopped talking to it. * nd/http-fetch-shallow-fix: t5537: move http tests out to t5539 fetch-pack: fix deepen shallow over smart http with no-done cap protocol-capabilities.txt: document no-done protocol-capabilities.txt: refer multi_ack_detailed back to pack-protocol.txt pack-protocol.txt: clarify 'obj-id' in the last ACK after 'done' test: rename http fetch and push test files
2 parents 0f9e62e + 0232852 commit 2de3478

File tree

9 files changed

+104
-29
lines changed

9 files changed

+104
-29
lines changed

‎Documentation/technical/pack-protocol.txt‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -338,7 +338,8 @@ during a prior round. This helps to ensure that at least one common
338338
ancestor is found before we give up entirely.
339339

340340
Once the 'done' line is read from the client, the server will either
341-
send a final 'ACK obj-id' or it will send a 'NAK'. The server only sends
341+
send a final 'ACK obj-id' or it will send a 'NAK'. 'obj-id' is the object
342+
name of the last commit determined to be common. The server only sends
342343
ACK after 'done' if there is at least one common base and multi_ack or
343344
multi_ack_detailed is enabled. The server always sends NAK after 'done'
344345
if there is no common base found.

‎Documentation/technical/protocol-capabilities.txt‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,24 @@ ends.
6969
Without multi_ack the client would have sent that c-b-a chain anyway,
7070
interleaved with S-R-Q.
7171

72+
multi_ack_detailed
73+
------------------
74+
This is an extension of multi_ack that permits client to better
75+
understand the server's in-memory state. See pack-protocol.txt,
76+
section "Packfile Negotiation" for more information.
77+
78+
no-done
79+
-------
80+
This capability should only be used with the smart HTTP protocol. If
81+
multi_ack_detailed and no-done are both present, then the sender is
82+
free to immediately send a pack following its first "ACK obj-id ready"
83+
message.
84+
85+
Without no-done in the smart HTTP protocol, the server session would
86+
end and the client has to make another trip to send "done" before
87+
the server can send the pack. no-done removes the last round and
88+
thus slightly reduces latency.
89+
7290
thin-pack
7391
---------
7492

‎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: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -173,31 +173,4 @@ EOF
173173
)
174174
'
175175

176-
if test -n "$NO_CURL" -o -z "$GIT_TEST_HTTPD"; then
177-
say 'skipping remaining tests, git built without http support'
178-
test_done
179-
fi
180-
181-
. "$TEST_DIRECTORY"/lib-httpd.sh
182-
start_httpd
183-
184-
test_expect_success 'clone http repository' '
185-
git clone --bare --no-local shallow "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
186-
git clone $HTTPD_URL/smart/repo.git clone &&
187-
(
188-
cd clone &&
189-
git fsck &&
190-
git log --format=%s origin/master >actual &&
191-
cat <<EOF >expect &&
192-
7
193-
6
194-
5
195-
4
196-
3
197-
EOF
198-
test_cmp expect actual
199-
)
200-
'
201-
202-
stop_httpd
203176
test_done

‎t/t5539-fetch-http-shallow.sh‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
#!/bin/sh
2+
3+
test_description='fetch/clone from a shallow clone over http'
4+
5+
. ./test-lib.sh
6+
7+
if test -n "$NO_CURL"; then
8+
skip_all='skipping test, git built without http support'
9+
test_done
10+
fi
11+
12+
. "$TEST_DIRECTORY"/lib-httpd.sh
13+
start_httpd
14+
15+
commit() {
16+
echo "$1" >tracked &&
17+
git add tracked &&
18+
git commit -m "$1"
19+
}
20+
21+
test_expect_success 'setup shallow clone' '
22+
commit 1 &&
23+
commit 2 &&
24+
commit 3 &&
25+
commit 4 &&
26+
commit 5 &&
27+
commit 6 &&
28+
commit 7 &&
29+
git clone --no-local --depth=5 .git shallow &&
30+
git config --global transfer.fsckObjects true
31+
'
32+
33+
test_expect_success 'clone http repository' '
34+
git clone --bare --no-local shallow "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
35+
git clone $HTTPD_URL/smart/repo.git clone &&
36+
(
37+
cd clone &&
38+
git fsck &&
39+
git log --format=%s origin/master >actual &&
40+
cat <<EOF >expect &&
41+
7
42+
6
43+
5
44+
4
45+
3
46+
EOF
47+
test_cmp expect actual
48+
)
49+
'
50+
51+
# This test is tricky. We need large enough "have"s that fetch-pack
52+
# will put pkt-flush in between. Then we need a "have" the server
53+
# does not have, it'll send "ACK %s ready"
54+
test_expect_success 'no shallow lines after receiving ACK ready' '
55+
(
56+
cd shallow &&
57+
for i in $(test_seq 15)
58+
do
59+
git checkout --orphan unrelated$i &&
60+
test_commit unrelated$i &&
61+
git push -q "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" \
62+
refs/heads/unrelated$i:refs/heads/unrelated$i &&
63+
git push -q ../clone/.git \
64+
refs/heads/unrelated$i:refs/heads/unrelated$i ||
65+
exit 1
66+
done &&
67+
git checkout master &&
68+
test_commit new &&
69+
git push "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" master
70+
) &&
71+
(
72+
cd clone &&
73+
git checkout --orphan newnew &&
74+
test_commit new-too &&
75+
GIT_TRACE_PACKET="$TRASH_DIRECTORY/trace" git fetch --depth=2 &&
76+
grep "fetch-pack< ACK .* ready" ../trace &&
77+
! grep "fetch-pack> done" ../trace
78+
)
79+
'
80+
81+
stop_httpd
82+
test_done

t/t5540-http-push.sh renamed to t/t5540-http-push-webdav.sh

File renamed without changes.

t/t5541-http-push.sh renamed to t/t5541-http-push-smart.sh

File renamed without changes.

t/t5550-http-fetch.sh renamed to t/t5550-http-fetch-dumb.sh

File renamed without changes.

t/t5551-http-fetch.sh renamed to t/t5551-http-fetch-smart.sh

File renamed without changes.

0 commit comments

Comments
 (0)