Subject: Re: [PATCH v3 01/12] check-non-portable-shell: check for "test <cond> -a/-o <cond>"

From: Junio C Hamano <[email-masked]>
Date: Wed, 21 Apr 2021 09:32:11 -0700

Ævar Arnfjörð Bjarmason <[email-masked]> writes:

>>> 	t6402-merge-rename.sh:865: error: "test A || test B" preferred to "test A -o B": test 0 -eq $(git ls-files -o | wc -l) &&
>>
>> With: 7dbe8c8003, (check-non-portable-shell.pl: `wc -l` may have
> ...
> I think those patches are good in their own right, i.e. replacing things
> with more incremental helpers and test_cmp-like functions.
>
> But I believe the code you're changing is not non-portable. It was using
> the output of "wc -l" with the "=" operator that wasn't portable.

Not necessarily.  The last one shown above

	test 0 -eq $(git ls-files -o | wc -l) &&

is still valid and correct with s/-eq/=/.  It lets shell to remove
excess whitespaces around output from some implementations of "wc",
so textual '=' is perfectly fine.

> These ones are all occurances that use "-eq".
>
> And:
>
>     test "0" -eq " 0"
>
> etc., is true, which is why these pass on OSX and beyond.

Perhaps, but I am not sure how portable it is to rely on "-eq"
ignoring the leading whitespaces.

A more conventional (read: time tested) way is to $(... | wc -l)
*NOT* enclosed inside dq, so that the shell will strip out any
excess spaces around it, and use '=', i.e.

    test 3 = $(... | wc -l)

and that is what we end up evaluating when we write

    test_line_count = 3 file

This does rely on that "wc -l" will run and produce _some_ output to
be syntactically correct, though.