Message ID | 20190825182223.76288-2-carenas@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | PCRE1 cleanup | expand |
Carlo Marcelo Arenas Belón <carenas@gmail.com> writes: > e87de7cab4 ("grep: un-break building with PCRE < 8.32", 2017-05-25) > added a restriction for JIT support that is no longer needed after > pcre_jit_exec() calls were removed. I was initially puzzled by this statement, until I realized that the removal of pcre_jit_exec() happens in the topic still in flight that this patch builds on top of, namely 685668fa ("grep: stop using a custom JIT stack with PCRE v1", 2019-07-26). So the logic is that because we do no longer call pcre_jit_exec() that weren't available between 8.20 and 8.32, these slightly older versions can now do JIT just like the ones post 8.32? Thanks. Queued.
On Mon, Aug 26, 2019 at 11:54 AM Junio C Hamano <gitster@pobox.com> wrote: > > Carlo Marcelo Arenas Belón <carenas@gmail.com> writes: > > > e87de7cab4 ("grep: un-break building with PCRE < 8.32", 2017-05-25) > > added a restriction for JIT support that is no longer needed after > > pcre_jit_exec() calls were removed. > > I was initially puzzled by this statement, until I realized that the > removal of pcre_jit_exec() happens in the topic still in flight that > this patch builds on top of, namely 685668fa ("grep: stop using a > custom JIT stack with PCRE v1", 2019-07-26). sorry about that, I thought I had mentioned it in the cover letter (since the hash is likely to change and so is not fit for a commit message) but it is not there either. how could this be tracked more effectively? > So the logic is that because we do no longer call pcre_jit_exec() > that weren't available between 8.20 and 8.32, these slightly older > versions can now do JIT just like the ones post 8.32? exactly; but also because it is no longer using the JIT fast path which skipped UTF-8 validation, will need a way to disable that or risk a regression as I mentioned in [1] was planning in proposing a fix for PCRE1 based on [2] but wasn't sure if it could be part of this series, an independent one that is also based on ab/pcre-jit-fixes, and like this one, is mostly a consequence of 685668fa ("grep: stop using a custom JIT stack with PCRE v1", 2019-07-26) or something else, specially considering that Ævar dismissed it as a non issue in his commit message. Carlo [1] https://public-inbox.org/git/CAPUEspj4BJLjXorUXMiZnFtNcmhym_2QL5GUqeaGaCoxk=zjtw@mail.gmail.com/T/#m6acc8f68c398951457da469530bafa7e18811366 [2] https://public-inbox.org/git/20190721183115.14985-1-carenas@gmail.com/
diff --git a/Makefile b/Makefile index f58bf14c7b..3f78ef942f 100644 --- a/Makefile +++ b/Makefile @@ -34,13 +34,8 @@ all:: # library. Support for version 1 will likely be removed in some future # release of Git, as upstream has all but abandoned it. # -# When using USE_LIBPCRE1, define NO_LIBPCRE1_JIT if the PCRE v1 -# library is compiled without --enable-jit. We will auto-detect -# whether the version of the PCRE v1 library in use has JIT support at -# all, but we unfortunately can't auto-detect whether JIT support -# hasn't been compiled in in an otherwise JIT-supporting version. If -# you have link-time errors about a missing `pcre_jit_exec` define -# this, or recompile PCRE v1 with --enable-jit. +# When using USE_LIBPCRE1, define NO_LIBPCRE1_JIT if you want to +# disable JIT even if supported by your library. # # Define LIBPCREDIR=/foo/bar if your PCRE header and library files are # in /foo/bar/include and /foo/bar/lib directories. Which version of diff --git a/grep.h b/grep.h index c0c71eb4a9..1a044c501e 100644 --- a/grep.h +++ b/grep.h @@ -3,14 +3,12 @@ #include "color.h" #ifdef USE_LIBPCRE1 #include <pcre.h> -#ifdef PCRE_CONFIG_JIT -#if PCRE_MAJOR >= 8 && PCRE_MINOR >= 32 #ifndef NO_LIBPCRE1_JIT +#ifdef PCRE_CONFIG_JIT #define GIT_PCRE1_USE_JIT #define GIT_PCRE_STUDY_JIT_COMPILE PCRE_STUDY_JIT_COMPILE #endif #endif -#endif #ifndef GIT_PCRE_STUDY_JIT_COMPILE #define GIT_PCRE_STUDY_JIT_COMPILE 0 #endif
e87de7cab4 ("grep: un-break building with PCRE < 8.32", 2017-05-25) added a restriction for JIT support that is no longer needed after pcre_jit_exec() calls were removed. Reorganize the definitions in grep.h so that JIT support could be detected early and NO_LIBPCRE1_JIT could be used reliably to enforce JIT doesn't get used. Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> --- Makefile | 9 ++------- grep.h | 4 +--- 2 files changed, 3 insertions(+), 10 deletions(-)