Re: [PATCH] t/test_lib: avoid naked bash arrays in file_lineno

[Junio C Hamano <gitster@pobox.com>]

Carlo Marcelo Arenas Belón  <carenas@gmail.com> writes:

> 662f9cf154 (tests: when run in Bash, annotate test failures with file
> name/line number, 2020-04-11), introduces a way to report the location
> (file:lineno) of a failed test case by traversing the bash callstack.
>
> The implementation requires bash and is therefore protected by a guard
> but NetBSD sh will still have to parse the function and therefore will
> result in:
>
>   ** t0000-basic.sh ***
>   ./test-lib.sh: 681: Syntax error: Bad substitution
>
> Enclose the bash specific code inside an eval to avoid parsing errors
> and while at it, simplify the logic so that instead of traversing the
> callstack just pop the two topmost entries that are required.
>
> Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

Is the rewritten bash-snippet in the evaled text what Dscho
suggested us to use, or is it totally yours?  I know Dscho helped to
shoot down some "simpler" reimplementations you came up with,
pointing out how they were broken, but it is unclear how we ended up
with this version.

I wish you didn't do the "while at it" rewrite in the same patch.
If it were only "put bash-only stuff in an evaled string", it would
have been a lot more comfortable applying it and merging quickly
down, as it would be clear that we won't be breaking bash codepath
and we'd be helping other shells.  With the "while at it", you made
it quite unclear.

> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
> ---
>  t/test-lib.sh | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 1b221951a8..60b8e70678 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -677,14 +677,13 @@ die () {
>  
>  file_lineno () {
>  	test -z "$GIT_TEST_FRAMEWORK_SELFTEST" && test -n "$BASH" || return 0
> -	local i
> -	for i in ${!BASH_SOURCE[*]}
> -	do
> -		case $i,"${BASH_SOURCE[$i]##*/}" in
> -		0,t[0-9]*.sh) echo "t/${BASH_SOURCE[$i]}:$LINENO: ${1+$1: }"; return;;
> -		*,t[0-9]*.sh) echo "t/${BASH_SOURCE[$i]}:${BASH_LINENO[$(($i-1))]}: ${1+$1: }"; return;;
> -		esac
> -	done
> +	eval '
> +		local n=$(echo ${#BASH_SOURCE[*]})
> +		if test $n -ge 2
> +		then
> +			echo "${BASH_SOURCE[$n - 1]}:${BASH_LINENO[$n - 2]}: $1: "
> +		fi
> +	'
>  }
>  
>  GIT_EXIT_OK=