On Fri, Apr 16, 2010 at 08:29:40PM -0600, Matthew Ogilvie wrote:
> I'm not sure anyone will think this is worth including, but I'm
> used to "make" (and the compiler) detecting trivial errors
> in compiled langauges, and was getting annoyed that it wasn't
> doing something similar for perl scripts (especially since in git you
> are really expected to "make" the scripts anyway).
I usually do the same thing in my perl makefiles, so I would find it
useful.
> The whole tradeoff between noise ("{script} syntax OK"), portability
> (PIPESTATUS is a bashism), or really ugly contortions with redirecting
> extra file descriptors (to avoid PIPESTATUS) seems to be the biggest
> downside of the idea behind this patch.
Why do you need to run it through grep? Doesn't:
echo 'use strict; bogosity' >foo.pl
perl -wc foo.pl
properly set the exit code? I get:
$ perl -wc foo.pl
Bareword "bogosity" not allowed while "strict subs" in use at foo.pl line 1.
foo.pl had compilation errors.
$ echo $?
255
> @@ -1553,6 +1557,14 @@ $(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl
> -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
> $@.perl >$@+ && \
> chmod +x $@+ && \
> + if test x"$(USE_PERL_CHECK)" != x"" ; then \
> + '$(PERL_PATH_SQ)' -cw $@+ 2>&1 | grep -v '^$@+ syntax OK$$' 1>&2 ; \
> + perlStat="$${PIPESTATUS[0]}" && \
> + if test x"$$perlStat" != x"0" ; then \
> + echo '"$(PERL_PATH_SQ) -c $@+" failed' 1>&2 ; \
> + exit "$$perlStat" ; \
> + fi ; \
> + fi && \
> mv $@+ $@
So something like:
diff --git a/Makefile b/Makefile
index 87c90d6..d9b6613 100644
--- a/Makefile
+++ b/Makefile
@@ -1545,6 +1545,10 @@ $(SCRIPT_LIB) : % : %.sh
ifndef NO_PERL
$(patsubst %.perl,%,$(SCRIPT_PERL)): perl/perl.mak
+ifdef USE_PERL_CHECK
+PERL_CHECK = perl -wc $@+ &&
+endif
+
perl/perl.mak: GIT-CFLAGS perl/Makefile perl/Makefile.PL
$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' prefix='$(prefix_SQ)' $(@F)
@@ -1562,6 +1566,7 @@ $(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl
-e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
$@.perl >$@+ && \
chmod +x $@+ && \
+ $(PERL_CHECK) \
mv $@+ $@
You could even just make it unconditional. I don't know that we have an
official policy, but we usually strive for strict, warnings-free perl.
-Peff
--
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