patchMakefile: Check for perl script errors with perl -c

This allows you to notice trivial syntax errors in perl scripts earlier, for example before running t/* tests that generate a lot of separate errors.

You have to set USE_PERL_CHECK to enable this, because it uses the non-standard PIPESTATUS bashism to grep out "{script} syntax OK" useless noise.

Signed-off-by: Matthew Ogilvie <mmogilvi_git@miniinfo.net>
---

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).

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.

-- Matthew Ogilvie [mmogilvi_git@miniinfo.net]

 Makefile |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

On Fri, Apr 16, 2010 at 08:29:40PM -0600, Matthew Ogilvie wrote:

I usually do the same thing in my perl makefiles, so I would find it useful.

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

So something like:

On Sat, Apr 17, 2010 at 03:27:21AM -0400, Jeff King wrote:

Yes, "perl -cw"'s exit code is always good, but the standard error is needlessly noisy in the success case:

  $ perl -cw -e 'print "hi\n"'
  -e syntax OK
  $ echo $?
  0

Which then leaves a choice among not-great options:

1. Accept the noise output from make and perl.  If we are willing to
   accept this, then a simpler and/or uncoditional patch would be fine.

2. Filter out the "{scriptName} syntax OK" noise with grep (or sed),
   but then $? is grep's status (not perl's), and you have to go
   through contortions to properly test perl's status:

    2a. Use PIPESTATUS, but this is a non-portable bashism.
        My current version of the patch elects to do this, but
        leaves the check disabled to (hopefully) avoid portability
        issues.  (A second advantage of leaving it disabled [or at
        least disablable] is if someone is in a cross-compile
        environment and the target perl path is different 
        from the build perl path.)

    2b. Use a portable technique that involves echoing the status
        redirected to file descriptor 3, then pulling the status out
        of file descriptor 3 outside the pipeline.  This is frankly
        kind of complicated and hard to read.

So, I take it you would be happy with the noisy output? Anyone else have an opinion? If someone knows a cleaner way to resolve this, or if the group consensus is that we like the patch's concept but would rather resolve the noisy output some other way (perhaps just accept it), I could change the patch.

-- Matthew Ogilvie [mmogilvi_git@miniinfo.net]

On Sat, Apr 17, 2010 at 11:05:00AM -0600, Matthew Ogilvie wrote:

Ah, OK. I misunderstood what you were trying to do before.

Though I would prefer it silenced, I don't personally have a big problem with this. I guess others might.

Hmm. The cross-compilation thing is interesting, but I'm not sure it even works now. We already are relying on generating perl.mak and using it as part of our build, I think. I haven't looked closely at the perl build stuff in git, though, so maybe there is a way to make it work.

Yeah, I have used that technique before, and it is unreadable. Maybe simpler is to cheat with a tempfile:

  if ! perl -wc $@+ 2>$@.stderr; \
    then cat >&2 $@.stderr; rm -f $@.stderr; exit 1; \
    else rm -f $@.stderr; fi && \

but that is getting a bit unreadable, too. I dunno.

-Peff