Junio C Hamano <gitster@xxxxxxxxx> writes:
> I would actually not add this to TEST_LINT by default, especially
> when "duplicates" and "executable" that are much simpler and less
> likely to hit false positives are not on by default.
>
> At least, a change to add this to TEST_LINT by default must wait to
> be merged to any integration branch until all the fix-up topics that
> this test triggers in the current codebase graduate to the branch.
>
>>> +test-lint-shell-syntax:
>>> + $(PERL_PATH) check-non-portable-shell.pl $(T)
>>
>> This is wrong if $(PERL_PATH) contains spaces, no?
>
> You are correct; "harness" thing in the same Makefile gets this
> wrong, too. I think the right invocation is:
>
> @'$(PERL_PATH_SQ)' check-non-portable.shell.pl $(T)
>
> although I do not offhand know if that symbol is already exported by
> the top-level Makefile.
I'll tentatively queue this instead. The log message has also been
cleaned up a bit.
-- >8 --
From: Torsten Bögershausen <tboegi@xxxxxx>
Date: Thu, 3 Jan 2013 00:20:19 +0100
Subject: [PATCH] test: Add check-non-portable-shell.pl
Add the perl script "check-non-portable-shell.pl" to detect
non-portable shell syntax.
"echo -n" is an example of a shell command working on Linux, but not
on Mac OS X.
These shell commands are checked and reported as error:
- "echo -n" (printf should be used)
- "sed -i" (GNUism; use a temp file instead)
- "declare" (bashism, often used with arrays)
- "which" (unreliable exit status and output; use type instead)
- "test a == b" (bashism for "test a = b")
"make test-lint-shell-syntax" can be used to run only the check.
Helped-By: Jeff King <peff@xxxxxxxx>
Signed-off-by: Torsten Bögershausen <tboegi@xxxxxx>
Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
---
t/Makefile | 8 ++++++--
t/check-non-portable-shell.pl | 27 +++++++++++++++++++++++++++
2 files changed, 33 insertions(+), 2 deletions(-)
create mode 100755 t/check-non-portable-shell.pl
diff --git a/t/Makefile b/t/Makefile
index 3025418..a43becb 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -16,6 +16,7 @@ DEFAULT_TEST_TARGET ?= test
# Shell quote;
SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
+PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))
T = $(sort $(wildcard t[0-9][0-9][0-9][0-9]-*.sh))
TSVN = $(sort $(wildcard t91[0-9][0-9]-*.sh))
@@ -43,7 +44,7 @@ clean-except-prove-cache:
clean: clean-except-prove-cache
$(RM) .prove
-test-lint: test-lint-duplicates test-lint-executable
+test-lint: test-lint-duplicates test-lint-executable test-lint-shell-syntax
test-lint-duplicates:
@dups=`echo $(T) | tr ' ' '\n' | sed 's/-.*//' | sort | uniq -d` && \
@@ -55,6 +56,9 @@ test-lint-executable:
test -z "$$bad" || { \
echo >&2 "non-executable tests:" $$bad; exit 1; }
+test-lint-shell-syntax:
+ @'$(PERL_PATH_SQ)' check-non-portable-shell.pl $(T)
+
aggregate-results-and-cleanup: $(T)
$(MAKE) aggregate-results
$(MAKE) clean
@@ -87,7 +91,7 @@ test-results:
mkdir -p test-results
test-results/git-smoke.tar.gz: test-results
- $(PERL_PATH) ./harness \
+ '$(PERL_PATH_SQ)' ./harness \
--archive="test-results/git-smoke.tar.gz" \
$(T)
diff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl
new file mode 100755
index 0000000..8b5a71d
--- /dev/null
+++ b/t/check-non-portable-shell.pl
@@ -0,0 +1,27 @@
+#!/usr/bin/perl
+
+# Test t0000..t9999.sh for non portable shell scripts
+# This script can be called with one or more filenames as parameters
+
+use strict;
+use warnings;
+
+my $exit_code=0;
+
+sub err {
+ my $msg = shift;
+ print "$ARGV:$.: error: $msg: $_\n";
+ $exit_code = 1;
+}
+
+while (<>) {
+ chomp;
+ /^\s*sed\s+-i/ and err 'sed -i is not portable';
+ /^\s*echo\s+-n/ and err 'echo -n is not portable (please use printf)';
+ /^\s*declare\s+/ and err 'arrays/declare not portable';
+ /^\s*[^#]\s*which\s/ and err 'which is not portable (please use type)';
+ /test\s+[^=]*==/ and err '"test a == b" is not portable (please use =)';
+ # this resets our $. for each file
+ close ARGV if eof;
+}
+exit $exit_code;
--
1.8.1.203.gc241474
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html