<i18n dev> RFR: 8224560: (tz) Upgrade time-zone data to tzdata2019a and 8225580: tzdata2018i integration causes test failures on jdk-13
ramanand.patil at oracle.com
Tue Jul 9 12:09:52 UTC 2019
Thank you for the review. Revised webrev: http://cr.openjdk.java.net/~rpatil/8224560/webrev.02/
I plan to push the changes tomorrow, if there are no further comments.
> -----Original Message-----
> From: Naoto Sato
> Sent: Monday, July 8, 2019 11:19 PM
> To: Ramanand Patil <ramanand.patil at oracle.com>; Andrew John Hughes
> <gnu.andrew at redhat.com>; core-libs-dev at openjdk.java.net; i18n-
> dev at openjdk.java.net
> Subject: Re: <i18n dev> RFR: 8224560: (tz) Upgrade time-zone data to
> tzdata2019a and 8225580: tzdata2018i integration causes test failures on jdk-
> Hi Ramanand,
> As to the change in ZoneInfoFile.java, I would put that special handling of
> Gaza/Hebron in line 577, as well as merging the comment in 575,576, so that
> it won't duplicate the code.
> Otherwise looks good.
> On 7/8/19 3:35 AM, Ramanand Patil wrote:
> > Hi Andrew,
> > Thank you for your review.
> > Updated webrev: http://cr.openjdk.java.net/~rpatil/8224560/webrev.01/
> > Regards,
> > Ramanand.
> >> -----Original Message-----
> >> From: Andrew John Hughes <gnu.andrew at redhat.com>
> >> Sent: Saturday, July 6, 2019 9:53 PM
> >> To: Ramanand Patil <ramanand.patil at oracle.com>; core-libs-
> >> dev at openjdk.java.net; i18n-dev at openjdk.java.net
> >> Subject: Re: <i18n dev> RFR: 8224560: (tz) Upgrade time-zone data to
> >> tzdata2019a and 8225580: tzdata2018i integration causes test failures
> >> on jdk-
> >> 13
> >> On 05/07/2019 15:16, Ramanand Patil wrote:
> >>> Hi all,
> >>> Please review the patch for tzdata2019a integration into jdk project.
> >>> Webrev: http://cr.openjdk.java.net/~rpatil/8224560/webrev.00/
> >>> Bugs: https://bugs.openjdk.java.net/browse/JDK-8224560
> >>> https://bugs.openjdk.java.net/browse/JDK-8225580
> >>> Summary:
> >>> - The fix contains cumulative tzdata changes from tzdata2018i and
> >> tzdata2019a, as tzdata2018i was not integrated into jdk/jdk earlier.
> >>> - In JDK-13/14, multiple failures were seen during integration of
> >>> tzdata2018i
> >> (JDK-8225580), those are fixed now. Many thanks to Naoto for
> >> providing a fix for this in CLDRConverter.java.
> >> I would guess this is due to the CLDR update (JDK-8221432: Upgrade
> >> CLDR to Version 35.1) in OpenJDK 13, making TimeZone.getAvailableIDs
> >> inappropriate in some way?
> >> Fix looks good. One minor change:
> >> + AVAILABLE_TZIDS = new
> >> + HashSet(ZoneId.getAvailableZoneIds());
> >> is missing the <String> or <>:
> >> + AVAILABLE_TZIDS = new
> >> + HashSet<>(ZoneId.getAvailableZoneIds());
> >> Will this fix also resolve JDK-8225580? If so, it's probably worth
> >> mentioning both bug IDs in the commit.
> > Yes, this fix will also resolve JDK-8225580, hence included in the subject
> > And thank you, I will add both bug IDs in the commit message.
> >>> - There are 2 type of test failures in TestZoneInfo310.java file,
> >>> which are
> >> solved in this patch by providing workarounds, But a permanent fix
> >> needs to be added in future for the same. Below are the 2 bugs
> >> created to track the development on it:
> >>> 1. https://bugs.openjdk.java.net/browse/JDK-8223388:
> >> TestZoneInfo310.java fails post tzdata2018i integration:
> >>> This failure is seen for the TimeZones which are having zone rules
> >>> defined
> >> till year 2037 or beyond. For now, the failing zones are skipped.
> >>> The supporting test class ZoneInfo.java has maxYear defined
> >> http://hg.openjdk.java.net/jdk/jdk/file/d01b345865d7/test/jdk/sun/uti
> >> l/cale ndar/zi/Zoneinfo.java#l40, changing this max value greater
> >> than the zone rule's last year also fixes the issue, but further
> >> investigation is needed on why this boundary condition is affecting
> >> the test behavior.
> >> I wonder if 2037 is in someway related to the rollover of 32-bit time
> > I think, not directly related but how the test and JDK handles these values.
> > In JDK, the transitions beyond 2037 are delegated to SimpleTimeZone, and I
> think the test somehow miscalculates it.
> > http://hg.openjdk.java.net/jdk/jdk/file/5919b273def6/src/java.base/sha
> > re/classes/sun/util/calendar/ZoneInfo.java#l48
> > I think, I should re-visit and see if these test are really needed
> > now. As per the long standing bug JDK-8166983 suggestion was to remove
> > the whole tests from test/sun/util/calendar/zi
> >>> 2. JDK-8227293: https://bugs.openjdk.java.net/browse/JDK-8227293
> >> TestZoneInfo310.java fails post tzdata2019a integration for Palestine
> >> zone
> >> rules:
> >>> There are many hacks and assumptions in the class
> >>> sun.util.calendar.ZoneInfoFile.java. This issue looks because of the
> >>> code starting from here:
> >>> http://hg.openjdk.java.net/jdk/jdk/file/963924f1c891/src/java.base/s
> >>> ha
> >>> re/classes/sun/util/calendar/ZoneInfoFile.java#l552
> >>> There is an assumption where the transition date is >=24,(line 577
> >>> and 599)
> >> it assumes it is the "last" rule, but it is not last rule in case of
> >> Asia/Gaza and Asia/Hebron zones.
> >>> For now, I have fixed these 2 problematic timezones by overwriting
> >>> the assumption made on line 577, where date of month dom =
> >>> startRule.dom; I
> >> think, overriding of the second jdk hack on line 599 is not required
> >> as the "dom" is calculated from the last rule there. Keeping this bug
> >> open as we need to find a generic solution for this issue, without
> >> hard-coding the values and adding specific time zone names in
> >> exclusion as seen in many places in this class file.
> >>> - The patch has passed all the related testing including JCK tests.
> >>> Regards,
> >>> Ramanand.
> >> Looks good to me, with the above minor adjustment.
> >> Thanks,
> >> --
> >> 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
> >> https://keybase.io/gnu_andrew
More information about the i18n-dev