Everything
php
Learn Community Releases The PHP Project
Home » Project » Newsgroups » php.internals » Lift ICU requirements »

Lift ICU requirements

php.internals

Christoph Becker

8 years ago
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

Remi Collet

8 years ago
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

Andrey Andreev

8 years ago
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.

Christoph Becker

8 years ago
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

Andrey Andreev

8 years ago
Hi, On Mon, Sep 3, 2018 at 6:13 PM, Christoph M. Becker <cmbecker69@gmx.de> 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.

Christoph Becker

8 years ago
On 04.09.2018 at 13:44, Andrey Andreev wrote:
> On Mon, Sep 3, 2018 at 6:13 PM, Christoph M. Becker <cmbecker69@gmx.de> 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

Sara Golemon

8 years ago
On Tue, Sep 4, 2018 at 6:44 AM, Andrey Andreev <narf@devilix.net> wrote:
> On Mon, Sep 3, 2018 at 6:13 PM, Christoph M. Becker <cmbecker69@gmx.de> 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

Andrey Andreev

8 years ago
Hi, On Tue, Sep 4, 2018 at 7:22 PM, Sara Golemon <pollita@php.net> wrote:
> On Tue, Sep 4, 2018 at 6:44 AM, Andrey Andreev <narf@devilix.net> wrote: >> On Mon, Sep 3, 2018 at 6:13 PM, Christoph M. Becker <cmbecker69@gmx.de> 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.

Anatoliy Belsky

8 years ago
> -----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

Christoph Becker

7 years ago
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