Download raw body.
fix serving a branch of unrelated history with gotd
Fix a case where gotd started sending pack progress output in response
to the client sending 'done', without any ACK or NAK in-between.
I saw this happen when trying to fetch an unrelated main branch from
gotd, with completely different history and initial root commit,
into an existing repository with a different main branch history.
Add test cases covering got fetch against a git-upload-pack server (is
always passing), and 'got fetch' and 'git fetch' against gotd (was not
passing, i.e. both got and git fail to fetch unless this fix is applied).
ok?
M lib/serve.c | 3+ 2-
M regress/cmdline/fetch.sh | 102+ 0-
M regress/gotd/repo_write.sh | 183+ 0-
3 files changed, 288 insertions(+), 2 deletions(-)
commit - 91c7e219d492ac01fd8fb77133a4d5a3d7f712f2
commit + 66474b7c1b327523f0423ec5f4921d6e9ae6c841
blob - e36084ae30ef3cd5c8813f82e04ba0c23a52a299
blob + 5d19ebd9dd67070635de77c325debade916ca4e7
--- lib/serve.c
+++ lib/serve.c
@@ -803,7 +803,7 @@ serve_read(int infd, int outfd, int gotd_sock, const c
STATE_DONE,
};
enum protostate curstate = STATE_EXPECT_WANT;
- int have_ack = 0, use_sidebands = 0, seen_have = 0;
+ int have_ack = 0, use_sidebands = 0, seen_have = 0, sent_nak = 0;
int packfd = -1;
size_t pack_chunksize;
@@ -864,6 +864,7 @@ serve_read(int infd, int outfd, int gotd_sock, const c
err = send_nak(outfd, chattygot);
if (err)
goto done;
+ sent_nak = 1;
}
if (curstate == STATE_EXPECT_MORE_WANT)
curstate = STATE_EXPECT_HAVE_OR_DONE;
@@ -908,7 +909,7 @@ serve_read(int infd, int outfd, int gotd_sock, const c
}
}
- if (!seen_have) {
+ if (!seen_have || (!have_ack && !sent_nak)) {
err = send_nak(outfd, chattygot);
if (err)
goto done;
blob - aac6a219e2b443b516724c8444a2c326b6bdcf49
blob + ebc64209e918302ec7f357d2a756cb1ea495c017
--- regress/cmdline/fetch.sh
+++ regress/cmdline/fetch.sh
@@ -2153,6 +2153,107 @@ test_fetch_basic_http() {
test_done "$testroot" "$ret"
}
+test_fetch_unrelated_history() {
+ local testroot=`test_init fetch_unrelated_history`
+ local testurl=ssh://127.0.0.1/$testroot
+ local commit_id=`git_show_head $testroot/repo`
+
+ got clone -q $testurl/repo $testroot/repo-clone
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ echo "got clone command failed unexpectedly" >&2
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ got ref -l -r $testroot/repo-clone > $testroot/stdout
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ echo "got ref command failed unexpectedly" >&2
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ echo "HEAD: refs/heads/master" > $testroot/stdout.expected
+ echo "refs/heads/master: $commit_id" >> $testroot/stdout.expected
+ echo "refs/remotes/origin/HEAD: refs/remotes/origin/master" \
+ >> $testroot/stdout.expected
+ echo "refs/remotes/origin/master: $commit_id" \
+ >> $testroot/stdout.expected
+
+ rm -rf $testroot/repo
+ mkdir $testroot/repo
+ git_init $testroot/repo
+ echo 'new history with just one file' > $testroot/repo/alpha
+ git -C $testroot/repo add alpha
+ git_commit $testroot/repo -m "adding the test tree"
+ touch $testroot/repo/.git/git-daemon-export-ok
+ local commit_id2=`git_show_head $testroot/repo`
+
+ got fetch -q -r $testroot/repo-clone > $testroot/stdout \
+ 2> $testroot/stderr
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ echo "got fetch command failed unexpectedly" >&2
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ echo -n > $testroot/stdout.expected
+
+ cmp -s $testroot/stdout $testroot/stdout.expected
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ diff -u $testroot/stdout.expected $testroot/stdout
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ got ref -l -r $testroot/repo > $testroot/stdout
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ echo "got ref command failed unexpectedly" >&2
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ echo "HEAD: refs/heads/master" > $testroot/stdout.expected
+ echo "refs/heads/master: $commit_id2" >> $testroot/stdout.expected
+
+ cmp -s $testroot/stdout $testroot/stdout.expected
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ diff -u $testroot/stdout.expected $testroot/stdout
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ got ref -l -r $testroot/repo-clone > $testroot/stdout
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ echo "got ref command failed unexpectedly" >&2
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ echo "HEAD: refs/heads/master" > $testroot/stdout.expected
+ echo "refs/heads/master: $commit_id" >> $testroot/stdout.expected
+ echo "refs/remotes/origin/HEAD: refs/remotes/origin/master" \
+ >> $testroot/stdout.expected
+ echo "refs/remotes/origin/master: $commit_id2" \
+ >> $testroot/stdout.expected
+
+ cmp -s $testroot/stdout $testroot/stdout.expected
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ diff -u $testroot/stdout.expected $testroot/stdout
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+ test_done "$testroot" "$ret"
+}
+
+
test_parseargs "$@"
run_test test_fetch_basic no-sha256
run_test test_fetch_list no-sha256
@@ -2172,3 +2273,4 @@ run_test test_fetch_delete_remote_refs no-sha256
run_test test_fetch_honor_wt_conf_bflag no-sha256
run_test test_fetch_from_out_of_date_remote no-sha256
run_test test_fetch_basic_http no-sha256
+run_test test_fetch_unrelated_history no-sha256
blob - b24dd5fd78d5c9570776c47220b98aa18dc4e3f1
blob + 79fd816a3d8d965fa50236f65eb132af7ec334b3
--- regress/gotd/repo_write.sh
+++ regress/gotd/repo_write.sh
@@ -554,9 +554,192 @@ EOF
test_done "$testroot" 0
}
+test_fetch_unrelated_history() {
+ local testroot=`test_init fetch_unrelated_history 1`
+
+ got clone -q ${GOTD_TEST_REPO_URL} $testroot/repo-clone
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ echo "got clone failed unexpectedly" >&2
+ test_done "$testroot" "1"
+ return 1
+ fi
+ local commit_id0=`git_show_head $testroot/repo-clone`
+
+ mkdir $testroot/new-repo
+ git_init $testroot/new-repo
+ git -C $testroot/new-repo symbolic-ref HEAD refs/heads/main
+
+ echo 'new history with just one file' > $testroot/new-repo/alpha
+ git -C $testroot/new-repo add alpha
+ git_commit $testroot/new-repo -m "adding the test tree"
+ for i in 1 2 3; do
+ echo 'line $i' >> $testroot/new-repo/alpha
+ git -C $testroot/new-repo add alpha
+ git_commit $testroot/new-repo -m "more changes"
+ done
+ got tag -r $testroot/new-repo -m "tagging 1.0" 1.0 >/dev/null
+ local commit_id2=`git_show_head $testroot/new-repo`
+ local tag_id=`got ref -r "$testroot/new-repo" -l refs/tags/1.0 | \
+ awk '{print $2}'`
+
+ cp $testroot/repo-clone/got.conf $testroot/new-repo/.git/got.conf
+
+ got send -q -b main -t 1.0 -r $testroot/new-repo -f
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ echo "got send failed unexpectedly" >&2
+ test_done "$testroot" "1"
+ return 1
+ fi
+
+ # Verify that the send operation worked fine.
+ got clone -q ${GOTD_TEST_REPO_URL} $testroot/repo-clone2
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ echo "got fetch failed unexpectedly" >&2
+ test_done "$testroot" "1"
+ return 1
+ fi
+
+ got tree -R -r $testroot/repo-clone2 > $testroot/stdout
+ cat > $testroot/stdout.expected <<EOF
+alpha
+EOF
+ cmp -s $testroot/stdout.expected $testroot/stdout
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ diff -u $testroot/stdout.expected $testroot/stdout
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ # This test checks for server-side behaviour where progress output
+ # gets generated before any ACK or NAK arrive.
+ # To trigger the problem we should not be sending any refs the
+ # server already knows about. So delete the following:
+ got ref -d -r $testroot/repo-clone refs/tags/1.0 > /dev/null
+ got fetch -X -r $testroot/repo-clone origin > /dev/null
+
+ # Rewrite main branch hisotry to avoid sending a HAVE with a
+ # commit ID which the server can find locally.
+ got ref -r $testroot/repo-clone -d refs/heads/main > /dev/null
+ mkdir $testroot/import-tree
+ echo 'parallel universe alpha' > $testroot/import-tree/alpha
+
+ got import -r $testroot/repo-clone -b main -m init \
+ $testroot/import-tree > /dev/null
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ echo "got import failed unexpectedly" >&2
+ test_done "$testroot" 1
+ return 1
+ fi
+ local commit_id=`git_show_head $testroot/repo-clone`
+
+ cp -r $testroot/repo-clone $testroot/repo-clone3
+ cp $testroot/repo-clone/config $testroot/repo-clone3/config
+
+ # verify that unrelated history can be fetched into the initial clone.
+ # This used to fail with:
+ # got-fetch-pack: readpkt: 48: [0x02]1 commits colored, \
+ # 0 objects found, deltify 0%[0x0d]
+ # got-fetch-pack: unexpected message from server
+ got fetch -q -r $testroot/repo-clone > $testroot/stdout \
+ 2> $testroot/stderr
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ echo "got fetch command failed unexpectedly" >&2
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ echo -n > $testroot/stdout.expected
+
+ cmp -s $testroot/stdout $testroot/stdout.expected
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ diff -u $testroot/stdout.expected $testroot/stdout
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ got ref -l -r $testroot/repo-clone > $testroot/stdout
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ echo "got ref command failed unexpectedly" >&2
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ cat > $testroot/stdout.expected <<EOF
+HEAD: refs/heads/main
+refs/heads/main: $commit_id
+refs/remotes/origin/HEAD: refs/remotes/origin/main
+refs/remotes/origin/main: $commit_id2
+refs/tags/1.0: $tag_id
+EOF
+ cmp -s $testroot/stdout $testroot/stdout.expected
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ diff -u $testroot/stdout.expected $testroot/stdout
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ # Now try the same fetch operation against gotd with Git.
+ # This used to fail with:
+ # fatal: git fetch-pack: expected ACK/NAK, got '?1 commits colored, \
+ # 0 objects found, deltify 0%?'
+ git -C $testroot/repo-clone3 fetch > $testroot/stdout \
+ 2> $testroot/stderr
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ echo "git fetch command failed unexpectedly" >&2
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ echo -n > $testroot/stdout.expected
+
+ cmp -s $testroot/stdout $testroot/stdout.expected
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ diff -u $testroot/stdout.expected $testroot/stdout
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ got ref -l -r $testroot/repo-clone3 > $testroot/stdout
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ echo "got ref command failed unexpectedly" >&2
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ cat > $testroot/stdout.expected <<EOF
+HEAD: refs/heads/main
+refs/heads/main: $commit_id
+refs/remotes/origin/HEAD: refs/remotes/origin/main
+refs/remotes/origin/main: $commit_id2
+refs/tags/1.0: $tag_id
+EOF
+ cmp -s $testroot/stdout $testroot/stdout.expected
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ diff -u $testroot/stdout.expected $testroot/stdout
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ test_done "$testroot" "$ret"
+}
+
test_parseargs "$@"
run_test test_send_basic
run_test test_fetch_more_history
run_test test_send_new_empty_branch
run_test test_delete_branch
run_test test_rewind_branch
+run_test test_fetch_unrelated_history
fix serving a branch of unrelated history with gotd