Re: [PATCH v2 1/2] t1510: remove need for "test_untraceable", retain coverage

["SZEDER Gábor" <szeder.dev@gmail.com>]

On Fri, Dec 10, 2021 at 04:47:23AM -0500, Jeff King wrote:
> On Thu, Dec 02, 2021 at 08:16:35PM +0100, SZEDER Gábor wrote:
> 
> > > @@ -62,7 +59,7 @@ test_repo () {
> > >  			export GIT_WORK_TREE
> > >  		fi &&
> > >  		rm -f trace &&
> > > -		GIT_TRACE_SETUP="$(pwd)/trace" git symbolic-ref HEAD >/dev/null &&
> > > +		GIT_TRACE_SETUP="$(pwd)/trace" git symbolic-ref HEAD >/dev/null 2>>stderr &&
> > 
> > I suspect that it's lines like this that make Peff argue for
> > BASH_XTRACEFD :)
> > 
> > While this is not a compound command, it does contain a command
> > substitution, and the trace generated when executing the command in
> > that command substitution goes to the command's stderr, and then,
> > because of the redirection, to the 'stderr' file.
> 
> Better still, the behavior varies between shells:

Indeed, although POSIX seems to be quite clear about what should
happen in this case: the specs for simple commands [1] state that
redirections should be performed before variable assignments are
expanded for, among other things, command substitution.

>   $ bash -c 'set -x; FOO=$(echo foo) echo main >stdout 2>stderr; set +x; grep . stdout stderr'
>   ++ echo foo
>   + FOO=foo
>   + echo main
>   + set +x
>   stdout:main
> 
>   $ dash -c 'set -x; FOO=$(echo foo) echo main >stdout 2>stderr; set +x; grep . stdout stderr'
>   + FOO=foo echo main
>   + set +x
>   stdout:main
>   stderr:+ echo foo

So in case of these commands the shell should first redirect stdout
and stderr, then expand the command substitution, thus it should write
the trace for the 'echo foo' within to stderr _while_ stderr is
redirected.  It seems that dash, for once, does conform to POSIX.

Although the standard-conform behavior is potentially more problematic
for us, because it would cause test failure with tracing enabled when
used like this:

  GIT_TRACE="$(pwd)/trace" git cmd --opts 2>actual.err &&
  test_cmp expected.err actual.err

because the "+ pwd" trace output from the command substitution would
go to 'err.actual'; 9b2ac68f27 (t5526: use $TRASH_DIRECTORY to specify
the path of GIT_TRACE log file, 2018-02-24) fixed a case like this.


[1] https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_09_01