From f24eaf4a21a1cf73c2ff494ba7ea5250c37f1d6e Mon Sep 17 00:00:00 2001 From: Thomas Gummerer Date: Thu, 20 Dec 2018 16:41:50 +0000 Subject: [PATCH 1/2] t5570: drop racy test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit t5570 being racy has been reported twice separately on the mailing list [*1*, *2*]. To make the test race proof, we'd either have to introduce another fifo the test snippet is waiting on, or somehow convincing "cat" to flush (and let us know when it has). Which really implies killing the daemon, and wait()ing on cat to process the EOF and exit. And that makes the tests a lot more expensive if we have to start the daemon for each snippet. As this is a test for a relatively minor fix (according to the author) in 19136be3f8 ("daemon: fix off-by-one in logging extended attributes", 2018-01-24), drop it to avoid this racyness. It doesn't seem worth making the test code much more complex, or slowing down all tests just to keep this one. *1*: 1522783990.964448.1325338528.0D49CC15@webmail.messagingengine.com/ *2*: 9d4e5224-9ff4-f3f8-519d-7b2a6f1ea7cd@web.de Reported-by: Jan Palus Reported-by: Torsten Bögershausen Helped-by: Jeff King Signed-off-by: Thomas Gummerer Signed-off-by: Junio C Hamano --- t/t5570-git-daemon.sh | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/t/t5570-git-daemon.sh b/t/t5570-git-daemon.sh index 7466aad111..58ee787685 100755 --- a/t/t5570-git-daemon.sh +++ b/t/t5570-git-daemon.sh @@ -183,19 +183,6 @@ test_expect_success 'hostname cannot break out of directory' ' git ls-remote "$GIT_DAEMON_URL/escape.git" ' -test_expect_success 'daemon log records all attributes' ' - cat >expect <<-\EOF && - Extended attribute "host": localhost - Extended attribute "protocol": version=1 - EOF - >daemon.log && - GIT_OVERRIDE_VIRTUAL_HOST=localhost \ - git -c protocol.version=1 \ - ls-remote "$GIT_DAEMON_URL/interp.git" && - grep -i extended.attribute daemon.log | cut -d" " -f2- >actual && - test_cmp expect actual -' - test_expect_success FAKENC 'hostname interpolation works after LF-stripping' ' { printf "git-upload-pack /interp.git\n\0host=localhost" | packetize From 3c78e97d5d553aa880cdaed2b51610595162dac4 Mon Sep 17 00:00:00 2001 From: Thomas Gummerer Date: Sun, 6 Jan 2019 17:53:10 +0000 Subject: [PATCH 2/2] Revert "t/lib-git-daemon: record daemon log" This reverts commit 314a73d658 (t/lib-git-daemon: record daemon log, 2018-01-25), which let tests use the output of git-daemon. The previous commit removed the last user of deamon.log in the tests, there's no good way to make checking for output in the log race-proof. Revert this commit as well, to make sure others are not tempted to use daemon.log in tests in the future, which would lead to racy tests. The original commit had one change that still makes sense, namely switching read/echo for "read -r" and "printf", which relays the data more faithfully. Don't revert that piece here, as it is still a useful change. Suggested-by: Jeff King Signed-off-by: Thomas Gummerer Signed-off-by: Junio C Hamano --- t/lib-git-daemon.sh | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/t/lib-git-daemon.sh b/t/lib-git-daemon.sh index f98de95c15..fd41229a8f 100644 --- a/t/lib-git-daemon.sh +++ b/t/lib-git-daemon.sh @@ -54,19 +54,11 @@ start_git_daemon() { "$@" "$GIT_DAEMON_DOCUMENT_ROOT_PATH" \ >&3 2>git_daemon_output & GIT_DAEMON_PID=$! - >daemon.log { read -r line <&7 - printf "%s\n" "$line" - printf >&4 "%s\n" "$line" - ( - while read -r line <&7 - do - printf "%s\n" "$line" - printf >&4 "%s\n" "$line" - done - ) & - } 7>"$TRASH_DIRECTORY/daemon.log" && + printf "%s\n" "$line" >&4 + cat <&7 >&4 & + } 7