commit c12ab9ad7f32b9eb712734e776de2f7b0370e643 from: Stefan Sperling via: Thomas Adam date: Tue Feb 21 12:31:55 2023 UTC fix "got fetch" hang against out-of-date remote repositories Do not assume that remote repositories will always have our objects. In Git protocol terms: Do not wait for an ACK from the server before sending the final "done" message. Otherwise servers might be waiting for more have-lines from us in order to find a common ancestor, which will never be sent by us. Problem reported by James Cook who also provided an initial test case ok op@ commit - 47c7ee21b5f269eed3127db9d3ac7c490d1ecf0a commit + c12ab9ad7f32b9eb712734e776de2f7b0370e643 blob - 0b5d861489f9276c73e9cf8da77eb3420f732e24 blob + 58acac3d67d7dfc870a3cb123211735a54f6d1ae --- libexec/got-fetch-pack/got-fetch-pack.c +++ libexec/got-fetch-pack/got-fetch-pack.c @@ -588,6 +588,11 @@ fetch_pack(int fd, int packfd, uint8_t *pack_sha1, nhave++; } + n = strlcpy(buf, "done\n", sizeof(buf)); + err = got_pkt_writepkt(fd, buf, n, chattygot); + if (err) + goto done; + while (nhave > 0 && !acked) { struct got_object_id common_id; @@ -600,8 +605,12 @@ fetch_pack(int fd, int packfd, uint8_t *pack_sha1, goto done; } if (n >= 4 && strncmp(buf, "NAK\n", 4) == 0) { - /* Server has not located our objects yet. */ - continue; + /* + * Server could not find a common ancestor. + * Perhaps it is an out-of-date mirror, or there + * is a repository with unrelated history. + */ + break; } if (n < 4 + SHA1_DIGEST_STRING_LENGTH || strncmp(buf, "ACK ", 4) != 0) { @@ -616,11 +625,6 @@ fetch_pack(int fd, int packfd, uint8_t *pack_sha1, } acked++; } - - n = strlcpy(buf, "done\n", sizeof(buf)); - err = got_pkt_writepkt(fd, buf, n, chattygot); - if (err) - goto done; if (nhave == 0) { err = got_pkt_readpkt(&n, fd, buf, sizeof(buf), chattygot); blob - d24224b5bc7fdac0107e60604c32eb162cf6598d blob + 737b0cb81e316d7acd423ca57c68f090d5f33d9f --- regress/cmdline/fetch.sh +++ regress/cmdline/fetch.sh @@ -1835,7 +1835,97 @@ test_fetch_honor_wt_conf_bflag() { fi test_done "$testroot" "$ret" } + +test_fetch_from_out_of_date_remote() { + local testroot=`test_init fetch_from_out_of_date_remote` + 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 + echo "modified alpha" > $testroot/repo/alpha + git_commit $testroot/repo -m "modified alpha" + local commit_id2=`git_show_head $testroot/repo` + + got clone -q $testurl/repo $testroot/repo-clone2 \ + > $testroot/stdout 2> $testroot/stderr + 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-clone2 > $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 <> $testroot/repo-clone2/got.conf < $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 + + got ref -l -r $testroot/repo-clone2 > $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 <