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

[Mandy Chung]

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?

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