From: John Keeping <john@keeping.me.uk>
To: Jeff King <peff@peff.net>
Cc: Eric Sunshine <sunshine@sunshineco.com>,
Git List <git@vger.kernel.org>,
Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH 1/2] t8005: avoid grep on non-ASCII data
Date: Sun, 21 Feb 2016 23:41:35 +0000 [thread overview]
Message-ID: <20160221234135.GA14382@river.lan> (raw)
In-Reply-To: <20160221231913.GA4094@sigill.intra.peff.net>
On Sun, Feb 21, 2016 at 06:19:14PM -0500, Jeff King wrote:
> On Sun, Feb 21, 2016 at 04:01:27PM -0500, Eric Sunshine wrote:
>
> > On Sun, Feb 21, 2016 at 12:32 PM, John Keeping <john@keeping.me.uk> wrote:
> > > GNU grep 2.23 detects the input used in this test as binary data so it
> > > does not work for extracting lines from a file. We could add the "-a"
> > > option to force grep to treat the input as text, but not all
> > > implementations support that. Instead, use sed to extract the desired
> > > lines since it will always treat its input as text.
> > >
> > > While touching these lines, modernize the test style to avoid hiding the
> > > exit status of "git blame" and remove a space following a redirection
> > > operator.
> > >
> > > Signed-off-by: John Keeping <john@keeping.me.uk>
> > > ---
> > > diff --git a/t/t8005-blame-i18n.sh b/t/t8005-blame-i18n.sh
> > > @@ -35,8 +35,8 @@ EOF
> > > test_expect_success !MINGW \
> > > 'blame respects i18n.commitencoding' '
> > > - git blame --incremental file | \
> > > - egrep "^(author|summary) " > actual &&
> > > + git blame --incremental file >output &&
> > > + sed -ne "/^\(author\|summary\) /p" output >actual &&
> >
> > These tests all crash and burn with BSD sed (including Mac OS X) since
> > you're not restricting yourself to BRE (basic regular expressions).
> > You _could_ request extended regular expressions, which do work on
> > those platforms, as well as with GNU sed:
> >
> > sed -nEe "/^(author|summary) /p" ...
>
> At that point, I think we may as well use grep, because obscure
> platforms are probably broken either way.
Also GNU sed doesn't understand "-E", it uses "-r" for --regexp-extended.
> I'm tempted to just go the perl route. We already depend on at least a
> baisc version of perl5 being installed for many of the other tests, so
> it's not really introducing a new dependency.
>
> Something like the patch below works for me. I think we could make it
> shorter by using $PERLIO to get the raw behavior, but using binmode will
> work even on ancient versions of perl.
>
> John, if you agree on the direction, feel free to combine it with your
> patch.
My original sed version was:
sed -ne "/^author /p" -e "/^summary /p"
which I think will work on all platforms (we already use it in
t0000-basic.sh) but then I decided to be too clever :-(
I still think sed is simpler than introducing a new function to wrap a
perl script.
next prev parent reply other threads:[~2016-02-21 23:41 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-02-07 16:25 Test failures with GNU grep 2.23 John Keeping
2016-02-19 11:59 ` Jeff King
2016-02-19 17:27 ` Eric Sunshine
2016-02-19 17:38 ` Junio C Hamano
2016-02-19 19:11 ` Jeff King
2016-02-19 19:23 ` John Keeping
2016-02-19 19:33 ` Jeff King
2016-02-21 17:32 ` [PATCH 0/2] Fix test " John Keeping
2016-02-21 17:32 ` [PATCH 1/2] t8005: avoid grep on non-ASCII data John Keeping
2016-02-21 21:01 ` Eric Sunshine
2016-02-21 23:19 ` Jeff King
2016-02-21 23:31 ` Eric Sunshine
2016-02-21 23:35 ` Jeff King
2016-02-21 23:41 ` John Keeping [this message]
2016-02-21 23:50 ` Eric Sunshine
2016-02-22 22:18 ` Jeff King
2016-02-22 22:25 ` Junio C Hamano
2016-02-23 23:01 ` Junio C Hamano
2016-02-24 10:24 ` John Keeping
2016-02-21 23:31 ` Junio C Hamano
2016-02-21 23:40 ` Eric Sunshine
2016-02-21 17:32 ` [PATCH 2/2] t9200: " John Keeping
2016-02-21 21:15 ` Eric Sunshine
2016-02-21 23:43 ` John Keeping
2016-02-22 0:04 ` Eric Sunshine
2016-02-22 22:25 ` Jeff King
2016-02-23 22:55 ` Junio C Hamano
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=20160221234135.GA14382@river.lan \
--to=john@keeping.me.uk \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=peff@peff.net \
--cc=sunshine@sunshineco.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.