Re: [PATCH 2/3] t5615: avoid re-using descriptor 4

From

Jeff King <peff@peff.net>

Date

Oct 20, 2017, 05:50 UTC

Message-ID

<20171020055058.s5dgxyfjnqtlxv4d@sigill.intra.peff.net>

In-Reply-To

<20171019232337.zfd7occtjboem7f4@sigill.intra.peff.net>

On Thu, Oct 19, 2017 at 07:23:37PM -0400, Jeff King wrote:

> So one trick is that we can't just set it to a higher number. We have to
> also open and manage that descriptor. It might be enough to do:
> 
>   if test -n "$BASH_VERSION"
>   then
> 	exec 999>&4
> 	BASH_XTRACEFD=999
>   fi
> [...]
> I think it might be workable, but I'm worried we're opening a can of
> worms. Or continuing to dig into the can of worms from the original
> BASH_XTRACEFD, if you prefer. :)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 14fac6d6f2..833ceaefd2 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -377,7 +377,12 @@ fi
 #
 # Note also that we don't need or want to export it. The tracing is local to
 # this shell, and we would not want to influence any shells we exec.
-BASH_XTRACEFD=4
+if test -n "$BASH_VERSION"
+then
+	exec 999>&4
+	BASH_XTRACEFD=999
+	silence_trace="999>/dev/null"
+fi
 
 test_failure=0
 test_count=0
@@ -627,14 +632,16 @@ test_eval_ () {
 	#     be _inside_ the block to avoid polluting the "set -x" output
 	#
 
-	test_eval_inner_ "$@" </dev/null >&3 2>&4
-	{
-		test_eval_ret_=$?
-		if want_trace
-		then
-			set +x
-		fi
-	} 2>/dev/null 4>&2
+	eval '
+		test_eval_inner_ "$@" </dev/null >&3 2>&4
+		{
+			test_eval_ret_=$?
+			if want_trace
+			then
+				set +x
+			fi
+		} 2>/dev/null '"$silence_trace"'
+	'
 
 	if test "$test_eval_ret_" != 0 && want_trace
 	then

I really hate that extra eval, since it adds an extra layer of "+" to
the tracing output in bash.

But I can't find a way to make it work without it. The "999>/dev/null"
is syntactically bogus in non-bash shells, so it _must_ appear as part
of an eval.  We can't conditionally stick it at the end of the eval run
by test_eval_inner_, because that one is running the actual test code
(so it may bail early with "return", for example).

TBH, I'm not 100% sure that the initial "exec 999>&4" won't cause
complaints in some shells. Dash seems to handle it, but others might
not. That can be worked around with another eval, though.

So I dunno. It does solve the problem in a way that the individual test
scripts wouldn't have to care about. But it's a lot of eval trickery.

-Peff