Re: [PATCH] t5570: forward git-daemon messages in a different way

On 04/14/2012 02:21 PM, Clemens Buchacher wrote:

On Sat, Apr 14, 2012 at 02:13:58PM +0200, Clemens Buchacher wrote:

quoted

In order to better understand the problem on your side, can you execute
this script and tell me what it does for you?

Oops, this is what I really wanted:

#!/bin/sh

mkfifo fd
yes>fd&
pid=$!
{
	read line
	echo $line
	cat<fd&
}<fd
sleep 1
kill $pid
wait $pid
rm -f fd

Hi,
many thanks for looking into this. I'm sorry I didn't reply sooner,
but I was away for the weekend.

And as a consequence, t5570 tests fail for you? I cannot reproduce with
bash 4.2.24(2). Which git version are you seeing this with?

Yes. Example test output is:

----(on master 146fe8ce2)------------------------------------------------------------------
$ (cd t && GIT_TEST_GIT_DAEMON=t ./t5570*sh)
ok 1 - setup repository
ok 2 - create git-accessible bare repository
not ok - 3 clone git repository
#
#               git clone "$GIT_DAEMON_URL/repo.git" clone &&
#               test_cmp file clone/file
#
not ok - 4 fetch changes via git protocol
#
#               echo content >>file &&
#               git commit -a -m two &&
#               git push public &&
#               (cd clone && git pull) &&
#               test_cmp file clone/file
#
not ok 5 - remote detects correct HEAD # TODO known breakage
ok 6 - prepare pack objects
ok 7 - fetch notices corrupt pack
ok 8 - fetch notices corrupt idx
not ok - 9 clone non-existent
#       test_remote_error    clone nowhere.git 'access denied or repository not exported'
not ok - 10 push disabled
#       test_remote_error    push  repo.git    'access denied or repository not exported'
not ok - 11 read access denied
#       test_remote_error -x fetch repo.git    'access denied or repository not exported'
not ok - 12 not exported
#       test_remote_error -n fetch repo.git    'access denied or repository not exported'
./t5570-git-daemon.sh: 59: kill: No such process

error: git daemon exited with status: 141
-----------------------------------------------------------------------------------------

OK, I run your test scripts and found the problem (test.sh is the first
version, and test2.sh is the second version with 'cat' inside {}). Yikes!
I have /bin/sh symlinked to dash, and dash behaves differently:

% bash -x test2.sh | wc -l
+ mkfifo fd
+ pid=10685
+ yes
+ read line
+ echo y
+ sleep 1
+ cat
+ kill 10685
+ wait 10685
test2.sh: line 13: 10685 Terminated              yes > fd
+ rm -f fd
45400064

% dash -x test2.sh | wc -l
+ mkfifo fd
+ pid=10738
+ read line
+ yes
+ echo y
+ sleep 1
+ kill 10738
test2.sh: 12: kill: No such process

+ wait 10738
+ rm -f fd
^C

It hangs at the end until killed with ^C. This seem to happen fairly reliably
(nineteen times out of twenty or so). This is with dash 0.5.7-3 and 0.5.5.1-7.4
from debian. With bash test2.sh seems to always run successfully.

I also run test.sh for comparison, and dash runs test.sh successfully
every once in a while, and test.sh always fails with bash.

So my patch was totally bogus, it was just probably changing the timing.

Now your patches (on top of next):
'git-daemon wrapper to wait until daemon is ready' fixes the problem, thanks!

(I now see that they are both in pu: pu runs fine too.)

Thanks,
Zbyszek