Subject: Re: [PATCH v2 0/8] grep: PCRE JIT fixes + ab/no-kwset fix

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Mon, 29 Jul 2019 11:20:48 +0200

On Fri, Jul 26 2019, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason  <[email-masked]> writes:
>
>> 1-3 here are a re-roll on "next". I figured that was easier for
>> everyone with the state of the in-flight patches, it certainly was for
>> me. Sorry Junio if this creates a mess for you.
>
> As long as I can just apply all of them on top of no-kwset and keep
> it a single topic, it wouldn't be too much of a hassle.
>
>> 4-8 are a "fix" for the UTF-8 matching error noted in Carlo's "grep:
>> skip UTF8 checks explicitally" in
>> https://public-inbox.org/git/[email-masked]/
>>
>> As noted the bug isn't fully fixed until 8/8, and that patch relies on
>> unreleased PCRE v2 code. I'm hoping that with 7/8 we're in a good
>> enough state to limp forward as noted in the rationale of those
>> commits.
>
> Yikes.  Perhaps we should kick the no-kwset thing out of 'next' and
> start from scratch?  It does not sound that the world is ready yet.

I have some fix-for-the-fix and was going to submit a v3 of this series,
but I think the more responsible thing to do at this point, especially
with various patches from Carlo that need to be integrated in one way or
another, is to back it out until the outstanding issues are addressed.

If it's not too much trouble, would you mind reverting just the two
patches at the tip of ab/no-kwset in "next"? I.e.

    b65abcafc7 ("grep: use PCRE v2 for optimized fixed-string search", 2019-07-01)
    48de2a768c ("grep: remove the kwset optimization", 2019-07-01)

I believe the rest are all settled & haven't had any issues raised with
them, and those tests & preparatory fixes would be very useful to have
in "master" for any re-roll without needing to be distracted by those
changes.

> But that is just a knee-jerk reaction before reading the actual
> patches.  Let's see how they look ;-)
>
> Thanks.
>
>> Ævar Arnfjörð Bjarmason (8):
>>   grep: remove overly paranoid BUG(...) code
>>   grep: stop "using" a custom JIT stack with PCRE v2
>>   grep: stop using a custom JIT stack with PCRE v1
>>   grep: consistently use "p->fixed" in compile_regexp()
>>   grep: create a "is_fixed" member in "grep_pat"
>>   grep: stess test PCRE v2 on invalid UTF-8 data
>>   grep: do not enter PCRE2_UTF mode on fixed matching
>>   grep: optimistically use PCRE2_MATCH_INVALID_UTF
>>
>>  Makefile                        |  1 +
>>  grep.c                          | 68 +++++++++++----------------------
>>  grep.h                          | 13 ++-----
>>  t/helper/test-pcre2-config.c    | 12 ++++++
>>  t/helper/test-tool.c            |  1 +
>>  t/helper/test-tool.h            |  1 +
>>  t/t7812-grep-icase-non-ascii.sh | 39 +++++++++++++++++++
>>  7 files changed, 80 insertions(+), 55 deletions(-)
>>  create mode 100644 t/helper/test-pcre2-config.c