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

On Fri, Dec 10 2021, Jeff King wrote:

On Thu, Dec 02, 2021 at 08:16:35PM +0100, SZEDER Gábor wrote:

quoted

quoted

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