Jeff King <peff@xxxxxxxx> writes: > On Tue, Jan 01, 2013 at 10:40:08PM +0100, Torsten Bögershausen wrote: > >> Add the perl script "check-non-portable-shell.pl" to detect non-portable >> shell syntax > > Cool. Thanks for adding more test-lint. But... > >> diff --git a/t/Makefile b/t/Makefile >> index 88e289f..7b0c4dc 100644 >> --- a/t/Makefile >> +++ b/t/Makefile >> @@ -23,7 +23,7 @@ TGITWEB = $(sort $(wildcard t95[0-9][0-9]-*.sh)) >> >> all: $(DEFAULT_TEST_TARGET) >> >> -test: pre-clean $(TEST_LINT) >> +test: pre-clean test-lint-shell-syntax $(TEST_LINT) >> $(MAKE) aggregate-results-and-cleanup > > I do not think it should be a hard-coded dependency of "test", as then > there is no way to turn it off. It would make more sense to me to set a > default TEST_LINT that includes it, but could be overridden by the user. 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. -- 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