From f65003b4c4abea49a00cc9f3d87b9f37db6b48f1 Mon Sep 17 00:00:00 2001 From: Jiang Xin Date: Wed, 11 Nov 2020 19:32:01 +0800 Subject: receive-pack: gently write messages to proc-receive Johannes found a flaky hang in `t5411/test-0013-bad-protocol.sh` in the osx-clang job of the CI/PR builds, and ran into an issue when using the `--stress` option with the following error messages: fatal: unable to write flush packet: Broken pipe send-pack: unexpected disconnect while reading sideband packet fatal: the remote end hung up unexpectedly In this test case, the "proc-receive" hook sends an error message and dies earlier. While "receive-pack" on the other side of the pipe should forward the error message of the "proc-receive" hook to the client side, but it fails to do so. This is because "receive-pack" uses `packet_write_fmt()` and `packet_flush()` to write pkt-line message to "proc-receive" hook, and these functions die immediately when pipe is broken. Using "gently" forms for these functions will get more predicable output. Add more "--die-*" options to test helper to test different stages of the protocol between "receive-pack" and "proc-receive" hook. Reported-by: Johannes Schindelin Suggested-by: Jeff King Signed-off-by: Jiang Xin Signed-off-by: Junio C Hamano --- t/t5411/test-0013-bad-protocol.sh | 138 +++++++++++++++++++-- t/t5411/test-0014-bad-protocol--porcelain.sh | 173 ++++++++++++++++++++++++++- 2 files changed, 297 insertions(+), 14 deletions(-) (limited to 't/t5411') diff --git a/t/t5411/test-0013-bad-protocol.sh b/t/t5411/test-0013-bad-protocol.sh index 854c3e884a..b9be12be77 100644 --- a/t/t5411/test-0013-bad-protocol.sh +++ b/t/t5411/test-0013-bad-protocol.sh @@ -16,7 +16,8 @@ test_expect_success "proc-receive: bad protocol (unknown version, $PROTOCOL)" ' # Check status report for git-push sed -n \ - -e "/^To / { p; n; p; }" \ + -e "/^To / { p; }" \ + -e "/^ ! / { p; }" \ actual-report && cat >expect <<-EOF && To @@ -41,32 +42,98 @@ test_expect_success "proc-receive: bad protocol (unknown version, $PROTOCOL)" ' test_cmp expect actual ' -test_expect_success "setup proc-receive hook (hook --die-version, $PROTOCOL)" ' +test_expect_success "setup proc-receive hook (hook --die-read-version, $PROTOCOL)" ' write_script "$upstream/hooks/proc-receive" <<-EOF printf >&2 "# proc-receive hook\n" - test-tool proc-receive -v --die-version + test-tool proc-receive -v --die-read-version EOF ' # Refs of upstream : main(A) # Refs of workbench: main(A) tags/v123 # git push : refs/for/main/topic(A) -test_expect_success "proc-receive: bad protocol (hook --die-version, $PROTOCOL)" ' +test_expect_success "proc-receive: bad protocol (hook --die-read-version, $PROTOCOL)" ' test_must_fail git -C workbench push origin \ HEAD:refs/for/main/topic \ >out 2>&1 && + filter_out_user_friendly_and_stable_output \ + -e "/^To / { p; }" \ + -e "/^ ! / { p; }" \ + actual && + cat >expect <<-EOF && + To + ! [remote rejected] HEAD -> refs/for/main/topic (fail to run proc-receive hook) + EOF + test_cmp expect actual && + grep "remote: fatal: die with the --die-read-version option" out && + grep "remote: error: fail to negotiate version with proc-receive hook" out && + + git -C "$upstream" show-ref >out && make_user_friendly_and_stable_output actual && + cat >expect <<-EOF && + refs/heads/main + EOF + test_cmp expect actual +' + +test_expect_success "setup proc-receive hook (hook --die-write-version, $PROTOCOL)" ' + write_script "$upstream/hooks/proc-receive" <<-EOF + printf >&2 "# proc-receive hook\n" + test-tool proc-receive -v --die-write-version + EOF +' +# Refs of upstream : main(A) +# Refs of workbench: main(A) tags/v123 +# git push : refs/for/main/topic(A) +test_expect_success "proc-receive: bad protocol (hook --die-write-version, $PROTOCOL)" ' + test_must_fail git -C workbench push origin \ + HEAD:refs/for/main/topic \ + >out 2>&1 && + filter_out_user_friendly_and_stable_output \ + -e "/^To / { p; }" \ + -e "/^ ! / { p; }" \ + actual && + cat >expect <<-EOF && + To + ! [remote rejected] HEAD -> refs/for/main/topic (fail to run proc-receive hook) + EOF + test_cmp expect actual && + grep "remote: fatal: die with the --die-write-version option" out && + grep "remote: error: fail to negotiate version with proc-receive hook" out && + + git -C "$upstream" show-ref >out && + make_user_friendly_and_stable_output actual && + cat >expect <<-EOF && + refs/heads/main + EOF + test_cmp expect actual +' + +test_expect_success "setup proc-receive hook (hook --die-read-commands, $PROTOCOL)" ' + write_script "$upstream/hooks/proc-receive" <<-EOF + printf >&2 "# proc-receive hook\n" + test-tool proc-receive -v --die-read-commands + EOF +' + +# Refs of upstream : main(A) +# Refs of workbench: main(A) tags/v123 +# git push : refs/for/main/topic(A) +test_expect_success "proc-receive: bad protocol (hook --die-read-commands, $PROTOCOL)" ' + test_must_fail git -C workbench push origin \ + HEAD:refs/for/main/topic \ + >out 2>&1 && + filter_out_user_friendly_and_stable_output \ + -e "/^To / { p; }" \ + -e "/^ ! / { p; }" \ + actual && cat >expect <<-EOF && - remote: # pre-receive hook - remote: pre-receive< refs/for/main/topic - remote: # proc-receive hook - remote: fatal: bad protocol version: 1 - remote: error: proc-receive version "0" is not supported To ! [remote rejected] HEAD -> refs/for/main/topic (fail to run proc-receive hook) EOF test_cmp expect actual && + grep "remote: fatal: die with the --die-read-commands option" out && git -C "$upstream" show-ref >out && make_user_friendly_and_stable_output actual && @@ -76,23 +143,65 @@ test_expect_success "proc-receive: bad protocol (hook --die-version, $PROTOCOL)" test_cmp expect actual ' -test_expect_success "setup proc-receive hook (hook --die-readline, $PROTOCOL)" ' +test_expect_success "setup proc-receive hook (hook --die-read-push-options, $PROTOCOL)" ' write_script "$upstream/hooks/proc-receive" <<-EOF printf >&2 "# proc-receive hook\n" - test-tool proc-receive -v --die-readline + test-tool proc-receive -v --die-read-push-options EOF ' # Refs of upstream : main(A) # Refs of workbench: main(A) tags/v123 # git push : refs/for/main/topic(A) -test_expect_success "proc-receive: bad protocol (hook --die-readline, $PROTOCOL)" ' +test_expect_success "proc-receive: bad protocol (hook --die-read-push-options, $PROTOCOL)" ' + git -C "$upstream" config receive.advertisePushOptions true && test_must_fail git -C workbench push origin \ + -o reviewers=user1,user2 \ HEAD:refs/for/main/topic \ >out 2>&1 && + filter_out_user_friendly_and_stable_output \ + -e "/^To / { p; }" \ + -e "/^ ! / { p; }" \ + actual && + cat >expect <<-EOF && + To + ! [remote rejected] HEAD -> refs/for/main/topic (fail to run proc-receive hook) + EOF + test_cmp expect actual && + grep "remote: fatal: die with the --die-read-push-options option" out && + + git -C "$upstream" show-ref >out && make_user_friendly_and_stable_output actual && + cat >expect <<-EOF && + refs/heads/main + EOF + test_cmp expect actual +' + +test_expect_success "setup proc-receive hook (hook --die-write-report, $PROTOCOL)" ' + write_script "$upstream/hooks/proc-receive" <<-EOF + printf >&2 "# proc-receive hook\n" + test-tool proc-receive -v --die-write-report + EOF +' - grep "remote: fatal: protocol error: expected \"old new ref\", got \" refs/for/main/topic\"" actual && +# Refs of upstream : main(A) +# Refs of workbench: main(A) tags/v123 +# git push : refs/for/main/topic(A) +test_expect_success "proc-receive: bad protocol (hook --die-write-report, $PROTOCOL)" ' + test_must_fail git -C workbench push origin \ + HEAD:refs/for/main/topic \ + >out 2>&1 && + filter_out_user_friendly_and_stable_output \ + -e "/^To / { p; }" \ + -e "/^ ! / { p; }" \ + actual && + cat >expect <<-EOF && + To + ! [remote rejected] HEAD -> refs/for/main/topic (fail to run proc-receive hook) + EOF + test_cmp expect actual && + grep "remote: fatal: die with the --die-write-report option" out && git -C "$upstream" show-ref >out && make_user_friendly_and_stable_output actual && @@ -130,6 +239,7 @@ test_expect_success "proc-receive: bad protocol (no report, $PROTOCOL)" ' ! [remote rejected] HEAD -> refs/for/main/topic (proc-receive failed to report status) EOF test_cmp expect actual && + git -C "$upstream" show-ref >out && make_user_friendly_and_stable_output actual && cat >expect <<-EOF && @@ -173,6 +283,7 @@ test_expect_success "proc-receive: bad protocol (no ref, $PROTOCOL)" ' ! [remote rejected] HEAD -> refs/for/main/topic (proc-receive failed to report status) EOF test_cmp expect actual && + git -C "$upstream" show-ref >out && make_user_friendly_and_stable_output actual && cat >expect <<-EOF && @@ -208,6 +319,7 @@ test_expect_success "proc-receive: bad protocol (unknown status, $PROTOCOL)" ' ! [remote rejected] HEAD -> refs/for/main/topic (proc-receive failed to report status) EOF test_cmp expect actual && + git -C "$upstream" show-ref >out && make_user_friendly_and_stable_output actual && cat >expect <<-EOF && diff --git a/t/t5411/test-0014-bad-protocol--porcelain.sh b/t/t5411/test-0014-bad-protocol--porcelain.sh index 88c56311da..fdb4569109 100644 --- a/t/t5411/test-0014-bad-protocol--porcelain.sh +++ b/t/t5411/test-0014-bad-protocol--porcelain.sh @@ -42,6 +42,175 @@ test_expect_success "proc-receive: bad protocol (unknown version, $PROTOCOL/porc test_cmp expect actual ' +test_expect_success "setup proc-receive hook (hook --die-read-version, $PROTOCOL/porcelain)" ' + write_script "$upstream/hooks/proc-receive" <<-EOF + printf >&2 "# proc-receive hook\n" + test-tool proc-receive -v --die-read-version + EOF +' + +# Refs of upstream : main(A) +# Refs of workbench: main(A) tags/v123 +# git push : refs/for/main/topic(A) +test_expect_success "proc-receive: bad protocol (hook --die-read-version, $PROTOCOL/porcelain)" ' + test_must_fail git -C workbench push --porcelain origin \ + HEAD:refs/for/main/topic \ + >out 2>&1 && + filter_out_user_friendly_and_stable_output \ + -e "/^To / { p; n; p; n; p; }" \ + actual && + cat >expect <<-EOF && + To + ! HEAD:refs/for/main/topic [remote rejected] (fail to run proc-receive hook) + Done + EOF + test_cmp expect actual && + grep "remote: fatal: die with the --die-read-version option" out && + grep "remote: error: fail to negotiate version with proc-receive hook" out && + + git -C "$upstream" show-ref >out && + make_user_friendly_and_stable_output actual && + cat >expect <<-EOF && + refs/heads/main + EOF + test_cmp expect actual +' + +test_expect_success "setup proc-receive hook (hook --die-write-version, $PROTOCOL/porcelain)" ' + write_script "$upstream/hooks/proc-receive" <<-EOF + printf >&2 "# proc-receive hook\n" + test-tool proc-receive -v --die-write-version + EOF +' + +# Refs of upstream : main(A) +# Refs of workbench: main(A) tags/v123 +# git push : refs/for/main/topic(A) +test_expect_success "proc-receive: bad protocol (hook --die-write-version, $PROTOCOL/porcelain)" ' + test_must_fail git -C workbench push --porcelain origin \ + HEAD:refs/for/main/topic \ + >out 2>&1 && + filter_out_user_friendly_and_stable_output \ + -e "/^To / { p; n; p; n; p; }" \ + actual && + cat >expect <<-EOF && + To + ! HEAD:refs/for/main/topic [remote rejected] (fail to run proc-receive hook) + Done + EOF + test_cmp expect actual && + grep "remote: fatal: die with the --die-write-version option" out && + grep "remote: error: fail to negotiate version with proc-receive hook" out && + + git -C "$upstream" show-ref >out && + make_user_friendly_and_stable_output actual && + cat >expect <<-EOF && + refs/heads/main + EOF + test_cmp expect actual +' + +test_expect_success "setup proc-receive hook (hook --die-read-commands, $PROTOCOL/porcelain)" ' + write_script "$upstream/hooks/proc-receive" <<-EOF + printf >&2 "# proc-receive hook\n" + test-tool proc-receive -v --die-read-commands + EOF +' + +# Refs of upstream : main(A) +# Refs of workbench: main(A) tags/v123 +# git push : refs/for/main/topic(A) +test_expect_success "proc-receive: bad protocol (hook --die-read-commands, $PROTOCOL/porcelain)" ' + test_must_fail git -C workbench push --porcelain origin \ + HEAD:refs/for/main/topic \ + >out 2>&1 && + filter_out_user_friendly_and_stable_output \ + -e "/^To / { p; n; p; n; p; }" \ + actual && + cat >expect <<-EOF && + To + ! HEAD:refs/for/main/topic [remote rejected] (fail to run proc-receive hook) + Done + EOF + test_cmp expect actual && + grep "remote: fatal: die with the --die-read-commands option" out && + + git -C "$upstream" show-ref >out && + make_user_friendly_and_stable_output actual && + cat >expect <<-EOF && + refs/heads/main + EOF + test_cmp expect actual +' + +test_expect_success "setup proc-receive hook (hook --die-read-push-options, $PROTOCOL/porcelain)" ' + write_script "$upstream/hooks/proc-receive" <<-EOF + printf >&2 "# proc-receive hook\n" + test-tool proc-receive -v --die-read-push-options + EOF +' + +# Refs of upstream : main(A) +# Refs of workbench: main(A) tags/v123 +# git push : refs/for/main/topic(A) +test_expect_success "proc-receive: bad protocol (hook --die-read-push-options, $PROTOCOL/porcelain)" ' + git -C "$upstream" config receive.advertisePushOptions true && + test_must_fail git -C workbench push --porcelain origin \ + -o reviewers=user1,user2 \ + HEAD:refs/for/main/topic \ + >out 2>&1 && + filter_out_user_friendly_and_stable_output \ + -e "/^To / { p; n; p; n; p; }" \ + actual && + cat >expect <<-EOF && + To + ! HEAD:refs/for/main/topic [remote rejected] (fail to run proc-receive hook) + Done + EOF + test_cmp expect actual && + grep "remote: fatal: die with the --die-read-push-options option" out && + + git -C "$upstream" show-ref >out && + make_user_friendly_and_stable_output actual && + cat >expect <<-EOF && + refs/heads/main + EOF + test_cmp expect actual +' + +test_expect_success "setup proc-receive hook (hook --die-write-report, $PROTOCOL/porcelain)" ' + write_script "$upstream/hooks/proc-receive" <<-EOF + printf >&2 "# proc-receive hook\n" + test-tool proc-receive -v --die-write-report + EOF +' + +# Refs of upstream : main(A) +# Refs of workbench: main(A) tags/v123 +# git push : refs/for/main/topic(A) +test_expect_success "proc-receive: bad protocol (hook --die-write-report, $PROTOCOL/porcelain)" ' + test_must_fail git -C workbench push --porcelain origin \ + HEAD:refs/for/main/topic \ + >out 2>&1 && + filter_out_user_friendly_and_stable_output \ + -e "/^To / { p; n; p; n; p; }" \ + actual && + cat >expect <<-EOF && + To + ! HEAD:refs/for/main/topic [remote rejected] (fail to run proc-receive hook) + Done + EOF + test_cmp expect actual && + grep "remote: fatal: die with the --die-write-report option" out && + + git -C "$upstream" show-ref >out && + make_user_friendly_and_stable_output actual && + cat >expect <<-EOF && + refs/heads/main + EOF + test_cmp expect actual +' + test_expect_success "setup proc-receive hook (no report, $PROTOCOL/porcelain)" ' write_script "$upstream/hooks/proc-receive" <<-EOF printf >&2 "# proc-receive hook\n" @@ -71,6 +240,7 @@ test_expect_success "proc-receive: bad protocol (no report, $PROTOCOL/porcelain) Done EOF test_cmp expect actual && + git -C "$upstream" show-ref >out && make_user_friendly_and_stable_output actual && cat >expect <<-EOF && @@ -84,7 +254,6 @@ test_expect_success "proc-receive: bad protocol (no report, $PROTOCOL/porcelain) # Refs of workbench: main(A) tags/v123 test_expect_success "cleanup ($PROTOCOL/porcelain)" ' git -C "$upstream" update-ref -d refs/heads/next - ' test_expect_success "setup proc-receive hook (no ref, $PROTOCOL/porcelain)" ' @@ -115,6 +284,7 @@ test_expect_success "proc-receive: bad protocol (no ref, $PROTOCOL/porcelain)" ' Done EOF test_cmp expect actual && + git -C "$upstream" show-ref >out && make_user_friendly_and_stable_output actual && cat >expect <<-EOF && @@ -151,6 +321,7 @@ test_expect_success "proc-receive: bad protocol (unknown status, $PROTOCOL/porce Done EOF test_cmp expect actual && + git -C "$upstream" show-ref >out && make_user_friendly_and_stable_output actual && cat >expect <<-EOF && -- cgit v1.2.3