Subject: Re: [PATCH] tests: turn on test-lint-shell-syntax by default

From: Torsten Bögershausen <[email-masked]>
Date: Sun, 27 Jan 2013 08:43:46 +0100

On 26.01.13 22:43, Junio C Hamano wrote:
> Torsten Bögershausen <[email-masked]> writes:
>
>> Do we really need  "which" to detect if frotz is installed?
> I think we all know the answer to that question is no, but why is
> that a relevant question in the context of this discussion?  One of
> us may be very confused.
>
> I thought the topic of this discussion was that, already knowing
> that "which" should never be used anywhere in our scripts, you are
> trying to devise a mechanical way to catch newcomers' attempts to
> use it in their changes, in order to prevent patches that add use of
> "which" to be sent for review to waste our time.  I was illustrating
> that the approach to override "which" in a shell function for test
> scripts will not be a useful solution for that goal.
Yes, the diskussion went away.

I would rather see the check-non-portable-shell.pl enabled per default.

It looks as if the "which" command is hard to find, when we want a minimal
risk of false positves.
I can see different solutions:

1) We can make a much simpler expression which only catches the most
common usage of which, like
"if whitch foo".

This will not catch lines like

if test -x "$(which foo 2>/dev/null)"

But I think the -x is not a useful anyway, because which should only
list command which have the executable bit set.

2) We drop the which from check-non-portable-shell.pl

I'll send a patch for 1)
/Torsten