[9] RFR 8038436: Re-examine the mechanism to determine available localedata and cldrdata

[Naoto Sato]

On 8/28/14, 5:11 PM, Mandy Chung wrote:
> Naoto,
>
> This looks better.  Thanks for the update.
>
> The getSupportedLocaleString method in both EnLocaleDataMetaInfo and
> NonEnLocaleDataMetaInfo has the javadoc that missing the description.
>
> sun.util.locale.provider is a package in java.base and
> NonEnLocaleDataMetaInfo has to be in a different package as it's in the
> jdk.localedata
>
> Previously the list of locales was generated at build time.
> NonEnLocaleDataMetaInfo now hardcodes the list in the source.  How often
> do we need to update this list?  Just wonder whether there is a better
> way to maintain this list and catch regression.  You have a new test
> Bug8038436.java to compare the golden list which is good.  However, the
> golden list is hardcoded too that seems to be error-prone.  Does
> Bug8038436.java pass with jdk9?

JRE's locale data will be just in maintenance mode as CLDR will become 
the defualt, so it won't change much, and the test case does pass with jdk9.

Having said that, I just reverted to the dynamic list generation at the 
build time. Just leave the test case though.

I will address your other suggestions in the next revision.

Naoto

>
> The current source layout implies the jdk.localedata module contains
> both cldr and localedata while they are generated as two separate JAR
> files in the jdk image.  Would you propose to split CLDR out as
> jdk.cldrdata?  Currently you have two service config files providing the
> same service type and leads to the awkward location.
>
> If they should be kept under jdk.localedata, we could live with the two
> service config files until the modular image is in place.
>
> I would suggest renaming
> src/jdk.localedata/share/classes/sun/util/cldr/sun.util.locale.provider.LocaleDataMetaInfo
>    ->
> src/jdk.localedata/META-INF/cldrdata-services/sun.util.locale.provider.LocaleDataMetaInfo
>
> src/jdk.localedata/share/classes/sun/util/locale/provider/META-INF/services/sun.util.locale.provider.LocaleDataMetaInfo
>    ->
> src/jdk.localedata/META-INF/localedata-services/sun.util.locale.provider.LocaleDataMetaInfo
>
> Mandy
>
> On 8/28/14 11:34 AM, Naoto Sato wrote:
>> Thank you, Mandy, Masayoshi, and Alan for your comments. I revised the
>> changes based on your suggestions as follows:
>>
>> http://cr.openjdk.java.net/~naoto/8038436/webrev.4/
>>
>> Here are the changes since webrev.3
>>
>> - CLDRLocaleProviderAdapter.java: modified to throw
>> UnsupportedOperationException with the actual exception set to its
>> "cause." More descriptive comment on it.
>>
>> - *ProviderImpl.java: Removed changes to them. Initial thought was to
>> make them performant by changing the langtags to static, but it turned
>> out that wasn't necessary.
>>
>> - JREENLocaleDataMetaInfo.java/JRENonENLocaleDataMetaInfo.java:
>> Renamed to EnLocaleDataMetaInfo and NonEnLocaleDataMetaInfo
>> respectively for readability. String constants are wrapped for
>> readability as well. Used getOrDefault() for getSupportedLocaleString().
>>
>> - Added test cases for SecurityException and JRE's supported locales
>> for each service.
>>
>> I'd appreciate it if someone in the build-dev could review the
>> makefile changes.
>>
>> Naoto
>>
>> On 8/22/14, 11:46 AM, Naoto Sato wrote:
>>> Hello,
>>>
>>> Please review the changes for the following issue:
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8038436
>>>
>>> The proposed changes are located at:
>>>
>>> http://cr.openjdk.java.net/~naoto/8038436/webrev.3/
>>>
>>> The idea is to introduce an SPI so that supported locales are
>>> dynamically searched at runtime, not depending on the existence of
>>> physical jar files.
>>>
>>> I'd appreciate it if build folks could review the makefile related
>>> changes, i18n forks to review locale framework files, and Mandy from
>>> modularization point of view.
>>>
>>> Naoto
>