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

community/php7-pecl-oauth: Fix build against PHP 7.3 #5905

Closed
wants to merge 1 commit into from
Closed

community/php7-pecl-oauth: Fix build against PHP 7.3 #5905

wants to merge 1 commit into from

Conversation

bratkartoffel
Copy link
Contributor

Relates to #5863

@andypost andypost added S-hold Must not be merged now for some reason A-upgrade Upgrades an abuild C-php labels Dec 24, 2018
@stale
Copy link

stale bot commented Mar 3, 2019

This pull request has been automatically marked as stale because it has not had activity in the last 60 days. It will be automatically closed if no further activity occurs in the next 14 days. If you are still waiting for a review or response from one of our developers you can leave a comment to remove the stale label. If you know the username of the maintainer of the aport you could try to mention the username in the comment. You can also join our IRC channels on freenode and see if you can find an available developer. Thank you for your contributions.

@stale stale bot added the S-stale label Mar 3, 2019
@andypost
Copy link
Contributor

andypost commented Mar 3, 2019

Still needs to hold

@stale
Copy link

stale bot commented Mar 17, 2019

This pull request has been automatically closed.

@stale stale bot closed this Mar 17, 2019
@bratkartoffel
Copy link
Contributor Author

Please reopen @andypost

@TBK
Copy link
Contributor

TBK commented May 23, 2019

@andypost is this ready to be merged?

@probot-autolabeler probot-autolabeler bot added the R-community Community repository label May 24, 2019
@andypost
Copy link
Contributor

@bratkartoffel please add re2c as makedepends because

configure: WARNING: You will need re2c 0.13.4 or later if you want to regenerate PHP parsers.

@TBK it looks like could be merged but as I see in build log the resulting binary does not depend on pcre2 (which is php 7.3 requirements which waiting 3.10 release)

@bratkartoffel
Copy link
Contributor Author

@andypost I've added re2c

@andypost
Copy link
Contributor

LGTG except the real requirement
http://git.php.net/?p=pecl/web_services/oauth.git;a=blob;f=INSTALL;h=86eeeeb0d7491f935177a8d09975e37677f6d4c3;hb=HEAD tells about libpcre3 but I see no mention in build log about it (the same about pcre2) so not clear is there dependency at all

@andypost
Copy link
Contributor

In buildlog

checking pcre.h usability... yes
checking pcre.h presence... yes
checking for pcre.h... yes 

but both pcre-dev and pcre2-dev added by dependencies...

@TBK
Copy link
Contributor

TBK commented May 31, 2019

@andypost The last commit for INSTALL was 2011-01-15 http://git.php.net/?p=pecl/web_services/oauth.git;a=commit;h=4b3af65373fbd4de1dbc41a4a01442294e73f68c .

The renaming of PCRE to PCRE2 was in 2015 https://lists.exim.org/lurker/message/20150105.162835.0666407a.en.html

The only is a mention of PCRE3 I can find is on Debian related distro sites and it redirect to pcre2.

So I would say all requirement are fulfilled and declare the INSTALL files recommendations for outdated.

@andypost
Copy link
Contributor

@TBK looking at code http://git.php.net/?p=pecl/web_services/oauth.git;a=blob;f=php_oauth.h;h=a05ce2d82ae5536afb4c30cf720e3838d947f1cb;hb=HEAD#l45 it looks like pcre-dev just needs to be removed because it links to php internals which do switch in 7.3 to pcre2, so any of this dependencies are selected by php-dev package which depends on pcre-dev now https://pkgs.alpinelinux.org/package/edge/community/x86_64/php7-dev
But that will change in #5863

@bratkartoffel
Copy link
Contributor Author

outdated

@bratkartoffel bratkartoffel deleted the community/php7-pecl-oauth_fix-php-7.3 branch June 14, 2019 11:56
@TBK
Copy link
Contributor

TBK commented Jun 14, 2019

Merged with dd49d71

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-upgrade Upgrades an abuild C-php R-community Community repository S-hold Must not be merged now for some reason
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants