Skip to content

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

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

Closed
wants to merge 1 commit into from

Conversation

hughmcmaster
Copy link
Contributor

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.

@hughmcmaster
Copy link
Contributor Author

@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

@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

@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.

The developers of icu recommend using pkg-config to detect icu,
because icu-config is deprecated.
@hughmcmaster
Copy link
Contributor Author

@nikic I've updated #3701 as discussed.

@nikic
Copy link
Member

nikic commented Dec 9, 2018

Merged as 20fa2e7. Thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants