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

From: Torsten Bögershausen <tboegi@web.de>
Date: Jan 27, 2013, 07:43 UTC
Message-ID: <5104DAB2.5000606@web.de>
In-Reply-To: <7v1ud71uys.fsf@alter.siamese.dyndns.org>

On 26.01.13 22:43, Junio C Hamano wrote:

Show 14 quoted lines

> Torsten Bögershausen <tboegi@web.de> 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