On 13/05/2020 15:59, Severin Gehwolf wrote:
> Hi,
>
> Could somebody please review this fix for the failing
> JapanEraNameCompatTest.java test (part of tier1)? The reason why it's
> failing is that there are missing locale data for no and sr_Latn which
> are exercised by the test. The original backport of JDK-8219781 didn't
> include those as the changed sections in the JDK 11 patch didn't exist
> in 8u. Those extra sections got introduced with JDK-8008577 or JDK-
> 8145136.
This also includes the JDK-8219781 changes for these locales. I think
it's worth mentioning all three bug IDs in the summary text so they
appear in a keyword search.
Changes for these locales look ok to me.
>
> This patch adds locale data for locales left out in the JDK-8219781 8u
> backport, except for JavaTimeSupplementary_in.java. That file doesn't
> exist at all in 8u, hence I've left that out.
This doesn't make sense to me. The in locale is present in
sun/text/resources, but the only one without JavaTimeSupplementary_*:
$ for locales in $(find jdk/src/share/classes/sun/text/resources/
-maxdepth 1 -mindepth 1 -type d)
> do
> locale=$(basename ${locales})
> echo ${locale}; ls ${locales}/JavaTimeSupplementary_${locale}*
> done
pt
jdk/src/share/classes/sun/text/resources/pt/JavaTimeSupplementary_pt.java
jdk/src/share/classes/sun/text/resources/pt/JavaTimeSupplementary_pt_PT.java
ro
jdk/src/share/classes/sun/text/resources/ro/JavaTimeSupplementary_ro.java
hr
jdk/src/share/classes/sun/text/resources/hr/JavaTimeSupplementary_hr.java
es
jdk/src/share/classes/sun/text/resources/es/JavaTimeSupplementary_es.java
de
jdk/src/share/classes/sun/text/resources/de/JavaTimeSupplementary_de.java
in
ls: cannot access
'jdk/src/share/classes/sun/text/resources/in/JavaTimeSupplementary_in*':
No such file or directory
tr
jdk/src/share/classes/sun/text/resources/tr/JavaTimeSupplementary_tr.java
sk
jdk/src/share/classes/sun/text/resources/sk/JavaTimeSupplementary_sk.java
mk
jdk/src/share/classes/sun/text/resources/mk/JavaTimeSupplementary_mk.java
ca
jdk/src/share/classes/sun/text/resources/ca/JavaTimeSupplementary_ca.java
iw
jdk/src/share/classes/sun/text/resources/iw/JavaTimeSupplementary_iw_IL.java
jdk/src/share/classes/sun/text/resources/iw/JavaTimeSupplementary_iw.java
is
jdk/src/share/classes/sun/text/resources/is/JavaTimeSupplementary_is.java
lv
jdk/src/share/classes/sun/text/resources/lv/JavaTimeSupplementary_lv.java
sq
jdk/src/share/classes/sun/text/resources/sq/JavaTimeSupplementary_sq.java
vi
jdk/src/share/classes/sun/text/resources/vi/JavaTimeSupplementary_vi.java
hu
jdk/src/share/classes/sun/text/resources/hu/JavaTimeSupplementary_hu.java
ms
jdk/src/share/classes/sun/text/resources/ms/JavaTimeSupplementary_ms.java
no
jdk/src/share/classes/sun/text/resources/no/JavaTimeSupplementary_no.java
et
jdk/src/share/classes/sun/text/resources/et/JavaTimeSupplementary_et.java
da
jdk/src/share/classes/sun/text/resources/da/JavaTimeSupplementary_da.java
sr
jdk/src/share/classes/sun/text/resources/sr/JavaTimeSupplementary_sr.java
jdk/src/share/classes/sun/text/resources/sr/JavaTimeSupplementary_sr_Latn.java
th
jdk/src/share/classes/sun/text/resources/th/JavaTimeSupplementary_th.java
it
jdk/src/share/classes/sun/text/resources/it/JavaTimeSupplementary_it.java
bg
jdk/src/share/classes/sun/text/resources/bg/JavaTimeSupplementary_bg.java
lt
jdk/src/share/classes/sun/text/resources/lt/JavaTimeSupplementary_lt.java
el
jdk/src/share/classes/sun/text/resources/el/JavaTimeSupplementary_el.java
pl
jdk/src/share/classes/sun/text/resources/pl/JavaTimeSupplementary_pl.java
ja
jdk/src/share/classes/sun/text/resources/ja/JavaTimeSupplementary_ja.java
ru
jdk/src/share/classes/sun/text/resources/ru/JavaTimeSupplementary_ru.java
be
jdk/src/share/classes/sun/text/resources/be/JavaTimeSupplementary_be.java
fi
jdk/src/share/classes/sun/text/resources/fi/JavaTimeSupplementary_fi.java
cs
jdk/src/share/classes/sun/text/resources/cs/JavaTimeSupplementary_cs.java
hi
jdk/src/share/classes/sun/text/resources/hi/JavaTimeSupplementary_hi_IN.java
en
jdk/src/share/classes/sun/text/resources/en/JavaTimeSupplementary_en_GB.java
jdk/src/share/classes/sun/text/resources/en/JavaTimeSupplementary_en.java
jdk/src/share/classes/sun/text/resources/en/JavaTimeSupplementary_en_SG.java
sv
jdk/src/share/classes/sun/text/resources/sv/JavaTimeSupplementary_sv.java
ga
jdk/src/share/classes/sun/text/resources/ga/JavaTimeSupplementary_ga.java
fr
jdk/src/share/classes/sun/text/resources/fr/JavaTimeSupplementary_fr.java
zh
jdk/src/share/classes/sun/text/resources/zh/JavaTimeSupplementary_zh.java
jdk/src/share/classes/sun/text/resources/zh/JavaTimeSupplementary_zh_TW.java
uk
jdk/src/share/classes/sun/text/resources/uk/JavaTimeSupplementary_uk.java
nl
jdk/src/share/classes/sun/text/resources/nl/JavaTimeSupplementary_nl.java
ko
jdk/src/share/classes/sun/text/resources/ko/JavaTimeSupplementary_ko.java
ar
jdk/src/share/classes/sun/text/resources/ar/JavaTimeSupplementary_ar.java
mt
jdk/src/share/classes/sun/text/resources/mt/JavaTimeSupplementary_mt.java
sl
jdk/src/share/classes/sun/text/resources/sl/JavaTimeSupplementary_sl.java
The file is introduced in JDK-8145136. Why not just take it from there
and update with the changes in JDK-8219781?
The test passes for this locale (as it does for nl, which is updated in
this patch) because the values are the same as for the base locale
(English). but there are other translations in that file which are
missing in 8u.
>
> Missing locale info has been retrieved from JDK-8008577 and JDK-8145136
> both of which aren't suitable to get backported to 8u.
Agreed.
The problem with these changesets is they attempt to do everything in
bulk. It's a problem I've seen with such changes before. We don't want
to backport the CLDR update, but the updates to legacy locale data
should be ok.
We did used to get locale data updates with most CPUs. I haven't seen
those since we switched to the current update system, which is a little
worrying.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8244843
> webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8244843/01/webrev/
>
> Testing: tier1. One less test failure, namely JapanEraNameCompatTest,
> no longer fails.
>
> Thoughts?
>
I guess the number of people hitting the lack of a Serbian translation
of a Japanese era name in the Latin character set was small, but it's
good to have this fixed. Thanks for looking into it.
> Thanks,
> Severin
>
--
Andrew :)
Senior Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)
PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net)
Fingerprint = 5132 579D D154 0ED2 3E04 C5A0 CFDA 0F9B 3596 4222
