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

[Jeff King <peff@peff.net>]

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

> > 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.

> >  	{
> >  		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