[PATCH v4 3/3] test-lib.sh: remove "BASH_XTRACEFD"

From

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

Date

Dec 13, 2021, 01:38 UTC

Message-ID

<patch-v4-3.3-8b5ae33376e-20211213T013559Z-avarab@gmail.com>

In-Reply-To

<cover-v4-0.3-00000000000-20211213T013559Z-avarab@gmail.com>

    "there here isn't a portable or scalable solution to this [...] we
    can work around it by pointing the "set -x" output to our
    descriptor 4"

And finally that:

    "Automatic tests for our "-x" option may be a bit too meta"

We could retain "BASH_XTRACEFD=4" anyway, but doing so is bad because:

 1) Since we're caring about "-x" passing in CI under "dash" on Ubuntu
    using "BASH_XTRACEFD=4" will amount to hiding an error we'll run
    into eventually. Tests will pass locally with "bash", but fail in
    CI with "dash" (or under other non-"bash" shells).

 2) As the amended code in "test_eval_" shows (an amended revert to
    the pre-image of d88785e424a) it's simpler to not have to take
    this "bash" special-case into account.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/test-lib.sh | 43 +++++++------------------------------------
 1 file changed, 7 insertions(+), 36 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index ddfa0567e9e..b008716917b 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -627,19 +627,6 @@ else
 	exec 4>/dev/null 3>/dev/null
 fi
 
-# Send any "-x" output directly to stderr to avoid polluting tests
-# which capture stderr. We can do this unconditionally since it
-# has no effect if tracing isn't turned on.
-#
-# Note that this sets up the trace fd as soon as we assign the variable, so it
-# must come after the creation of descriptor 4 above. Likewise, we must never
-# unset this, as it has the side effect of closing descriptor 4, which we
-# use to show verbose tests to the user.
-#
-# 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
-
 test_failure=0
 test_count=0
 test_fixed=0
@@ -926,36 +913,20 @@ test_eval_ () {
 	# the shell from printing the "set +x" to turn it off (nor the saving
 	# of $? before that). But we can make sure that the output goes to
 	# /dev/null.
-	#
-	# There are a few subtleties here:
-	#
-	#   - we have to redirect descriptor 4 in addition to 2, to cover
-	#     BASH_XTRACEFD
-	#
-	#   - the actual eval has to come before the redirection block (since
-	#     it needs to see descriptor 4 to set up its stderr)
-	#
-	#   - likewise, any error message we print must be outside the block to
-	#     access descriptor 4
-	#
-	#   - checking $? has to come immediately after the eval, but it must
-	#     be _inside_ the block to avoid polluting the "set -x" output
-	#
-
-	test_eval_inner_ "$@" </dev/null >&3 2>&4
 	{
+		test_eval_inner_ "$@" </dev/null >&3 2>&4
 		test_eval_ret_=$?
 		if want_trace
 		then
 			test 1 = $trace_level_ && set +x
 			trace_level_=$(($trace_level_-1))
-		fi
-	} 2>/dev/null 4>&2
 
-	if test "$test_eval_ret_" != 0 && want_trace
-	then
-		say_color error >&4 "error: last command exited with \$?=$test_eval_ret_"
-	fi
+			if test "$test_eval_ret_" != 0
+			then
+				say_color error >&4 "error: last command exited with \$?=$test_eval_ret_"
+			fi
+		fi
+	} 2>/dev/null
 	return $test_eval_ret_
 }
 

-- 
2.34.1.1024.g573f2f4b767