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

migrate pcre #10491

Closed
andyli opened this issue Nov 18, 2021 · 15 comments · Fixed by #11032
Closed

migrate pcre #10491

andyli opened this issue Nov 18, 2021 · 15 comments · Fixed by #11032
Milestone

Comments

@andyli
Copy link
Member

andyli commented Nov 18, 2021

See https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1000117

@Simn
Copy link
Member

Simn commented Nov 23, 2021

I'm guessing this is more complicated than just changing libpcre3 to libpcre2 in some places?

@andyli
Copy link
Member Author

andyli commented Nov 26, 2021

I've heard that the APIs are different, so prepare to have some code changes.

@Simn
Copy link
Member

Simn commented Nov 26, 2021

At times like this I realize that GitHub needs a crying emoji...

@andyli
Copy link
Member Author

andyli commented Nov 26, 2021

I think the best info is (will be) over PCRE2Project/pcre2#51

@RealyUniqueName
Copy link
Member

Recently I did this for mac 62ee6a9
And it works.

@andyli
Copy link
Member Author

andyli commented Nov 26, 2021

Recently I did this for mac 62ee6a9 And it works.

hmm... Shouldn't pcre2 uses pcre2.h instead of pcre.h?
How come it compiles...

@RealyUniqueName
Copy link
Member

Maybe the worker has it pre-installed now and our manual compilation of pcre is obsolete?

@andyli
Copy link
Member Author

andyli commented Nov 26, 2021

Maybe the worker has it pre-installed now and our manual compilation of pcre is obsolete?

Probably. Then it is still not using pcre2.

Should also check whether the haxe binary dynamic depends on pcre or not. I think we build and install our own pcre because we want to static link it.

@tobil4sk
Copy link
Member

tobil4sk commented Apr 23, 2022

Compared to Neko and Hashlink, migrating eval to pcre2 is a bit more involved. The current ocaml pcre library used by haxe is just a copy of: https://github.com/mmottl/pcre-ocaml (besides minor edits, most of which were lost when updating).

Unfortunately, there does not seem to be a pcre2 equivalent of this library for ocaml yet. See mmottl/pcre-ocaml#25.

Unless such a library is released, if we were to do this ourselves it would require writing:

@Uzume
Copy link

Uzume commented May 9, 2022

@tobil4sk Since a comprehensive pcre2 ocaml binding does not seem to be forthcoming, we could build our own with ctypes. The down side to this is that we would have to write our own binding but the up side is we would only need to flesh out enough of it to make our haxe eval port work instead of trying to make a comprehensive ocaml binding.

@Simn
Copy link
Member

Simn commented May 9, 2022

Indeed, while writing OCaml bindings isn't much fun, we're only using like 5 or so PCRE functions for which we would need bindings.

@Uzume
Copy link

Uzume commented May 10, 2022

* a pcre2 equivalent for https://opam.ocaml.org/packages/conf-libpcre/ (the easy bit)

Well, I submitted ocaml/opam-repository#21349, it was merged and we now at least have conf-libpcre2-8 and should be able to build some sort of binding on top of it.

@Uzume
Copy link

Uzume commented May 14, 2022

@RealyUniqueName:

Recently I did this for mac 62ee6a9 And it works.

I do not think this actually does work. And I am not sure 56bb846 was the right thing to do either. If we needed mbedtls why was ac399e2 done? Are we trying to move away from homebrew for macos? I notice we are still depending on: conf-libpcre, conf-zlib and conf-neko, each of which specifies homebrew as the system installation source for macos (despite using the HaxeFoundation build of neko and building our own libz during our own build process for ci and binary releases, etc.).

hmm... Shouldn't pcre2 uses pcre2.h instead of pcre.h? How come it compiles...

Yes, we should be and the library should be changed from -lpcre, libpcre-1.dll and /usr/local/lib/libpcre.a to -lpcre2-8, libpcre2-8-0.dll and /usr/local/lib/libpcre2-8.a, etc. But that will only break the build until the code is ported to the pcre2 API specified in pcre2.h.

Maybe the worker has it pre-installed now and our manual compilation of pcre is obsolete?

Probably for two reasons. One, maybe we ran the tests earlier and the lua tests install pcre (not pcre2) including from homebrew on macos here. opam users (I notice opam is way behind for haxe) will fail upon install because we depend on conf-libpcre (see above).

Probably. Then it is still not using pcre2.

Should also check whether the haxe binary dynamic depends on pcre or not. I think we build and install our own pcre because we want to static link it.

Exactly, and building pcre2 during the haxe build just wastes time during the build because it is not actually used. We should be using a system install of that anyway and I recommend using homebrew as we tell our users to do in the build instructions,

@Uzume
Copy link

Uzume commented May 14, 2022

@tobil4sk: as per my recent comments, you can probably tell I am have been poking about to understand how haxe uses pcre in order to understand how to port to pcre2. There are at least two other uses of pcre: php and lua targets.

I am not sure we can do anything about php. We directly use the binding in php itself which has already changed in php 7.3. Users have noticed a few minor functional changes but apparently the php API stable despite the changes under the hood.

As for the lua target, that might prove pretty easy (but I need to study the interfaces a bit more). Apparently haxe's main use for pcre is to implement a EReg interface and for lua we use lrexlib-pcre. lrexlib already has a binding for pcre2 so maybe we can just switch to lrexlib-pcre2 if the interfaces are similar enough (see manual). I might have a go at that before considering tackling haxe eval.

EDIT: Okay, this is a pain in the butt. It seems our lua test infrastructure (see tests/runci/targets/Lua.hx) likely has not been updated in quite a while. It properly system installs libpcre3-dev via apt on Linux and pcre via brew on macos (I am not sure what it does for Windows; I can only assume that is not tested at all). Then it installs lua and luarocks via python hererocks. So far so good, but then it installs the lua rocks haxe-deps and luasec. luasec isn't so bad but haxe-deps is empty save requiring other dependencies, among them being lrexlib-pcre (yay, we found it). It appears haxe-deps was created to help users install all the dependencies needed for haxe lua but it seems quite outdated despite HaxeFoundation/haxe-deps#1, an unmerged pull request for updates by @Aurel300 since 2021-09-10. So it is out-of-date, not taking updates and does not include luasec (which we are installing separately). I am not sure how to subsume a luarock and I am not sure I really want to fork that so perhaps I should just consider dropping that in lieu directly installing all the dependencies in the test script but we might want to document this somewhere for user support.

EDIT: Thankfully this has now been straightened out (for now) by bypassing haxe-deps with #10916.

Of course php and lua are source targets and as such are not direct targets of haxe itself but rather the code it generates. So from our perspective, that should only change dependencies here for tests. php takes care of itself (you cannot install php without either pcre or prec2 for 7.3+ being installed) but we will have to update the lua target tests here to specify pcre2 system install vs. pcre previously to account for the move from lrexlib-pcre to lrexlib-pcre2.

Sadly, it appears pcre-ocaml (which our eval binding is based upon) appears to export many lower level details of pcre (such as the study interface that disappears in pcre2; see API doc). I doubt we use that much of it to implement EReg though. We might be able to get by with a our own minimized update of the binding as suggested earlier.

justin-espedal added a commit to justin-espedal/haxe that referenced this issue Aug 21, 2022
Until the pcre2 migration is actually done, only build pcre.

Context: HaxeFoundation#10491
justin-espedal added a commit to justin-espedal/haxe that referenced this issue Nov 16, 2022
Until the pcre2 migration is actually done, only build pcre.

Context: HaxeFoundation#10491
Simn pushed a commit that referenced this issue Nov 16, 2022
* Differentiate PCRE2 from PCRE, fix missed version substitution

* cd back out of source folders after building dependencies

* Build pcre from source to fix macOS 10.13 compatibility

pcre is linked into Haxe, so it's important that it honors our macOS
deployment target.

43df805 replaced pcre with pcre2. This
commit brings pcre back so both are built side by side.

This fixes #10723

* Update pcre download links

From pcre.org:

> Note that the former ftp.pcre.org FTP site is no longer available.

and

> You can also download PCRE2 or the older, unmaintained PCRE library
> from an unofficial mirror at SourceForge:
>
> https://sourceforge.net/projects/pcre/files/

* Remove unused pcre2 build

Until the pcre2 migration is actually done, only build pcre.

Context: #10491
@Simn Simn modified the milestones: Release 4.3, Later Mar 24, 2023
@Uzume
Copy link

Uzume commented Mar 25, 2023

Now with #11030 and #11032 merged hxcpp should be only remaining thing left to reference PCRE1.

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 a pull request may close this issue.

5 participants