Re: [PATCH] t/test_lib: avoid naked bash arrays in file_lineno
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 7, 2020, 14:53 UTC
- Message-ID
- <xmqqr1vv4wzo.fsf@gitster.c.googlers.com>
- In-Reply-To
- <20200507055118.69971-1-carenas@gmail.com>
Carlo Marcelo Arenas Belón <carenas@gmail.com> writes:
Show 16 quoted lines
> 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.
Show 31 quoted lines
> 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=