+1
On 7/9/19 9:07 AM, naoto.s...@oracle.com wrote:
Looks good, Ramanand.
Naoto
On 7/9/19 5:09 AM, Ramanand Patil wrote:
Hi Naoto,
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.
Regards,
Ramanand.
-----Original Message-----
From: Naoto Sato
Sent: Monday, July 8, 2019 11:19 PM
To: Ramanand Patil <ramanand.pa...@oracle.com>; Andrew John Hughes
<gnu.and...@redhat.com>; core-libs-dev@openjdk.java.net; i18n-
d...@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
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.
Naoto
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.and...@redhat.com>
Sent: Saturday, July 6, 2019 9:53 PM
To: Ramanand Patil <ramanand.pa...@oracle.com>; core-libs-
d...@openjdk.java.net; i18n-...@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
line.
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
values?
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
