Great work. I spent some time reviewing the implementation last couple
of weeks.
So far, it looks good.
I have one suggestion -
http://cr.openjdk.java.net/~naoto/6336885/webrev.03/src/share/classes/sun/util/cldr/CLDRLocaleProviderAdapter.java.html
- line 49-
49 private static final String LOCALE_DATA_JAR_NAME = "cldrdata.jar";
50
51 public CLDRLocaleProviderAdapter() {
52 final String sep = File.separator;
53 String localeDataJar = java.security.AccessController.doPrivileged(
54 new sun.security.action.GetPropertyAction("java.home"))
55 + sep + "lib" + sep + "ext" + sep + LOCALE_DATA_JAR_NAME;
56
57 // Peek at the installed extension directory to see if the jar
file for
58 // CLDR resources is installed or not.
59 final File f = new File(localeDataJar);
60 boolean result = AccessController.doPrivileged(
61 new PrivilegedAction<Boolean>() {
62 @Override
63 public Boolean run() {
64 return f.exists();
65 }
66 });
67 if (!result) {
68 throw new UnsupportedOperationException();
69 }
70 }
For now, the path to "cldrdata.jar" is hardcoded. I'm wondering if we
can support "cldrdata.jar" in a custom path.
The proposed code imports CLDR 21.0.1 data files. Someone may want to
update the contents imported from a later version of CLDR in future.
With the enhancement in this change set, you should be able to create a
new version of cldrdata.jar relatively easily. But JRE consumer may not
want to repackage JRE binaries. So, I suggest the implementation resolve
the path of "cldrdata.jar" via system property if exists. For example,
system property "java.locale.cldrdata = /home/yoshito/cldr22data.jar".
Thanks,
Yoshito
On 8/20/2012 1:14 PM, Naoto Sato wrote:
I have updated the changeset by removing the copyright headers from
all of the CLDR files, and added a LICENSE file at the top of CLDR
source directory (src/share/classes/sun/util/cldr/resources/21_0_1).
No other changes have been made this time.
http://cr.openjdk.java.net/~naoto/6336885/webrev.03/
Naoto
On H.24/08/14 9:16, Jeff Dinkins wrote:
Andrew - good catch. We're reviewing.
On Aug 14, 2012, at 2:51 AM, Andrew Hughes<ahug...@redhat.com> wrote:
----- Original Message -----
Hello,
Please review the JDK8 changes for JEP 127: Improve Locale Data
Packaging and Adopt Unicode CLDR Data
(http://openjdk.java.net/jeps/127). The webrev is located at:
http://cr.openjdk.java.net/~naoto/6336885/webrev.00/
The main bug id for this enhancement is:
6336885: RFE: Locale Data Deployment Enhancements
Along with this, the following bugs/enhancements are also implemented
in
this change:
4609153 Provide locale data for Indic locales
5104387 Support for gl_ES locale (galician language)
6337471 desktop/system locale preferences support
7056139 (cal) SPI support for locale-dependent Calendar parameters
7058206 Provide CalendarData SPI for week params and display field
value
names
7073852 Support multiple scripts for digits and decimal symbols per
locale
7079560 [Fmt-Da] Context dependent month names support in
SimpleDateFormat
7171324 getAvailableLocales() of locale sensitive services should
return
the actual availability of locales
7151414 (cal) Support calendar type identification
7168528 LocaleServiceProvider needs to be aware of Locale extensions
7171372 (cal) locale's default Calendar should be created if unknown
calendar is specified
Please note that packaging changes that relate to Jigsaw module
system
aren't included in this changeset.
Naoto Sato and Masayoshi Okutsu
First of all, I'd like to say congratulations on completing this
week and
adopting the CLDR as a source for locale data. This is something
we've used
for GNU Classpath's locale data for many years, and, with these
changes, it
should mean that the Java API itself should see changes which allow
it to
support some of the features present in the CLDR data.
However, I am a little concerned by the inclusion of CLDR data files
in this
webrev with a GPL header assigning copyright to Oracle e.g.
http://cr.openjdk.java.net/~naoto/6336885/webrev.00/src/share/classes/sun/util/cldr/resources/21_0_1/common/main/agq.xml.patch
Is this legal? IANAL, but
http://unicode.org/copyright.html#Exhibit1 seems to
state that the header given there should be present on this data.
The current
files make it look like this data was created by Oracle this year,
which is clearly
inaccurate.
Thanks,
--
Andrew :)
Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)
PGP Key: 248BDC07 (https://keys.indymedia.org/)
Fingerprint = EC5A 1F5E C0AD 1D15 8F1F 8F91 3B96 A578 248B DC07
