Skip to content

ext/intl: Guard Spoofchecker restriction-level APIs at ICU 53 (fix build on ICU 50.x)#22248

Closed
GrahamCampbell wants to merge 5 commits into
php:PHP-8.4from
GrahamCampbell:intl-spoofchecker-icu53-guard
Closed

ext/intl: Guard Spoofchecker restriction-level APIs at ICU 53 (fix build on ICU 50.x)#22248
GrahamCampbell wants to merge 5 commits into
php:PHP-8.4from
GrahamCampbell:intl-spoofchecker-icu53-guard

Conversation

@GrahamCampbell

@GrahamCampbell GrahamCampbell commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

PHP 8.4.22 fails to compile the intl extension when the system ICU is older than 53 — on Amazon Linux 2 (system ICU 50.2) the build breaks with USPOOF_ASCII, the other Spoofchecker restriction-level constants, and URestrictionLevel undeclared. The cause is GH-22055 (fixing #22053), which removed the #if U_ICU_VERSION_MAJOR_NUM >= 58 guards around the Spoofchecker restriction-level constants and the setRestrictionLevel() method, so they are now referenced unconditionally.

This is fine on master and PHP-8.5 (icu-uc >= 57.1) but not on PHP-8.4, which still accepts icu-uc >= 50.1. The symbols have existed since ICU 51 except USPOOF_SINGLE_SCRIPT_RESTRICTIVE (ICU 53, used in the setRestrictionLevel() body); since 50.2 satisfies the 50.1 minimum, configure passes and only the compile fails. ICU 53 is therefore the right floor: guarding at >= 53 builds on every ICU PHP-8.4 supports while still exposing the API on ICU 53+ — which is what GH-22055 wanted, since its >= 58 guard was too strict and hid the API on ICU 57.x.

The fix restores the guard at #if U_ICU_VERSION_MAJOR_NUM >= 53 around the constants and setRestrictionLevel() in spoofchecker.stub.php (regenerating spoofchecker_arginfo.h) and around the setRestrictionLevel() body in spoofchecker_main.c. The unrelated setAllowedChars() message change is kept.

@GrahamCampbell GrahamCampbell force-pushed the intl-spoofchecker-icu53-guard branch from f814198 to 384a146 Compare June 7, 2026 19:50
@devnexen devnexen requested a review from LamentXU123 June 7, 2026 20:11

@LamentXU123 LamentXU123 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed correct analysis. I wasn't aware PHP 8.4 has a different minimum ICU version than 8.5 and 8.6. Thank you for this PR!

@LamentXU123

LamentXU123 commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

@GrahamCampbell We may need to add this into the PHP 8.4 NEWS file to notice others that we fix this.
This should only be merged into 8.4 so I will create 2 blank commit on upper branches (i.e. 8.5 and master)

@LamentXU123 LamentXU123 self-assigned this Jun 8, 2026
@GrahamCampbell

Copy link
Copy Markdown
Contributor Author

Added a news entry.

@LamentXU123

Copy link
Copy Markdown
Contributor

@GrahamCampbell Thank you. The PR looks good and we will merge it if @devnexen has no opinions :)

@devnexen

devnexen commented Jun 8, 2026

Copy link
Copy Markdown
Member

none I just wanted to see how you do :)

Comment thread ext/intl/spoofchecker/spoofchecker.stub.php
Comment thread ext/intl/tests/spoofchecker_007.phpt Outdated
Comment thread ext/intl/tests/spoofchecker_supported_icu57_apis.phpt Outdated
Comment thread ext/intl/tests/spoofchecker_unknown_restriction_level.phpt Outdated
GrahamCampbell and others added 3 commits June 9, 2026 23:01
Co-authored-by: Weilin Du <1372449351@qq.com>
Co-authored-by: Weilin Du <1372449351@qq.com>
Co-authored-by: Weilin Du <1372449351@qq.com>
@LamentXU123

LamentXU123 commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Thanks!
(Crap. I forgot to add the author header in the commit... Sorry. But given the NEWS file the authorship is actually indicated. Lesson learnt :) )

@GrahamCampbell

Copy link
Copy Markdown
Contributor Author

No worries. Thanks for the quick review.

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.

3 participants