Skip to content

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

Closed
hughmcmaster wants to merge 1 commit into
php:masterfrom
hughmcmaster:pkgconfig
Closed

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

Conversation

@hughmcmaster

Copy link
Copy Markdown
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

cmb69 commented Dec 5, 2018

Copy link
Copy Markdown
Member

Closely related to PR #3654.

Comment threadext/intl/config.m4 Outdated
Comment threadext/intl/config.m4 Outdated
@hughmcmaster

Copy link
Copy Markdown
ContributorAuthor

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

@nikic

nikic commented Dec 5, 2018

Copy link
Copy Markdown
Member

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
Copy Markdown
ContributorAuthor

@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

nikic commented Dec 5, 2018

Copy link
Copy Markdown
Member

@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
Copy Markdown
ContributorAuthor

@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
Copy Markdown
ContributorAuthor

@nikic I've updated #3701 as discussed.

@nikic

nikic commented Dec 9, 2018

Copy link
Copy Markdown
Member

Merged as 20fa2e7. Thanks a lot!

Sign up for freeto 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

@hughmcmaster@cmb69@nikic@petk