Re: [PATCH 2/6] t/lib-git-daemon: record daemon log

From: Jeff King <peff@peff.net>
Date: Jan 25, 2018, 19:08 UTC
Message-ID: <20180125190824.GA26309@sigill.intra.peff.net>
In-Reply-To: <38b41541-5758-d013-3d64-314eec7e31ed@lucaswerkmeister.de>

On Thu, Jan 25, 2018 at 12:56:47PM +0100, Lucas Werkmeister wrote:

Show 12 quoted lines

> > Let's dump the log into a file, as well, so that future
> > tests can check the log. There are two subtleties worth
> > calling out here:
> > 
> >   - we replace "cat" with a subshell loop around "read" to
> >     ensure that there's no buffering (so that tests can be
> >     sure that after a request has been served, the matching
> >     log entries will have made it to the file)
> 
> POSIX specifies the -u option for that behavior, can’t you use that?
> (GNU coreutils’ cat ignores it, since writing without delay is
> apparently its default behavior already.)

Actually, this glosses over one other detail, which is that we'd also need to replace "cat" with "tee" to keep output going to descriptor 4. That's not strictly necessary (it's just for debugging output), so we could drop that. But the shell loop seemed easy enough.

Show 24 quoted lines

> >  	{
> >  		read line <&7
> > +		echo "$line"
> >  		echo >&4 "$line"
> > -		cat <&7 >&4 &
> > -	} 7<git_daemon_output &&
> > +		(
> > +			while read line <&7
> > +			do
> > +				echo "$line"
> > +				echo >&4 "$line"
> > +			done
> > +		) &
> > +	} 7<git_daemon_output >>"$TRASH_DIRECTORY/daemon.log" &&
> >  
> >  	# Check expected output
> >  	if test x"$(expr "$line" : "\[[0-9]*\] \(.*\)")" != x"Ready to rumble"
> > 
> 
> read without -r clobbers backslashes, and echo may interpret escape
> sequences. To faithfully reproduce the output, it would be better to use
> read -r and printf '%s\n' "$line", I think. (However, it looks like the
> existing code already uses read+echo, so I guess you could also keep
> that pattern in this change and then fix it in a later one.)

Yeah. I doubt it matters much, since this is just inside our tests, and we control the input. But it doesn't hurt to do it in the more robust way. I'll re-roll this patch.

-Peff