git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Johannes Sixt <j6t@kdbg.org>
To: Clemens Buchacher <drizzd@aon.at>
Cc: "Jeff King" <peff@peff.net>,
	"Zbigniew Jędrzejewski-Szmek" <zbyszek@in.waw.pl>,
	git@vger.kernel.org, gitster@pobox.com
Subject: [PATCH] t5570: fix forwarding of git-daemon messages via cat
Date: Thu, 26 Apr 2012 23:00:39 +0200	[thread overview]
Message-ID: <4F99B777.4020103@kdbg.org> (raw)
In-Reply-To: <20120426195503.GA29526@ecki>

The shell function that starts git-daemon wants to read the first line of
the daemon's stderr to ensure that it started correctly. Subsequent daemon
errors should be redirected to fd 4 (which is the terminal in verbose mode
or /dev/null in quiet mode). To that end the shell script used 'read' to
get the first line of output, and then 'cat &' to forward everything else
in a background process.

The problem is, that 'cat >&4 &' does not produce any output because the
shell redirects a background process's stdin to /dev/null. To have this
command invocation do anything useful, we have to redirect its stdin
explicitly (which overrides the /dev/null redirection).

The shell function connects the daemon's stderr to its consumers via a
FIFO. We cannot just do this:

   read line <git_daemon_output
   cat <git_daemon_output >&4 &

because after the first redirection the pipe is closed and the daemon
could receive SIGPIPE if it writes at the wrong moment. Therefore, we open
the readable end of the FIFO only once on fd 7 in the shell and dup from
there to the stdin of the two consumers.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
Am 26.04.2012 21:55, schrieb Clemens Buchacher:
> I won't pretend to understand why this works. I have to study this some
> more. But if this is 'correct', then it is obviously preferable to the
> comparatively complicated wrapper.
> 
> We should move the cat <&7 >&4 & and exec 7<&- part in front of the
> output check, otherwise output would be truncated in an error condition.
> This can be tested by passing an invalid argument to git daemon above,
> for example.

How about this?

 t/lib-git-daemon.sh |   22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/t/lib-git-daemon.sh b/t/lib-git-daemon.sh
index ef2d01f..87f0ad8 100644
--- a/t/lib-git-daemon.sh
+++ b/t/lib-git-daemon.sh
@@ -31,19 +31,19 @@ start_git_daemon() {
 		>&3 2>git_daemon_output &
 	GIT_DAEMON_PID=$!
 	{
-		read line
+		read line <&7
 		echo >&4 "$line"
-		cat >&4 &
+		cat <&7 >&4 &
+	} 7<git_daemon_output &&
 
-		# Check expected output
-		if test x"$(expr "$line" : "\[[0-9]*\] \(.*\)")" != x"Ready to rumble"
-		then
-			kill "$GIT_DAEMON_PID"
-			wait "$GIT_DAEMON_PID"
-			trap 'die' EXIT
-			error "git daemon failed to start"
-		fi
-	} <git_daemon_output
+	# Check expected output
+	if test x"$(expr "$line" : "\[[0-9]*\] \(.*\)")" != x"Ready to rumble"
+	then
+		kill "$GIT_DAEMON_PID"
+		wait "$GIT_DAEMON_PID"
+		trap 'die' EXIT
+		error "git daemon failed to start"
+	fi
 }
 
 stop_git_daemon() {
-- 
1.7.10.4.g51807

  reply	other threads:[~2012-04-26 21:00 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-14  8:44 [PATCH] t5570: forward git-daemon messages in a different way Zbigniew Jędrzejewski-Szmek
2012-04-14 12:13 ` Clemens Buchacher
2012-04-14 12:21   ` Clemens Buchacher
2012-04-16 15:43     ` Zbigniew Jędrzejewski-Szmek
2012-04-16 17:09       ` Junio C Hamano
2012-04-16 21:22         ` Clemens Buchacher
2012-04-16 22:06           ` Zbigniew Jędrzejewski-Szmek
2012-04-17 15:43             ` Junio C Hamano
2012-04-16 17:42       ` Jeff King
2012-04-16 22:44         ` Clemens Buchacher
2012-04-19  6:03           ` Jeff King
2012-04-19  6:58             ` Johannes Sixt
2012-04-26 13:01               ` Jeff King
2012-04-26 18:16                 ` Johannes Sixt
2012-04-26 19:55                   ` Clemens Buchacher
2012-04-26 21:00                     ` Johannes Sixt [this message]
2012-04-26 21:10                       ` [PATCH] t5570: fix forwarding of git-daemon messages via cat Zbigniew Jędrzejewski-Szmek
2012-04-27  7:59                       ` Jeff King
2012-04-27 15:02                       ` Junio C Hamano
2012-04-27  7:55                   ` [PATCH] t5570: forward git-daemon messages in a different way Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4F99B777.4020103@kdbg.org \
    --to=j6t@kdbg.org \
    --cc=drizzd@aon.at \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    --cc=zbyszek@in.waw.pl \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).