On Mon, Jul 22, 2019 at 12:42 PM Ævar Arnfjörð Bjarmason
[off-list ref] wrote:

On Mon, Jul 22 2019, Johannes Schindelin wrote:

quoted

So I am fine with this patch.

I'm not, not because I dislike the approach. I haven't made up my mind
yet.

my bad, I should had explained better the regression I was trying to
fix with this patch :

$ git version
git version 2.22.0.709.g102302147b.dirty
$ git grep "Nguyễn Thái"
.mailmap:Nguyễn Thái Ngọc Duy [off-list ref]
fatal: pcre2_match failed with error code -22: UTF-8 error: isolated
byte with 0x80 bit set
$ git grep -I "Nguyễn Thái"
.mailmap:Nguyễn Thái Ngọc Duy [off-list ref]
po/TEAMS:Members:       Nguyễn Thái Ngọc Duy <pclouds AT gmail.com>
po/vi.po:# Nguyễn Thái Ngọc Duy [off-list ref], 2012.
t/t1302-repo-version.sh:# Copyright (c) 2007 Nguyễn Thái Ngọc Duy
t/t2104-update-index-skip-worktree.sh:# Copyright (c) 2008 Nguyễn Thái Ngọc Duy
fatal: pcre2_match failed with error code -8: UTF-8 error: byte 2 top
bits not 0x80

I stopped paying attention to this error-with-not-JIT discussion when I
heard that some other series going into next for Windows fixed that
issue[1]

But now we have it again in some form? My ab/no-kwset has a lot of tests
for encodings & locales combined with grep, don't some of those trigger
this? If so we should make any such failure a test & part of this patch.

I don't have a windows environment to test, and I admit I wasn't
following clearly
the whole conversation but at least in my case, I never got any test
to fail, and I haven't
seen any test with broken UTF-8.

I apologize for not sending a test and will correct that, but I am
concerned that
according to PCRE documentation the behaviour is undefined and therefore the
test might be flacky.

Right now we don't have the info of whether we're really using the JIT
or not, but that would be easy to add to grep's --debug mode for use in
a test prereq.

I would think that extending testtool would be a better option, I have indeed
a patch checking for UTF-8 support using git directly which I hadn't sent just
because I thought it looked too hacky, but agree it will be nice to have all
those dependencies clear and the corresponding tests otherwise

It is interesting to note that because of the conservative way we enable UTF-8
then it will depend on several factors if the problematic code gets triggered

As noted in [2] I'd be inclined to go the other way, if we indeed have
some cases where PCRE skips its own checks does not dying actually give
us anything useful? I'd think not, so just ignoring the issue seems like
the wrong thing to do.

we could reenable the checks by moving out of the JIT fast path in pcre so
that pcre2_match is used for all matches (with JIT compiled as an optimization)
and that might have the added benefit of solving the PCRE errors when JIT is
broken[1]

$ git version
git version 2.22.0
$ git grep "Nguyễn Thái"
.mailmap:Nguyễn Thái Ngọc Duy [off-list ref]
po/TEAMS:Members:       Nguyễn Thái Ngọc Duy <pclouds AT gmail.com>
po/vi.po:# Nguyễn Thái Ngọc Duy [off-list ref], 2012.
t/t1302-repo-version.sh:# Copyright (c) 2007 Nguyễn Thái Ngọc Duy
t/t2104-update-index-skip-worktree.sh:# Copyright (c) 2008 Nguyễn Thái Ngọc Duy
t/t7011-skip-worktree-reading.sh:# Copyright (c) 2008 Nguyễn Thái Ngọc Duy
t/t7012-skip-worktree-writing.sh:# Copyright (c) 2008 Nguyễn Thái Ngọc Duy

important to note that at least on my system (macOS 10.14.6) the output of all
engines for grep is the same for a UTF-8 pattern match, even if using invalid
UTF-8 in the corpus, and I would expect that to be a better indicative
of correctness

FWIW GNU grep (with -P) also ignores UTF-8 errors using the same flag and
even PCRE seems to be going in the other direction with the recent addition
of PCRE2_MATCH_INVALID_UTF[1]

$ od -b int.p
0000000   102 145 154 303 263 156 012 303
$ pcre2grep -U 'Beló' int.p
Belón

Carlo

[1] https://public-inbox.org/git/20181209230024.43444-1-carenas@gmail.com/
[2] https://lists.exim.org/lurker/message/20190528.141422.2af1e386.gl.html