I forgot to mention one optimization that we might make later on: this PR packages 2 versions of the ICU data (one per format version.) The more recent version is way more compact, but it cannot be read by intl extensions, compiled against the old format (but the newer version can read the old format.)

So, if there are no significant performance gains when using the newest format, we can drop it altogether and only package the old format (that will save about 8Mb.)

For those reviewing this PR, the only interesting part is the changes done in the IcuData.php class: https://github.com/symfony/Icu/pull/12/files#diff-1

Looks good to me.

For DX reasons, I suggest to completely deprecate symfony/icu and create a new package symfony/intl-data instead. This will make the meaning of the package much clearer, especially now that people have to manually add it.

Furthermore, the intl/icu dependencies should be removed from composer.json. Otherwise people can't install lock-files that contain symfony/intl-data unless they have the extension loaded.

@webmozart I've removed the "ext-intl": "*" requirement in composer.json.
I also agree that a new repository is better.

he more recent version is way more compact, but it cannot be read by intl extensions, compiled against the old format [...] So, if there are no significant performance gains when using the newest format, we can drop it altogether and only package the old format

An alternative would be to implement the \ResourceBundle class for the new format in PHP and drop the files in the old format instead. This would make the files accessible even without the intl extension, and would reduce the package size to a minimum.

@webmozart What about dropping the dependency on symfony/intl as well. As we only have data, they could be used by someone else without the symfony/intl component.

@fabpot Yes, makes sense.

@webmozart I've added the following to the composer file:

    "suggest": {
        "symfony/intl": "To easily access the ICU data from PHP"
    },

@webmozart We can indeed create a new repository named symfony/intl-data but what about the namespace for the IcuData class? Do we keep the current namespace?

I also agree about changing the name of the package. If we want to be consistent, the class name could be Symfony\Component\IntlData\IntlData.
Btw, given that it is not really a Symfony component which is providing data, does it make sense to drop the Component part of the namespace here ?

@stof I agree with you: Symfony\IntlData\ for the namespace sounds good to me. IntlData for the class is fine as well.

And the repository would be named symfony/intl-data, right?

yes (and the composer package too)

btw, should it be intl-data or icu-data ? We are talking about ICU currently

It should be intl-data. To be 100% precise, we're talking about the CLDR data used by the ICU project, which is in turn used by PHP's intl extension. But that's useless information for our users. So intl-data should be fine.

Closing this PR as I've moved everything to https://github.com/symfony/intl-data

@fabpot I thinkt that the getBundleReader() and the dependencies on Intl should be removed from IntlData.

I'm not sure whether it's good to drop the Component from the namespace for consistency reasons. Do we have any other package that lies directly under Symfony?

@fabpot the readme needs to be updated

@webmozart I'm not sure we have in the Symfony organization (which mostly contains subtree splits), but the PHP-CS-Fixer uses Symfony\CS and the prototype of the new Symfony installer uses Symfony\Toolbelt

symfony/symfony#11884 is the counterpart of this PR. I've submitted here now to gather feedback but as I worked on it quite a long time ago, I'm sure it's finished yet.

@webmozart For the Intl dep, I've created an issue on the other repository (symfony/intl-data#1.)

@stof which README file?

https://github.com/symfony/intl-data/blob/7cbe6f1b278072fa9e5c3339c4eb7cea11ae1388/README.md which says it contains data for ICU 4.4+

I forgot to mention one optimization that we might make later on: this PR packages 2 versions of the ICU data (one per format version.) The more recent version is way more compact, but it cannot be read by intl extensions, compiled against the old format (but the newer version can read the old format.)

So, if there are no significant performance gains when using the newest format, we can drop it altogether and only package the old format (that will save about 8Mb.)

I can't seem to find my post where I tested all this and then described everything. But it's complicated.

AFAIR: The old format has to be generated with the old genrb as even though you specify the new genrb to generate the old format res file it doesn't do so.
Also if I remember correctly some new ICU libraries weren't able to open the old format although they should.

P.S. The data shipped here is old. As there is already ICU 53

The old format has to be generated with the old genrb as even though you specify the new genrb to generate the old format res file it doesn't do so.

That's what we do ATM.

The data shipped here is old. As there is already ICU 53

Yes, we need to update it when this bug fix is through.