Lift ICU requirements

6 years ago by Christoph M. Becker — view source

unread

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

6 years ago by Remi Collet — view source

unread

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

6 years ago by Andrey Andreev — view source

unread

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.

6 years ago by Christoph M. Becker — view source

unread

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

6 years ago by Andrey Andreev — view source

unread

Hi,

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.

6 years ago by Christoph M. Becker — view source

unread

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

6 years ago by Sara Golemon — view source

unread

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

6 years ago by Andrey Andreev — view source

unread

Hi,

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.

6 years ago by Anatol Belski — view source

unread

-----Original Message-----
From: Remi Collet remi@fedoraproject.org
Sent: Monday, September 3, 2018 3:55 PM
To: internals@lists.php.net
Subject: Re: [PHP-DEV] Lift ICU requirements

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)

The further idea was also to check with a newer C++ version which could bring improvements. In most it would concern the ICU headers compatibility for ICU < 59, but the older the ICU version would be - the less would be the chance to get it working. I'd be going for it after Christoph's patch has landed.

Does this require an RFC?

I don't think.

Same here. Versions older than 50.1 are likely not used widely and the most of tests run also on some more recent versions, thus it's more about the dead code elimination and refactoring.

Regards

Anatol

6 years ago by Christoph M. Becker — view source

unread

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