diff mbox series

[v3] parallel-checkout: avoid dash local bug in tests

Message ID 3b71b62d-b299-aac8-f796-ee7a5d7f87b0@web.de (mailing list archive)
State New, archived
Headers show
Series [v3] parallel-checkout: avoid dash local bug in tests | expand

Commit Message

René Scharfe June 6, 2021, 1:01 a.m. UTC
Dash bug https://bugs.launchpad.net/ubuntu/+source/dash/+bug/139097
lets the shell erroneously perform field splitting on the expansion of a
command substitution during declaration of a local variable.  It causes
the parallel-checkout tests to fail e.g. when running them with
/bin/dash on MacOS 11.4, where they error out like this:

   ./t2080-parallel-checkout-basics.sh: 33: local: 0: bad variable name

That's because the output of wc -l contains leading spaces and the
returned number of lines is treated as another variable to declare, i.e.
as in "local workers= 0".

Work around it by enclosing the command substitution in quotes.

Helped-by: Matheus Tavares Bernardino <matheus.bernardino@usp.br>
Helped-by: SZEDER Gábor <szeder.dev@gmail.com>
Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: René Scharfe <l.s.r@web.de>
---
Changes since v2:
- Use minimal fix.
- New commit message.

 t/lib-parallel-checkout.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--
2.31.1

Comments

Junio C Hamano June 6, 2021, 1:41 a.m. UTC | #1
René Scharfe <l.s.r@web.de> writes:

> Dash bug https://bugs.launchpad.net/ubuntu/+source/dash/+bug/139097
> lets the shell erroneously perform field splitting on the expansion of a
> command substitution during declaration of a local variable.  It causes
> the parallel-checkout tests to fail e.g. when running them with
> /bin/dash on MacOS 11.4, where they error out like this:
>
>    ./t2080-parallel-checkout-basics.sh: 33: local: 0: bad variable name
>
> That's because the output of wc -l contains leading spaces and the
> returned number of lines is treated as another variable to declare, i.e.
> as in "local workers= 0".
>
> Work around it by enclosing the command substitution in quotes.
>
> Helped-by: Matheus Tavares Bernardino <matheus.bernardino@usp.br>
> Helped-by: SZEDER Gábor <szeder.dev@gmail.com>
> Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> Signed-off-by: René Scharfe <l.s.r@web.de>
> ---
> Changes since v2:
> - Use minimal fix.
> - New commit message.

Thanks.  As I said, I do not necessarily think this is conceptually
"minimal", but we use workers only once and without surrounding dq
so it is easy to see that this also is correct.

Will queue.

>
>  t/lib-parallel-checkout.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/lib-parallel-checkout.sh b/t/lib-parallel-checkout.sh
> index 21f5759732..83b279a846 100644
> --- a/t/lib-parallel-checkout.sh
> +++ b/t/lib-parallel-checkout.sh
> @@ -27,7 +27,7 @@ test_checkout_workers () {
>  	rm -f "$trace_file" &&
>  	GIT_TRACE2="$(pwd)/$trace_file" "$@" 2>&8 &&
>
> -	local workers=$(grep "child_start\[..*\] git checkout--worker" "$trace_file" | wc -l) &&
> +	local workers="$(grep "child_start\[..*\] git checkout--worker" "$trace_file" | wc -l)" &&
>  	test $workers -eq $expected_workers &&
>  	rm "$trace_file"
>  } 8>&2 2>&4
> --
> 2.31.1
diff mbox series

Patch

diff --git a/t/lib-parallel-checkout.sh b/t/lib-parallel-checkout.sh
index 21f5759732..83b279a846 100644
--- a/t/lib-parallel-checkout.sh
+++ b/t/lib-parallel-checkout.sh
@@ -27,7 +27,7 @@  test_checkout_workers () {
 	rm -f "$trace_file" &&
 	GIT_TRACE2="$(pwd)/$trace_file" "$@" 2>&8 &&

-	local workers=$(grep "child_start\[..*\] git checkout--worker" "$trace_file" | wc -l) &&
+	local workers="$(grep "child_start\[..*\] git checkout--worker" "$trace_file" | wc -l)" &&
 	test $workers -eq $expected_workers &&
 	rm "$trace_file"
 } 8>&2 2>&4