Re: [RFC] Add basic syntax check on shell scripts

[Junio C Hamano <gitster@pobox.com>]

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

> On Wed, Dec 5, 2012 at 2:39 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Or a project commit hook?
>>
>> Surely.  It is OK to have "cd t && make test-lint" in your
>> pre-commit hook.
>
> No, what I meant is a shared pre-commit script that all git devs are
> encouraged (or forced) to install so bugs are found locally rather
> than after patches are sent to you. The hook content does not really
> matter.

Honestly, I do not really care (yet); what you are talkng about is
merely an addition to Documentation/SubmittingPatches, which is
trivial.

The content of the hook is much more important.

If it has too many false positives, it is not worth even encouraging
its use to less experienced ones, as they will have hard time to
figure out which errors matter and which erros can be ignored.  It
will make contributing to the project harder, not easier.

As I do not think (1) we would be able to do a good job reducing
false positives without writing a full POSIX shell parser, and (2)
we would want to be in the business of writing POSIX shell parser
[*1*], I am somewhat skeptical.

And if we cannot come up with a reliable hook in the first place,
there isn't much point in discussing a policy to encourage such a
hook, is it?


[Footnote]

*1* Is there a free one that is portable, perhaps written in either
Perl or Python, to which we can easily plug our own logic?  In order
to catch basic errors, I think it is sufficient to tokenize the
script into independent series of simple commands, even ignoring
variable substitutions and evals, and just have a bunch of "we do
not allow option X to command Y" rules (e.g. "no -i to sed", "the
first argument must be 'git' for "test_must_fail").