Thread (77 messages) 77 messages, 5 authors, 2021-01-21

Re: [PATCH 1/2] t5411: remove file after use to prevent overwriting

From: Jiang Xin <hidden>
Date: 2021-01-19 00:49:55

Johannes Sixt [off-list ref] 于2021年1月19日周二 上午2:21写道:
Am 18.01.21 um 14:30 schrieb Jiang Xin:
quoted
From: Jiang Xin <redacted>

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...
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help