Re: [PATCH 1/2] t5411: remove file after use to prevent overwriting
From: Johannes Sixt <hidden>
Date: 2021-01-18 18:23:07
Am 18.01.21 um 14:30 schrieb Jiang Xin:
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 hunk ↗ jump to hunk
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. 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?
cat >expect <<-EOF && To <URL/of/upstream.git> ! [rejected] main -> main (non-fast-forward)
-- Hannes