Re: [PATCH v2] parallel-checkout: avoid dash local bug in tests

On Sat, Jun 05, 2021 at 08:11:24PM +0200, René Scharfe 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 because wc's output contains leading spaces and this version of
dash erroneously expands the variable declaration as "local workers= 0",
i.e. it tries to set the "workers" variable to the empty string and also
declare a variable named "0", which not a valid name.  This is a known
dash bug (https://bugs.launchpad.net/ubuntu/+source/dash/+bug/139097).

Perhaps a more accurate wording for this bug would be:

  ... and even fairly recent versions of dash erroneously perform
  field splitting on the expansion of the command substitution before
  assigning it to a local variable.

I think the relevant part of POSIX is section 2.9.1 Simple Commands:

  4. Each variable assignment shall be expanded for tilde expansion,
     parameter expansion, command substitution, arithmetic expansion,
     and quote removal prior to assigning the value.

  https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_09_01

Note that it didn't mention field splitting; though POSIX doesn't
specifies local variables in the first place, so...

Anyway, this bug has been fixed in v0.5.11 (2020-06-01).
This is an old bug, it was already present in v0.5.5 (2009-01-13); I
didn't check earlier versions.

Work around it by passing the command output directly to test instead of
storing it in a variable first.  While at it, let grep count the number
of lines instead of piping its output to wc, which is a bit shorter and
more efficient.

A more debug-friendly alternative would be to save 'grep's output to a
temporary file and use 'test_line_count = $expected_workers'.