Re: [PATCH 1/4] test: Add target test-lint-shell-syntax

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 1, 2013, 22:07 UTC
Message-ID: <7v7gnw8slq.fsf@alter.siamese.dyndns.org>
In-Reply-To: <201301012240.10722.tboegi@web.de>

Torsten Bögershausen <tboegi@web.de> writes:

Show 19 quoted lines

> Add the perl script "check-non-portable-shell.pl" to detect non-portable
> shell syntax
> Many systems use gnu tools which accept an extended syntax in shell scripts,
> which is not portable on all systems and causes the test suite to fail.
>
> To prevent contributors using e.g. Linux to add non-portable test code,
> "check-non-portable-shell.pl" is run as part of
> "make test" or "make in the t/ directory.
>
> "echo -n" is an example of a statement working on Linux,
> but not on e.g. Mac OS X.
>
> Beside "echo -n" we check for
> "sed -i",
> arrays in shell scripts (declare statement),
> "which" (use type instead),
> or "==" (bash style of =)
>
> Signed-off-by: Torsten Bögershausen <tboegi@web.de>

What it checks looks like a good start, but the indentation of it (and the log message) seems very screwed up.

I also have to wonder what's the false positive rate of this. When you are preparing a new test, you would ideally want a mode that checks only parts that you just added, without seeing noises from existing violations and false positives from the part you did not touch. Otherwise, it will be too cumbersome to run for developers, and the check mechanism will end up used by nobody.

Show 6 quoted lines

> +######################################################################
> +# Test t0000..t9999.sh for non portable shell scripts                #
> +# Examples are "echo -n" or "sed -i"                                 #
> +# This script can be called with one or more filenames as parameters #
> +#
> +######################################################################

Just a style thing (style requests are not optional, though ;-), but these box comments are moderately annoying to read and extremely annoying to modify. Writing it like this:

Show 5 quoted lines

> +#
> +# Test t0000..t9999.sh for non portable shell scripts
> +# Examples are "echo -n" or "sed -i"
> +# This script can be called with one or more filenames as parameters
> +#

should be sufficiently loud.