-
Notifications
You must be signed in to change notification settings - Fork 695
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
add libpcre2 support #2543
add libpcre2 support #2543
Conversation
I've played with the following configuration file and it seems to behave as expected.
|
Thanks for the PR! Unfortunately we cannot remove pcre implementation and we need to support both to avoid breaking people deployments. |
pcre is not maintained anymore, and being removed in distributions, that's why I did not work on all those |
But it's still available in the LTS distributions people is using now and uwsgi has been built against. |
pcre2 is also available in LTS distributions. Anyway I'll update this PR with fixes as I get feedback (this just got integrated in Debian). I cannot commit time to update to support both but will hapilly help (answer questions) someone who would. |
@niol could you please cherry-pick and rebase on top of xrmx@7835662 so at least we avoid conflicts? |
I know it's available, what I'm saying I don't like to break the build of the majority of uwsgi users that is pip installing uwsgi and not using uwsgi from the distribution. |
@xrmx is this what you had in mind for retaining pcre1 support https://gist.github.com/loqs/f79a4095875defb294f6617ba06dcd41 |
@loqs i thought more of two alternative definition (UWSGI_PCRE vs UWSGI_PCRE2) but that's fine too |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please ignore the formatting comments if they are too pedantic.
https://gist.github.com/loqs/0aa5f478b0c684c935b0b4f202298aa6 with two alternative definitions. |
Any more comments on this? |
I'll try to give a review this afternoon |
Kind reminder, what needs to be done to have this merged, please? |
There's one more moment to adjust for pcre2: https://github.com/rockdrilla/sentry-hacks/blob/960d41476665c48ccbe0f87d7a47481ec556476d/patches/uwsgi/1500-libpcre2.patch |
I would really like pcre2 support merged in uwsgi. We are currently doing bundling of .so files with auditwheel because pcre is not even installed by default in most if not all latest distros.
|
ee6bb72
to
0d31786
Compare
got bit by this (silently because pip hides the build output by default, it seems) and it took me a long time to figure out why the "Expires" settings for static files was not being set. So a kind +1 from me. TIA |
@xrmx friendly ping. Could we merge this and get a release out? pcre is going to be dropped sooner or later from all major distros. |
Squashed and merged to master in #2624. Thanks! |
@niol just noticed that when squashing I erased your authorship, sorry about that. Will fix for 2.0 backport at least. |
@xrmx could we please get a new release including this? |
I've started the work. It compiles.
I need to test if it works but wanted to share to avoid somebody else wasting time doing the same.
This would fix #2486