Re: [PATCH] Makefile: Check for perl script errors with perl -c

From: Matthew Ogilvie <mmogilvi_git@miniinfo.net>
Date: Apr 17, 2010, 17:05 UTC
Message-ID: <20100417170500.GA4587@comcast.net>
In-Reply-To: <20100417072721.GD10365@coredump.intra.peff.net>

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

Show 18 quoted lines

> On Fri, Apr 16, 2010 at 08:29:40PM -0600, Matthew Ogilvie wrote:
> > 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

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]