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.
> - 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/util/calendar/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
values?
> 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/share/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
