Welcome! Log In Create A New Profile

Advanced

[PHP-DEV] Lift ICU requirements

Posted by Christoph M. Becker 
Christoph M. Becker
[PHP-DEV] Lift ICU requirements
September 03, 2018 03:20PM
Hi!

ext/intl presently requires ICU ≥ 4.0 (ICU 4.0 has been released on
2008-07-02[1]). Since ICU is still under vivid development our code is
full of version checks (grep for ICU_VERSION), very old ICU versions are
unlikely to be still around widespreadly, and we're generally not doing
us a favor in supporting such old ICU versions (many PHPTs have multiple
variants for different ICU versions). Therefore I have submitted PR
#3487[2] which would raise the requirements to ICU ≥ 4.6 (ICU 4.6 has
been released on 2010-12-02[3]).

Anatol suggested to consider to lift the requirements even to ICU ≥ 50
(ICU 50.1 has been released on 2012-11-05[4]), or maybe even to ICU ≥ 52
(ICU 52.1 has been released on 2013-10-09[5]).

What do you think? Does this require an RFC?

[1] <http://icu-project.org/download/4.0.html#ICU4C>;
[2] https://github.com/php/php-src/pull/3487
[3] <http://site.icu-project.org/download/46#ICU4C-Download>;
[4] <http://site.icu-project.org/download/50#TOC-ICU4C-Download>;
[5] <http://site.icu-project.org/download/52#TOC-ICU4C-Download>;

--
Christoph M. Becker

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Remi Collet
Re: [PHP-DEV] Lift ICU requirements
September 03, 2018 04:00PM
Le 03/09/2018 à 15:12, Christoph M. Becker a écrit :
> Hi!
>
> ext/intl presently requires ICU ≥ 4.0 (ICU 4.0 has been released on
> 2008-07-02[1]). Since ICU is still under vivid development our code is
> full of version checks (grep for ICU_VERSION), very old ICU versions are
> unlikely to be still around widespreadly, and we're generally not doing
> us a favor in supporting such old ICU versions (many PHPTs have multiple
> variants for different ICU versions). Therefore I have submitted PR
> #3487[2] which would raise the requirements to ICU ≥ 4.6 (ICU 4.6 has
> been released on 2010-12-02[3]).
>
> Anatol suggested to consider to lift the requirements even to ICU ≥ 50
> (ICU 50.1 has been released on 2012-11-05[4]), or maybe even to ICU ≥ 52
> (ICU 52.1 has been released on 2013-10-09[5]).
>
> What do you think?

50.1 seems a good target (version in RHEL/CentOS 7)

> Does this require an RFC?

I don't think.

Remi

>
> [1] <http://icu-project.org/download/4.0.html#ICU4C>;
> [2] https://github.com/php/php-src/pull/3487
> [3] <http://site.icu-project.org/download/46#ICU4C-Download>;
> [4] <http://site.icu-project.org/download/50#TOC-ICU4C-Download>;
> [5] <http://site.icu-project.org/download/52#TOC-ICU4C-Download>;
>
Andrey Andreev
Re: [PHP-DEV] Lift ICU requirements
September 03, 2018 05:10PM
Hi,

I raised this issue back in January[1], but then got distracted and
completely forgot about it ... sorry about that.

Would the change target 7.3 or 7.4?

[1] https://externals.io/message/101626

Cheers,
Andrey.

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Christoph M. Becker
Re: [PHP-DEV] Lift ICU requirements
September 03, 2018 05:20PM
On 03.09.2018 at 17:02, Andrey Andreev wrote:

> I raised this issue back in January[1], but then got distracted and
> completely forgot about it ... sorry about that.

Thanks for having raised the issue. :)

> Would the change target 7.3 or 7.4?

IMO, we should target master (i.e. 7.4) only.

> [1] https://externals.io/message/101626

--
Christoph M. Becker

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Anatol Belski
RE: [PHP-DEV] Lift ICU requirements
September 03, 2018 07:40PM
Andrey Andreev
Re: [PHP-DEV] Lift ICU requirements
September 04, 2018 01:50PM
Hi,

On Mon, Sep 3, 2018 at 6:13 PM, Christoph M. Becker <[email protected]> wrote:
> On 03.09.2018 at 17:02, Andrey Andreev wrote:
>
>> I raised this issue back in January[1], but then got distracted and
>> completely forgot about it ... sorry about that.
>
> Thanks for having raised the issue. :)
>
>> Would the change target 7.3 or 7.4?
>
> IMO, we should target master (i.e. 7.4) only.
>
>> [1] https://externals.io/message/101626
>

Well, bummer. Now I feel real guilty about not acting on time,
especially since you yourself were fine with bumping to ICU 4.6 for
PHP 7.2.2 at the time.

I know we're past feature freeze, but can we maybe do 4.6 in 7.3 and
then 50 in 7.4? We already know ICU 4.6 works well and older versions
aren't widespread; it's not like it needs extensive evaluation.

Cheers,
Andrey.

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Christoph M. Becker
Re: [PHP-DEV] Lift ICU requirements
September 04, 2018 04:40PM
On 04.09.2018 at 13:44, Andrey Andreev wrote:

> On Mon, Sep 3, 2018 at 6:13 PM, Christoph M. Becker <[email protected]> wrote:
>
>> On 03.09.2018 at 17:02, Andrey Andreev wrote:
>>
>>> I raised this issue back in January[1], but then got distracted and
>>> completely forgot about it ... sorry about that.
>>
>> Thanks for having raised the issue. :)
>>
>>> Would the change target 7.3 or 7.4?
>>
>> IMO, we should target master (i.e. 7.4) only.
>>
>>> [1] https://externals.io/message/101626
>
> Well, bummer. Now I feel real guilty about not acting on time,
> especially since you yourself were fine with bumping to ICU 4.6 for
> PHP 7.2.2 at the time.
>
> I know we're past feature freeze, but can we maybe do 4.6 in 7.3 and
> then 50 in 7.4? We already know ICU 4.6 works well and older versions
> aren't widespread; it's not like it needs extensive evaluation.

I don't see a real issue here. If anybody using PHP 7.3 (or even 7.2)
still uses a very old ICU, than they get the deprecation warning. If
they change to INTL_IDNA_VARIANT_UTS46, they quickly notice that it
doesn't work, and likely look up in the PHP manual (where it should
already have been documented that INTL_IDNA_VARIANT_UTS46 required ICU ≥
4.6; I'll catch up on this soon), so they have the choice to upgrade
their ICU, or to stick with the deprecation warning.

Only for PHP 7.4 it is important to require at least ICU 4.6, since
otherwise changing the default of the $variant parameter would not be
possible.

--
Christoph M. Becker

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Sara Golemon
Re: [PHP-DEV] Lift ICU requirements
September 04, 2018 06:30PM
On Tue, Sep 4, 2018 at 6:44 AM, Andrey Andreev <[email protected]> wrote:
> On Mon, Sep 3, 2018 at 6:13 PM, Christoph M. Becker <[email protected]> wrote:
>> On 03.09.2018 at 17:02, Andrey Andreev wrote:
>>
>>> I raised this issue back in January[1], but then got distracted and
>>> completely forgot about it ... sorry about that.
>>
>> Thanks for having raised the issue. :)
>
Yeah, I recall that discussion and I don't recall any strong
objections. +1 to lifting the bar. If you're using latest PHP, then
you can be expected to have reasonably-recent ICU.

>>> Would the change target 7.3 or 7.4?
>>
>> IMO, we should target master (i.e. 7.4) only.
>>
>
Concur. Feature freeze means feature freeze. Nothing stops anyone
from using a more recent ICU with 7.3. There's no urgency
justification for overriding FF here.

> I know we're past feature freeze, but can we maybe do 4.6 in 7.3 and
> then 50 in 7.4? We already know ICU 4.6 works well and older versions
> aren't widespread; it's not like it needs extensive evaluation.
>
No. See above. There is no urgency justification for it.

-Sara

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Andrey Andreev
Re: [PHP-DEV] Lift ICU requirements
September 04, 2018 06:40PM
Hi,

On Tue, Sep 4, 2018 at 7:22 PM, Sara Golemon <[email protected]> wrote:
> On Tue, Sep 4, 2018 at 6:44 AM, Andrey Andreev <[email protected]> wrote:
>> On Mon, Sep 3, 2018 at 6:13 PM, Christoph M. Becker <[email protected]> wrote:
>>> On 03.09.2018 at 17:02, Andrey Andreev wrote:
>>>
>>>> I raised this issue back in January[1], but then got distracted and
>>>> completely forgot about it ... sorry about that.
>>>
>>> Thanks for having raised the issue. :)
>>
> Yeah, I recall that discussion and I don't recall any strong
> objections. +1 to lifting the bar. If you're using latest PHP, then
> you can be expected to have reasonably-recent ICU.
>
>>>> Would the change target 7.3 or 7.4?
>>>
>>> IMO, we should target master (i.e. 7.4) only.
>>>
>>
> Concur. Feature freeze means feature freeze. Nothing stops anyone
> from using a more recent ICU with 7.3. There's no urgency
> justification for overriding FF here.
>
>> I know we're past feature freeze, but can we maybe do 4.6 in 7.3 and
>> then 50 in 7.4? We already know ICU 4.6 works well and older versions
>> aren't widespread; it's not like it needs extensive evaluation.
>>
> No. See above. There is no urgency justification for it.
>

Right ... I wasn't saying it's urgent, just wasn't sure whether FF
rule applies. If it does, I'm not trying to bend it.

Cheers,
Andrey.

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Christoph M. Becker
[PHP-DEV] Re: Lift ICU requirements
September 08, 2018 02:10PM
On 03.09.2018 at 15:12, Christoph M. Becker wrote:

> ext/intl presently requires ICU ≥ 4.0 (ICU 4.0 has been released on
> 2008-07-02[1]). Since ICU is still under vivid development our code is
> full of version checks (grep for ICU_VERSION), very old ICU versions are
> unlikely to be still around widespreadly, and we're generally not doing
> us a favor in supporting such old ICU versions (many PHPTs have multiple
> variants for different ICU versions). Therefore I have submitted PR
> #3487[2] which would raise the requirements to ICU ≥ 4.6 (ICU 4.6 has
> been released on 2010-12-02[3]).
>
> Anatol suggested to consider to lift the requirements even to ICU ≥ 50
> (ICU 50.1 has been released on 2012-11-05[4]), or maybe even to ICU ≥ 52
> (ICU 52.1 has been released on 2013-10-09[5]).
>
> What do you think? Does this require an RFC?
>
> [1] <http://icu-project.org/download/4.0.html#ICU4C>;
> [2] https://github.com/php/php-src/pull/3487
> [3] <http://site.icu-project.org/download/46#ICU4C-Download>;
> [4] <http://site.icu-project.org/download/50#TOC-ICU4C-Download>;
> [5] <http://site.icu-project.org/download/52#TOC-ICU4C-Download>;

It appears there is consensus on requiring ICU ≥ 50.1 for master (PHP
7.4). Unless there are objections, I'll merge PR #3487 (which I've
already amended for ICU 50.1) on the next weekend (i.e. in roughly a week).

--
Christoph M. Becker

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Sorry, only registered users may post in this forum.

Click here to login