Re: [PATCH 1/2] t8005: avoid grep on non-ASCII data

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Feb 21, 2016, 23:31 UTC
Message-ID: <CAPig+cSXsk4Pp9adi4KvYjdCwaw4R0Jrv2vwC0JTCyzomWxaww@mail.gmail.com>
In-Reply-To: <20160221231913.GA4094@sigill.intra.peff.net>

On Sun, Feb 21, 2016 at 6:19 PM, Jeff King <peff@peff.net> wrote:

Show 16 quoted lines

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

I came to the same conclusion but forgot to say so at the end of my message.

Show 12 quoted lines

> 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.
>
> +filter_blame () {
> +       perl -e '
> +               binmode STDIN;
> +               binmode STDOUT;

I was worried about binmode() due to some vague recollection from years and years ago of it being problematic on Windows, but I see these tests are all protected by !MINGW anyhow...

Show 40 quoted lines

> +               while (<>) {
> +                       print if /^(author|summary) /;
> +               }
> +       '
> +}
> +
>  test_expect_success !MINGW \
>         'blame respects i18n.commitencoding' '
>         git blame --incremental file | \
> -               egrep "^(author|summary) " > actual &&
> +               filter_blame >actual &&
>         test_cmp actual expected
>  '
>
> @@ -53,7 +63,7 @@ test_expect_success !MINGW \
>         'blame respects i18n.logoutputencoding' '
>         git config i18n.logoutputencoding eucJP &&
>         git blame --incremental file | \
> -               egrep "^(author|summary) " > actual &&
> +               filter_blame > actual &&
>         test_cmp actual expected
>  '
>
> @@ -69,7 +79,7 @@ EOF
>  test_expect_success !MINGW \
>         'blame respects --encoding=UTF-8' '
>         git blame --incremental --encoding=UTF-8 file | \
> -               egrep "^(author|summary) " > actual &&
> +               filter_blame >actual &&
>         test_cmp actual expected
>  '
>
> @@ -85,7 +95,7 @@ EOF
>  test_expect_success !MINGW \
>         'blame respects --encoding=none' '
>         git blame --incremental --encoding=none file | \
> -               egrep "^(author|summary) " > actual &&
> +               filter_blame >actual &&
>         test_cmp actual expected
>  '