SZEDER Gábor <szeder.dev@xxxxxxxxx> 于2021年1月20日周三 下午8:29写道: > > On Mon, Jan 18, 2021 at 04:24:11PM +0800, Jiang Xin wrote: > > SZEDER Gábor <szeder.dev@xxxxxxxxx> 于2021年1月18日周一 上午6:21写道: > > > > > > > > > This patch added a whole lot of test cases, and two of them '86 - > > > proc-receive: not support push options (builtin protocol)' and '95 - > > > proc-receive: not support push options (builtin protocol/porcelain)' > > > are prone to rare failures. > > > > > > On Thu, Aug 27, 2020 at 11:45:44AM -0400, Jiang Xin wrote: > > > > diff --git a/t/t5411/test-0026-push-options.sh b/t/t5411/test-0026-push-options.sh > > > > new file mode 100644 > > > > index 0000000000..d0c4da8b23 > > > > --- /dev/null > > > > +++ b/t/t5411/test-0026-push-options.sh > > > > > > > +# Refs of upstream : master(A) > > > > +# Refs of workbench: master(A) tags/v123 > > > > +# git push -o ... : refs/for/master/topic > > > > +test_expect_success "proc-receive: not support push options ($PROTOCOL)" ' > > > > + test_must_fail git -C workbench push \ > > > > + -o issue=123 \ > > > > + -o reviewer=user1 \ > > > > + origin \ > > > > + HEAD:refs/for/master/topic \ > > > > + >out 2>&1 && > > > > > > Three relevant things are happening here: > > > > > > - 'git push' is executed with its standard output and error > > > redirected to the file 'out'. > > > > > > - 'git push' executes 'git receive-pack' internally, which inherits > > > the open file descriptors, so its output and error goes into that > > > same 'out' file. > > > > > > - 'git push' is expected to fail when it finds out that the other > > > side doesn't support push options, but it does so with a simple > > > die() right away, without waiting for its child 'git receive-pack' > > > process to finish. > > > > > > > + make_user_friendly_and_stable_output <out >actual && > > > > + test_i18ngrep "fatal: the receiving end does not support push options" \ > > > > + actual && > > > > + git -C "$upstream" show-ref >out && > > > > > > Here the shell opens and truncates the file 'out' to write 'git > > > show-ref's output, i.e. it is still the same 'out' file that was used > > > earlier. > > > > > > Consequently, it is possible that 'git receive-pack' is still running, > > > its open file descriptors to 'out' are still valid, and its "fatal: > > > the remote end hung up unexpectedly" error message about the suddenly > > > disappeared 'git push' can partially overwrite the output from 'git > > > show-ref'. > > > > > I think these are the only two tests that can cause this racy > > > behavior: by instrumenting finish_command() I found that in all other > > > tests where 'git push' is expected to fail it errors out gracefully > > > and waits for its 'git receive-pack' child process. > > > > Atomic push may have the same problem. > > I don't think so, because send_pack() doesn't die() when a ref is > rejected in an atomic push, but returns, and lets its caller terminate > in an usual way, including waiting for 'git receive-pack'. I find many places where the client side will die() before closing the service side gracefully: + In `transport_push()`, if fail to push a submodule, will die(). + In `git_transport_push()`, will die() for an unimplemented v2 protocol. + In `send_pack()`, will die() if the server side has an incompatible hash algorithm, or the receiving end does not support --signed push, or the receiving end does not support --atomic push, or the receiving end does not support push options