Skip to content

[Bug #77241] ext/intl: Use pkg-config to detect icu#3701

Closed
hughmcmaster wants to merge 1 commit intophp:masterfrom
hughmcmaster:pkgconfig
Closed

[Bug #77241] ext/intl: Use pkg-config to detect icu#3701
hughmcmaster wants to merge 1 commit intophp:masterfrom
hughmcmaster:pkgconfig

Conversation

@hughmcmaster
Copy link
Contributor

@hughmcmaster hughmcmaster commented Dec 5, 2018

icu-config has been deprecated by its upstream developers for some years now. In icu 63.1, icu-config is optional to install, but is installed by default. In the next version, icu-config will no longer be installed by default.

The developers recommend downstream packages use pkg-config to detect the icu libraries.

Debian (and its derivatives, such as Ubuntu) will remove icu-config from its icu development package (libicu-dev) very soon. Hence, php needs to use pkg-config to detect icu.

@cmb69
Copy link
Member

cmb69 commented Dec 5, 2018

Closely related to PR #3654.

nikic
nikic reviewed Dec 5, 2018
View reviewed changes
nikic
nikic reviewed Dec 5, 2018
View reviewed changes
@hughmcmaster
Copy link
Contributor Author

hughmcmaster commented Dec 5, 2018

@nikic I've updated #3701. Thanks for the review.

@nikic
Copy link
Member

nikic commented Dec 5, 2018

LGTM. Only thing I'm uncertain about is whether we need to preserve the PHP_SETUP_ICU macro, so this is also usable by 3rd-party extension. I don't know if anything actually uses it, but it might be safer to keep it.

@hughmcmaster
Copy link
Contributor Author

hughmcmaster commented Dec 5, 2018

@nikic Thanks. Remember, though, that icu-config is deprecated and will be removed from icu in version 64. This means the PHP_SETUP_ICU macro will be redundant. Even now, it has a very limited lifespan.

That said, if you still want to keep the macro, I'll update the PR for this. Please let me know what you want to do.

@nikic
Copy link
Member

nikic commented Dec 5, 2018

@hughdavenport Yeah, we shouldn't keep the current implementation of the macro around, but it would be possible to move the new implementation into PHP_SETUP_ICU and then call PHP_SETUP_ICU from ext/intl, the way it was done previously.

@hughmcmaster
Copy link
Contributor Author

hughmcmaster commented Dec 5, 2018

@nikic That seems fair. I thought moving to ext/intl was a better option, because ext/gd does this for FreeType 2. Plus icu is used in ext/intl, so it has a closer relationship.

@hughmcmaster
ext/intl: Use pkg-config to detect icu
99cc551 
The developers of icu recommend using pkg-config to detect icu,
because icu-config is deprecated.
@hughmcmaster
Copy link
Contributor Author

hughmcmaster commented Dec 5, 2018

@nikic I've updated #3701 as discussed.

@nikic
Copy link
Member

nikic commented Dec 9, 2018

Merged as 20fa2e7. Thanks a lot!

@nikic nikic closed this Dec 9, 2018
This was referenced Jul 25, 2019
Merged
Closed
inode64 added a commit to inode64/php-src-backports that referenced this pull request Apr 24, 2025
@inode64
Based on the following upstream commits:
d03537b 
php/php-src@9a8e7b5
php/php-src#3701
php/php-src@2bd299f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nikic nikic nikic approved these changes

Assignees

No one assigned

Labels

Category: Build System

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@hughmcmaster @cmb69 @nikic @petk