Skip to content

Bump Regex extension to PCRE2 - #2570

Open
EricZhang456 wants to merge 17 commits into
alliedmodders:masterfrom
EricZhang456:pcre2
Open

EricZhang456 wants to merge 17 commits into
alliedmodders:masterfrom
EricZhang456:pcre2

Conversation

@EricZhang456

@EricZhang456 EricZhang456 commented Sep 11, 2026

Copy link
Copy Markdown
Member

For obvious reasons I didn't check in the .lib files I used for development. But I left instructions on how I built the libs in the README. I'd imagine PCRE2 needs to be handled like MariaDB where the repo is checked into the AM organization and libs are built with CI.

Tested it with some simple regex patterns under 64-bit TF2 on Windows

@dvander

dvander commented Sep 11, 2026

Copy link
Copy Markdown
Member

PCRE is probably simple enough it could be built as part of the build?

@EricZhang456

EricZhang456 commented Sep 11, 2026

Copy link
Copy Markdown
Member Author

PCRE is probably simple enough it could be built as part of the build?

Done. Looked at NON-AUTOTOOLS-BUILD for this.

@EricZhang456

Copy link
Copy Markdown
Member Author

Is it okay to symlink? I am very tempted to symlink pcre2.h from the pcre2 source tree to extension root.

@dvander

dvander commented Sep 11, 2026

Copy link
Copy Markdown
Member

as long as it works on Windows, seems fine to me.

re: pcre, putting the code here is fine, I'm supportive of this PR as-is. But for newer projects we have a "third_party" directory at the top level with git submodules underneath. It makes these kinds of changes way easier. Then we add something like "pcre.AMBuild" in third_party. Of course that only works if you can import it without any modifications. If you need modifications we have to fork the repository and import the fork instead.

Comment thread extensions/regex/CRegEx.cpp Outdated
@EricZhang456

EricZhang456 commented Sep 11, 2026

Copy link
Copy Markdown
Member Author

I am not going to put any symlinks since not all applications on Windows understands symlink, and when core.symlinks is set to false git will just checkout a text file with the path to the original file which isn't very helpful

@EricZhang456

EricZhang456 commented Sep 15, 2026

Copy link
Copy Markdown
Member Author

@dvander: re: pcre, putting the code here is fine, I'm supportive of this PR as-is. But for newer projects we have a "third_party" directory at the top level with git submodules underneath. It makes these kinds of changes way easier. Then we add something like "pcre.AMBuild" in third_party. Of course that only works if you can import it without any modifications. If you need modifications we have to fork the repository and import the fork instead.

Are we still worried about third-party submodules? I can do that in this PR and might do the same for Stripper if it's okay to put third-party submodules.

@dvander

dvander commented Sep 15, 2026

Copy link
Copy Markdown
Member

For a major well supported library, where we don't need custom modifications and can pin a SHA, it's fine. If we need source modifications or the library is questionable in terms of long term survival... Then I can make a 1p fork for you.

@EricZhang456

Copy link
Copy Markdown
Member Author

For a major well supported library, where we don't need custom modifications and can pin a SHA, it's fine. If we need source modifications or the library is questionable in terms of long term survival... Then I can make a 1p fork for you.

Don't really need source modifications, but I need to rename a couple of files. Copying them to the build folder under the new name should be fine right?

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.

2 participants