Re: [PATCH 1/2] t5411: remove file after use to prevent overwriting
- From
Jiang Xin <worldhello.net@gmail.com>
- Date
- Jan 19, 2021, 00:48 UTC
- Message-ID
- <CANYiYbF4gv0x78SX=TbQKP+GwUVCCRpNob+riRs5wPiFKcsSsQ@mail.gmail.com>
- In-Reply-To
- <e1c9c9cf-4498-6861-bb39-53ceeb9c0fe7@kdbg.org>
Johannes Sixt <j6t@kdbg.org> 于2021年1月19日周二 上午2:21写道:
Show 54 quoted lines
>
> Am 18.01.21 um 14:30 schrieb Jiang Xin:
> > From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
> >
> > SZEDER reported that t5411 failed in Travis CI's s390x environment a
> > couple of times, and could be reproduced with '--stress' test on this
> > specific environment. The test failure messages might look like this:
> >
> > + test_cmp expect actual
> > --- expect 2021-01-17 21:55:23.430750004 +0000
> > +++ actual 2021-01-17 21:55:23.430750004 +0000
> > @@ -1 +1 @@
> > -<COMMIT-A> refs/heads/main
> > +<COMMIT-A> refs/heads/maifatal: the remote end hung up unexpectedly
> > error: last command exited with $?=1
> > not ok 86 - proc-receive: not support push options (builtin protocol)
> >
> > The file 'actual' is filtered from the file 'out' which contains result
> > of 'git show-ref' command. Due to the error messages from other process
> > is written into the file 'out' accidentally, t5411 failed. SZEDER finds
> > the root cause of this issue:
> >
> > - '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' ends without waiting for the close of 'git-receive-pack'
> > for some cases, and the file 'out' is reused for test of
> > 'git show-ref' afterwards.
> >
> > - A mixture of the output of 'git show-ref' abd 'git receive-pack'
> > leads to this issue.
> >
> > To resolve this issue, we can remove the file 'out' after use. The
> > long-running 'git receive-pack' will not redirect its output to the new
> > created 'out' file which has a different file descriptor.
>
> On Windows, removing an open file is not possible and this...
>
> > diff --git a/t/t5411/test-0000-standard-git-push.sh b/t/t5411/test-0000-standard-git-push.sh
> > index 47b058af7e..694d8e8dc2 100644
> > --- a/t/t5411/test-0000-standard-git-push.sh
> > +++ b/t/t5411/test-0000-standard-git-push.sh
> > @@ -40,6 +40,8 @@ test_expect_success "git-push --atomic ($PROTOCOL)" '
> > -e "/^To / { p; }" \
> > -e "/^ ! / { p; }" \
> > <out >actual &&
> > + # Prevent accidential changes by the internal "receive-pack" process.
> > + rm out &&
>
> ... would fail.Thank you for reminding. It seems that the only way is to use a different output file name.
> That said, your next patch removes a lot of uses of the 'out' file > against which this 'rm out' should protect. Doesn't this make this patch > unnecessary?
The other test cases also write to the same "out" file, so the issue still needs to be fixed.
-- Jiang Xin