Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[eval] Migrate to pcre2 #11032

Merged
merged 7 commits into from
Mar 25, 2023
Merged

[eval] Migrate to pcre2 #11032

merged 7 commits into from
Mar 25, 2023

Conversation

tobil4sk
Copy link
Member

Closes #10491

I rewrote the existing ocaml pcre bindings for pcre2. I also uploaded them here: https://github.com/tobil4sk/pcre2-ocaml, but the version in this PR is slightly modified for compatibility with the OCaml version used by Haxe.

This addresses the null byte in regex bug as well: #10592

@Simn Simn added this to the Release 4.3 milestone Mar 25, 2023
@Simn
Copy link
Member

Simn commented Mar 25, 2023

Oh shit, CI is actually green...! I'll quickly merge this before it changes its mind.

Thank you so much, this has certainly been on my personal "Oh God not that problem"-backlog.

@Simn Simn merged commit 962ccad into HaxeFoundation:development Mar 25, 2023
@tobil4sk tobil4sk deleted the pcre2 branch March 25, 2023 11:05
@Uzume
Copy link

Uzume commented Mar 25, 2023

@tobil4sk Great work! Are you going to look at hxcpp next?

Maybe I will try to take tobil4sk/pcre2-ocaml and work it into a PR at mmottl/pcre-ocaml or something and try to get it introduced into opam. There are many dependencies on opam - pcre that would do well to be ported to pcre2. I know many distributions (like Debian and Ubuntu, etc.) that want to drop pcre1 packages (like the confusingly named pcre3) but there is still too much software that depends on them and this might go a long ways towards helping that situation.

@tobil4sk
Copy link
Member Author

Are you going to look at hxcpp next?

@Uzume Yes, I'll make a PR soon. I've ported our code, but there are still some building issues that have to be resolved.

Maybe I will try to take tobil4sk/pcre2-ocaml and work it into a PR at mmottl/pcre-ocaml

The author of pcre-ocaml has said he thinks pcre2-ocaml should be kept separate, but we can upload prce2 as a new opam package. There are still a few things that are missing (namely jit compilation) which we didn't need for Haxe but which would have to be implemented before the library is ready more generally. If you like, feel free to open an issue in pcre2-ocaml and we can discuss further over there.

@Uzume
Copy link

Uzume commented Mar 25, 2023

The author of pcre-ocaml has said he thinks pcre2-ocaml should be kept separate, but we can upload prce2 as a new opam package. There are still a few things that are missing (namely jit compilation) which we didn't need for Haxe but which would have to be implemented before the library is ready more generally. If you like, feel free to open an issue in pcre2-ocaml and we can discuss further over there.

@tobil4sk: Yes, I am aware of that. I was just thinking to create a branch in the original repo for pcre-ocaml and move the default master branch over to pcre2-ocaml (or something similar). It makes little sense to consider maintenance for these in entirely different repos when a separate branch should suffice.

This was referenced Mar 25, 2023
@RblSb
Copy link
Member

RblSb commented Mar 26, 2023

I don't think that brew install pcre2 from BUILDING.md is enough for macos setup, there is pcre2.h not found error anyway.
I fixed this for myself with manual pcre2 installation from this archive, and then doing configure/make from haxe CI instructions and chmoding everything in usr/local to make it install at any permission price (don't care much).

@tobil4sk
Copy link
Member Author

Hm, could be related to #11012

@skial skial mentioned this pull request Mar 29, 2023
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

migrate pcre
4 participants