Re: [PATCH] parallel-checkout: use grep -c to count workers in tests
From: Matheus Tavares Bernardino <hidden>
Date: 2021-06-05 14:32:13
On Sat, Jun 5, 2021 at 9:27 AM René Scharfe [off-list ref] wrote:
The parallel checkout tests fail when run with /bin/dash on MacOS 11.4, reporting the following error: ./t2080-parallel-checkout-basics.sh: 33: local: 0: bad variable name
That's interesting. It seems that dash is trying to use wc's output
(in this case "0") as a local variable name which, of course, is not
valid.
This reply [1] to this bug report [2] mentions that dash expands a
local assignment like the following:
x="1 2 3"
local y=$x ---expands-to---> local y=1 2 3
So, in this case, dash thinks we are trying to declare three local
variables, y, 2, and 3, which is an error. In bash, the above commands
would result in $y getting the value "1 2 3", even though we didn't
quote $x in the assignment. (BTW, the reply mentions that quoting the
right side of the assignment should make this work in dash as well.)
I wonder if that's what's happening here. Maybe "wc -l" is outputting
a space before the number, and that makes dash parse this line as
something like `local workers="" 0="" `? If that's really the case (I
can't confirm because the bug seems to have been fixed in the dash
version I have), maybe we could mention that in the commit message.
[1]: https://bugs.launchpad.net/ubuntu/+source/dash/+bug/139097/comments/6
[2]: https://bugs.launchpad.net/ubuntu/+source/dash/+bug/139097
quoted hunk ↗ jump to hunk
This seems to be caused by a bug in dash, which doesn't like the pipe into wc for some reason. We can work around it and make the test slightly shorter and faster by having grep do the counting, though, so let's do that. Signed-off-by: René Scharfe <redacted> --- Reduced test case for underlying dash issue: $ dash -c 'foo () { local bar=$(echo baz | wc); }; foo' dash: 1: local: 1: bad variable name The pipe is not even required to trigger the issue: $ dash -c 'foo () { local bar=$(wc /dev/null); }; foo' dash: 1: local: 0: bad variable name Turning wc into a function calms the shell: $ dash -c 'foo () { local bar=$(echo baz | wc); }; wc () { :; }; foo' Setting a global variable also works fine: $ dash -c 'foo () { bar=$(echo baz | wc); }; foo' 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..145276eb4c 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 -c "child_start\[..*\] git checkout--worker" <"$trace_file") &&
Nice, and the result is both cleaner and more efficient :) Just one minor nit: I think we could drop the redirection as grep can take the file name as an argument.
test $workers -eq $expected_workers &&
rm "$trace_file"
} 8>&2 2>&4
--
2.31.1