From: Jeff King <peff@peff.net>
To: Matthew Ogilvie <mmogilvi_git@miniinfo.net>
Cc: git@vger.kernel.org, gitster@pobox.com
Subject: Re: [PATCH] Makefile: Check for perl script errors with perl -c
Date: Sat, 17 Apr 2010 13:55:53 -0400 [thread overview]
Message-ID: <20100417175553.GC23642@coredump.intra.peff.net> (raw)
In-Reply-To: <20100417170500.GA4587@comcast.net>
On Sat, Apr 17, 2010 at 11:05:00AM -0600, Matthew Ogilvie 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
Ah, OK. I misunderstood what you were trying to do before.
> 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.
Though I would prefer it silenced, I don't personally have a big problem
with this. I guess others might.
> 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.)
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.
> 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.
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
prev parent reply other threads:[~2010-04-17 17:56 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-04-17 2:29 [PATCH] Makefile: Check for perl script errors with perl -c Matthew Ogilvie
2010-04-17 7:27 ` Jeff King
2010-04-17 17:05 ` Matthew Ogilvie
2010-04-17 17:55 ` Jeff King [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20100417175553.GC23642@coredump.intra.peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=mmogilvi_git@miniinfo.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox