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

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Dec 10, 2021, 10:08 UTC
Message-ID: <211210.86pmq4daxm.gmgdl@evledraar.gmail.com>
In-Reply-To: <YbMiK1wHzBfYvK2a@coredump.intra.peff.net>

On Fri, Dec 10 2021, Jeff King wrote:

Show 33 quoted lines

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