root icon indicating copy to clipboard operation
root copied to clipboard

Support PCRE2

Open ellert opened this issue 1 year ago • 7 comments

This Pull request:

Makes it possible to use PCRE2 as an alternative to PCRE

Checklist:

  • [x] tested changes locally
  • [ ] updated the docs (if necessary)

This PR fixes #11395

ellert avatar Oct 02 '23 05:10 ellert

What we (you, the ROOT team, the users) need to discuss is whether we can deprecate TPRegex altogether, given that std::regex is now (since C++17 is required, as a side effect of kicking out old compilers) guaranteed to be available. Or reimplement it using std:: regex.

Thoughts?

Axel-Naumann avatar Oct 02 '23 05:10 Axel-Naumann

Test Results

    10 files      10 suites   2d 4h 27m 22s :stopwatch:  2 636 tests  2 635 :white_check_mark: 0 :zzz: 1 :x: 25 131 runs  25 130 :white_check_mark: 0 :zzz: 1 :x:

For more details on these failures, see this check.

Results for commit f89fc640.

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar Oct 02 '23 14:10 github-actions[bot]

Some background: https://fedoraproject.org/wiki/Changes/PcreDeprecation https://bugzilla.redhat.com/show_bug.cgi?id=2128369

ellert avatar Oct 03 '23 06:10 ellert

pcre2 10.40 (as mentioned in #11395) already comes with a pcre2-config.cmake (https://github.com/PCRE2Project/pcre2/blob/pcre2-10.40/cmake/pcre2-config.cmake.in) file. Can that be used instead of adding FindPCRE2.cmake ?

andresailer avatar Oct 19 '23 08:10 andresailer

pcre2 10.40 (as mentioned in #11395) already comes with a pcre2-config.cmake (https://github.com/PCRE2Project/pcre2/blob/pcre2-10.40/cmake/pcre2-config.cmake.in) file. Can that be used instead of adding FindPCRE2.cmake ?

The packaged pcre2 in Fedora and RHEL/CentOS/Rocky/Alma does not ship the cmake files. They are only create/installed if pcre2 is built using cmake. The RPM for Fedora and RHEL are built using autotools, and therefore do not contain the cmake files.

ellert avatar Oct 19 '23 13:10 ellert

Not adding this to the 6.32 milestone, since even Fedora rawhide still has the old pcre for now.

guitargeek avatar May 21 '24 03:05 guitargeek

What we (you, the ROOT team, the users) need to discuss is whether we can deprecate TPRegex altogether, given that std::regex is now (since C++17 is required, as a side effect of kicking out old compilers) guaranteed to be available. Or reimplement it using std:: regex.

Thoughts?

I wouldn't go into this direction yet. We have a couple of problems with std::regexp, like these symbol clashes with other Python libraries: https://github.com/root-project/root/issues/15309

Therefore, provided that it's tested, I'm in favor of merging this PR. I'm adding pcre2 to the CI images, so that we can test it: https://github.com/root-project/root-ci-images/pull/31

guitargeek avatar May 21 '24 03:05 guitargeek

Potential regression: https://github.com/root-project/root/issues/15986

ferdymercury avatar Jul 04 '24 13:07 ferdymercury