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

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Fri, Dec 10 2021, 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:
>
>   $ 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
>
> -Peff

I just re-rolled a v3 with a fix for this and the general issue pointed
out by SZEDER, thanks both.

But as far as keeping test_untraceable goes, isn't this a good argument
for dropping it?

I.e. if bash was the odd shell out that included the $(...) command in
the trace output it would be a good reason to keep it, then we could
perhaps use it without worrying about our trace output being corrupted
when we did a "test_cmp" on "stderr" later.

But it's "dash" that's including it, so unless we're going to outright
forbid it to have -x output (which is inconvienent) we'll need to deal
with t either way (in my re-roll I just replaced $(pwd) with $PWD).