Re: RFR: 8285838: DST not applying properly with zone id offset set with TZ env variable [v7]
On Tue, 7 Jun 2022 14:15:55 GMT, Gaurav Chaudhari wrote: >> This fix ensures that when a lookup for a custom TZ code fails, and an >> attempt is made to find the GMT offset in order to get the current time, >> Daylight savings rules are applied correctly. > > Gaurav Chaudhari has updated the pull request incrementally with one > additional commit since the last revision: > > 8285838: Simplified Daylight period checks for test. Looks good. Thanks for the contribution. - Marked as reviewed by naoto (Reviewer). PR: https://git.openjdk.org/jdk/pull/8661
Re: RFR: 8285838: DST not applying properly with zone id offset set with TZ env variable [v6]
On Tue, 7 Jun 2022 14:11:57 GMT, Gaurav Chaudhari wrote: >> Gaurav Chaudhari has updated the pull request incrementally with two >> additional commits since the last revision: >> >> - 8285838: Cleanup of trailing whitespace >> - 8285838: Corrected month comparison check for TZ DST > > Appreciate the suggestion, I fixed up my test and simplified it as you said, > and checked against a few spawned LocalDate(s) to verify the functionality is > still good. @Deigue Please do not rebase or force-push to an active PR as it invalidates existing review comments. All changes will be squashed into a single commit automatically when integrating. See [OpenJDK Developers’ Guide](https://openjdk.java.net/guide/#working-with-pull-requests) for more information. - PR: https://git.openjdk.java.net/jdk/pull/8661
Re: RFR: 8285838: DST not applying properly with zone id offset set with TZ env variable [v6]
On Wed, 1 Jun 2022 13:32:36 GMT, Gaurav Chaudhari wrote: >> This fix ensures that when a lookup for a custom TZ code fails, and an >> attempt is made to find the GMT offset in order to get the current time, >> Daylight savings rules are applied correctly. > > Gaurav Chaudhari has updated the pull request incrementally with two > additional commits since the last revision: > > - 8285838: Cleanup of trailing whitespace > - 8285838: Corrected month comparison check for TZ DST Appreciate the suggestion, I fixed up my test and simplified it as you said, and checked against a few spawned LocalDate(s) to verify the functionality is still good. - PR: https://git.openjdk.java.net/jdk/pull/8661
Re: RFR: 8285838: DST not applying properly with zone id offset set with TZ env variable [v7]
> This fix ensures that when a lookup for a custom TZ code fails, and an > attempt is made to find the GMT offset in order to get the current time, > Daylight savings rules are applied correctly. Gaurav Chaudhari has updated the pull request incrementally with one additional commit since the last revision: 8285838: Simplified Daylight period checks for test. - Changes: - all: https://git.openjdk.java.net/jdk/pull/8661/files - new: https://git.openjdk.java.net/jdk/pull/8661/files/87832663..f855493a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8661=06 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8661=05-06 Stats: 12 lines in 1 file changed: 0 ins; 6 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/8661.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8661/head:pull/8661 PR: https://git.openjdk.java.net/jdk/pull/8661
Re: RFR: 8285838: DST not applying properly with zone id offset set with TZ env variable [v6]
On Thu, 2 Jun 2022 18:14:55 GMT, Gaurav Chaudhari wrote:
> Is the suggestion here to substitute the setting of the TZ environment
> variable, and simply getting a date based off this `SimpleTimeZone` , so as
> to bypass the process creation and just have it as a more simpler test?
No. The test process still has to be spawned with the TZ env variable set. The
suggested piece is supposed to replace the following `if` in `runTZTest()`
method:
Calendar calendar = Calendar.getInstance();
Date time = calendar.getTime();
// Add 1 since getMonth() starts from 0.
int month = time.getMonth() + 1;
ZonedDateTime date = ZonedDateTime.ofInstant(time.toInstant(),
ZoneId.systemDefault());
if ((month > Month.MARCH.getValue() && month <
Month.OCTOBER.getValue()) ||
(month == Month.MARCH.getValue() &&
date.isAfter(getLastSundayOfMonth(date))) ||
(month == Month.OCTOBER.getValue() &&
date.isBefore(getLastSundayOfMonth(date {
-
PR: https://git.openjdk.java.net/jdk/pull/8661
Re: RFR: 8285838: DST not applying properly with zone id offset set with TZ env variable [v6]
On Thu, 2 Jun 2022 18:07:39 GMT, Naoto Sato wrote: >> Gaurav Chaudhari has updated the pull request incrementally with two >> additional commits since the last revision: >> >> - 8285838: Cleanup of trailing whitespace >> - 8285838: Corrected month comparison check for TZ DST > > I tried several runs of CI tests and found no failures. Sorry for the false > alarm. BTW, now I've looked the test more closely, the test can be simplified > by using `SimpleTimeZone`, such as: > > new SimpleTimeZone(360, "MEZ-1MESZ", Calendar.MARCH, -1, > Calendar.SUNDAY, 0, Calendar.OCTOBER, -1, Calendar.SUNDAY, > 0).inDaylightTime(new Date()) > > This piece will check the current date is in DST, emulating > `TZ=MEZ-1MESZ,M3.5.0,M10.5.0` Hi @naotoj , Thanks for revisiting that and confirming it is all good. Initially the problem was brought to my attention through a third party client utilizing this particular TZ code, and that is where we got the basis for the need of the fix, where we are bypassing typical tzdb access to find named Timezones such as "Europe/Belgium" and trying to find the GMT offset and computing the time. Is the suggestion here to substitute the setting of the TZ environment variable, and simply getting a date based off this `SimpleTimeZone` , so as to bypass the process creation and just have it as a more simpler test? This sounds perfectly fine as long as SimpleTimeZone is have the zone ID setup exactly as the `MEZ-1MESZ,M3.5.0,M10.5.0` indicated. I can try this out ... just wanted to confirm with you how to make use of the above instantiation. - PR: https://git.openjdk.java.net/jdk/pull/8661
Re: RFR: 8285838: DST not applying properly with zone id offset set with TZ env variable [v6]
On Wed, 1 Jun 2022 13:32:36 GMT, Gaurav Chaudhari wrote: >> This fix ensures that when a lookup for a custom TZ code fails, and an >> attempt is made to find the GMT offset in order to get the current time, >> Daylight savings rules are applied correctly. > > Gaurav Chaudhari has updated the pull request incrementally with two > additional commits since the last revision: > > - 8285838: Cleanup of trailing whitespace > - 8285838: Corrected month comparison check for TZ DST I tried several runs of CI tests and found no failures. Sorry for the false alarm. BTW, now I've looked the test more closely, the test can be simplified by using `SimpleTimeZone`, such as: new SimpleTimeZone(360, "MEZ-1MESZ", Calendar.MARCH, -1, Calendar.SUNDAY, 0, Calendar.OCTOBER, -1, Calendar.SUNDAY, 0).inDaylightTime(new Date()) This piece will check the current date is in DST, emulating `TZ=MEZ-1MESZ,M3.5.0,M10.5.0` - PR: https://git.openjdk.java.net/jdk/pull/8661
Re: RFR: 8285838: DST not applying properly with zone id offset set with TZ env variable [v6]
On Wed, 1 Jun 2022 13:32:36 GMT, Gaurav Chaudhari wrote: >> This fix ensures that when a lookup for a custom TZ code fails, and an >> attempt is made to find the GMT offset in order to get the current time, >> Daylight savings rules are applied correctly. > > Gaurav Chaudhari has updated the pull request incrementally with two > additional commits since the last revision: > > - 8285838: Cleanup of trailing whitespace > - 8285838: Corrected month comparison check for TZ DST The machine has `/etc/localtime` pointing to `/usr/share/zoneinfo/GMT`, thus the system zone is `GMT`. However, for some reason, I cannot reproduce it on that machine any longer. Could be some inconsistency in my repo. Will test in our CI system and let you know the result later. - PR: https://git.openjdk.java.net/jdk/pull/8661
Re: RFR: 8285838: DST not applying properly with zone id offset set with TZ env variable [v5]
On Tue, 31 May 2022 22:34:11 GMT, Naoto Sato wrote: >> Gaurav Chaudhari has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 8285838: Corrected month comparison check for TZ DST > > I tried out your patch on my local Linux machine, but the new test failed > with the following exception: > > ACTION: main -- Failed. Execution failed: `main' threw exception: > java.lang.RuntimeException: Expected to get exit value of [0] > REASON: User specified action: run main/othervm CustomTzIDCheckDST > TIME: 1.564 seconds > messages: > command: main CustomTzIDCheckDST > reason: User specified action: run main/othervm CustomTzIDCheckDST > Mode: othervm [/othervm specified] > elapsed time (seconds): 1.564 > configuration: > STDOUT: > Command line: > [/home/nsato/projects/jdk/git/jdk/build/linux-x64/images/jdk/bin/java -cp > /home/nsato/projects/jdk/git/jdk/build/linux-x64/test-support/jtreg_open_test_jdk_java_util_TimeZone/classes/0/java/util/TimeZone/CustomTzIDCheckDST.d:/home/nsato/projects/jdk/git/jdk/open/test/jdk/java/util/TimeZone:/home/nsato/projects/jdk/git/jdk/build/linux-x64/test-support/jtreg_open_test_jdk_java_util_TimeZone/classes/0/test/lib:/home/nsato/projects/jdk/git/jdk/open/test/lib:/var/tmp/jib-nsato/install/jtreg/6.1/1/bundles/jtreg-6.1+1.zip/jtreg/lib/javatest.jar:/var/tmp/jib-nsato/install/jtreg/6.1/1/bundles/jtreg-6.1+1.zip/jtreg/lib/jtreg.jar > CustomTzIDCheckDST runTZTest ] > [2022-05-31T22:27:05.958567816Z] Gathering output for process 14771 > [2022-05-31T22:27:06.635595481Z] Waiting for completion for process 14771 > [2022-05-31T22:27:06.635976964Z] Waiting for completion finished for process > 14771 > Output and diagnostic info for process 14771 was saved into > 'pid-14771-output.log' > [2022-05-31T22:27:06.663087767Z] Waiting for completion for process 14771 > [2022-05-31T22:27:06.663360403Z] Waiting for completion finished for process > 14771 > [2022-05-31T22:27:06.663754609Z] Waiting for completion for process 14771 > [2022-05-31T22:27:06.663869034Z] Waiting for completion finished for process > 14771 > STDERR: > stdout: []; > stderr: [Exception in thread "main" java.time.DateTimeException: Invalid ID > for offset-based ZoneId: GMT-22:00 > at java.base/java.time.ZoneId.ofWithPrefix(ZoneId.java:436) > at java.base/java.time.ZoneId.of(ZoneId.java:406) > at java.base/java.time.ZoneId.of(ZoneId.java:358) > at java.base/java.time.ZoneId.of(ZoneId.java:314) > at java.base/java.util.TimeZone.toZoneId0(TimeZone.java:581) > at java.base/java.util.TimeZone.toZoneId(TimeZone.java:558) > at java.base/java.util.TimeZone.toZoneId0(TimeZone.java:570) > at java.base/java.util.TimeZone.toZoneId(TimeZone.java:558) > at java.base/java.time.ZoneId.systemDefault(ZoneId.java:274) > at CustomTzIDCheckDST.runTZTest(CustomTzIDCheckDST.java:64) > at CustomTzIDCheckDST.main(CustomTzIDCheckDST.java:51) > Caused by: java.time.DateTimeException: Zone offset hours not in valid range: > value -22 is not in the range -18 to 18 > at java.base/java.time.ZoneOffset.validate(ZoneOffset.java:373) > at > java.base/java.time.ZoneOffset.ofHoursMinutesSeconds(ZoneOffset.java:326) > at java.base/java.time.ZoneOffset.of(ZoneOffset.java:257) > at java.base/java.time.ZoneId.ofWithPrefix(ZoneId.java:430) > ... 10 more > ] > exitValue = 1 > > java.lang.RuntimeException: Expected to get exit value of [0] > > at > jdk.test.lib.process.OutputAnalyzer.shouldHaveExitValue(OutputAnalyzer.java:489) > at CustomTzIDCheckDST.main(CustomTzIDCheckDST.java:49) > at > java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104) > at java.base/java.lang.reflect.Method.invoke(Method.java:578) > at > com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:127) > at java.base/java.lang.Thread.run(Thread.java:1585) > > JavaTest Message: Test threw exception: java.lang.RuntimeException: Expected > to get exit value of [0] > > JavaTest Message: shutting down test > > STATUS:Failed.`main' threw exception: java.lang.RuntimeException: Expected to > get exit value of [0] Hi @naotoj , I am unable to reproduce the above error you are seeing, and it is passing on my linux machine. What is the system default timezone on your local machine? Wondering if it will help me see what you are seeing (even though the timezone should be overriden via the TZ variable). Looks like the test is not even reaching the checks and failing at the point where its simply trying to retrieve the current time. - PR: https://git.openjdk.java.net/jdk/pull/8661
Re: RFR: 8285838: DST not applying properly with zone id offset set with TZ env variable [v6]
> This fix ensures that when a lookup for a custom TZ code fails, and an > attempt is made to find the GMT offset in order to get the current time, > Daylight savings rules are applied correctly. Gaurav Chaudhari has updated the pull request incrementally with two additional commits since the last revision: - 8285838: Cleanup of trailing whitespace - 8285838: Corrected month comparison check for TZ DST - Changes: - all: https://git.openjdk.java.net/jdk/pull/8661/files - new: https://git.openjdk.java.net/jdk/pull/8661/files/9e3b1bb4..87832663 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8661=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8661=04-05 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8661.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8661/head:pull/8661 PR: https://git.openjdk.java.net/jdk/pull/8661
Re: RFR: 8285838: DST not applying properly with zone id offset set with TZ env variable [v5]
On Tue, 31 May 2022 20:10:44 GMT, Gaurav Chaudhari wrote: >> This fix ensures that when a lookup for a custom TZ code fails, and an >> attempt is made to find the GMT offset in order to get the current time, >> Daylight savings rules are applied correctly. > > Gaurav Chaudhari has updated the pull request incrementally with one > additional commit since the last revision: > > 8285838: Corrected month comparison check for TZ DST I tried out your patch on my local Linux machine, but the new test failed with the following exception: ACTION: main -- Failed. Execution failed: `main' threw exception: java.lang.RuntimeException: Expected to get exit value of [0] REASON: User specified action: run main/othervm CustomTzIDCheckDST TIME: 1.564 seconds messages: command: main CustomTzIDCheckDST reason: User specified action: run main/othervm CustomTzIDCheckDST Mode: othervm [/othervm specified] elapsed time (seconds): 1.564 configuration: STDOUT: Command line: [/home/nsato/projects/jdk/git/jdk/build/linux-x64/images/jdk/bin/java -cp /home/nsato/projects/jdk/git/jdk/build/linux-x64/test-support/jtreg_open_test_jdk_java_util_TimeZone/classes/0/java/util/TimeZone/CustomTzIDCheckDST.d:/home/nsato/projects/jdk/git/jdk/open/test/jdk/java/util/TimeZone:/home/nsato/projects/jdk/git/jdk/build/linux-x64/test-support/jtreg_open_test_jdk_java_util_TimeZone/classes/0/test/lib:/home/nsato/projects/jdk/git/jdk/open/test/lib:/var/tmp/jib-nsato/install/jtreg/6.1/1/bundles/jtreg-6.1+1.zip/jtreg/lib/javatest.jar:/var/tmp/jib-nsato/install/jtreg/6.1/1/bundles/jtreg-6.1+1.zip/jtreg/lib/jtreg.jar CustomTzIDCheckDST runTZTest ] [2022-05-31T22:27:05.958567816Z] Gathering output for process 14771 [2022-05-31T22:27:06.635595481Z] Waiting for completion for process 14771 [2022-05-31T22:27:06.635976964Z] Waiting for completion finished for process 14771 Output and diagnostic info for process 14771 was saved into 'pid-14771-output.log' [2022-05-31T22:27:06.663087767Z] Waiting for completion for process 14771 [2022-05-31T22:27:06.663360403Z] Waiting for completion finished for process 14771 [2022-05-31T22:27:06.663754609Z] Waiting for completion for process 14771 [2022-05-31T22:27:06.663869034Z] Waiting for completion finished for process 14771 STDERR: stdout: []; stderr: [Exception in thread "main" java.time.DateTimeException: Invalid ID for offset-based ZoneId: GMT-22:00 at java.base/java.time.ZoneId.ofWithPrefix(ZoneId.java:436) at java.base/java.time.ZoneId.of(ZoneId.java:406) at java.base/java.time.ZoneId.of(ZoneId.java:358) at java.base/java.time.ZoneId.of(ZoneId.java:314) at java.base/java.util.TimeZone.toZoneId0(TimeZone.java:581) at java.base/java.util.TimeZone.toZoneId(TimeZone.java:558) at java.base/java.util.TimeZone.toZoneId0(TimeZone.java:570) at java.base/java.util.TimeZone.toZoneId(TimeZone.java:558) at java.base/java.time.ZoneId.systemDefault(ZoneId.java:274) at CustomTzIDCheckDST.runTZTest(CustomTzIDCheckDST.java:64) at CustomTzIDCheckDST.main(CustomTzIDCheckDST.java:51) Caused by: java.time.DateTimeException: Zone offset hours not in valid range: value -22 is not in the range -18 to 18 at java.base/java.time.ZoneOffset.validate(ZoneOffset.java:373) at java.base/java.time.ZoneOffset.ofHoursMinutesSeconds(ZoneOffset.java:326) at java.base/java.time.ZoneOffset.of(ZoneOffset.java:257) at java.base/java.time.ZoneId.ofWithPrefix(ZoneId.java:430) ... 10 more ] exitValue = 1 java.lang.RuntimeException: Expected to get exit value of [0] at jdk.test.lib.process.OutputAnalyzer.shouldHaveExitValue(OutputAnalyzer.java:489) at CustomTzIDCheckDST.main(CustomTzIDCheckDST.java:49) at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104) at java.base/java.lang.reflect.Method.invoke(Method.java:578) at com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:127) at java.base/java.lang.Thread.run(Thread.java:1585) JavaTest Message: Test threw exception: java.lang.RuntimeException: Expected to get exit value of [0] JavaTest Message: shutting down test STATUS:Failed.`main' threw exception: java.lang.RuntimeException: Expected to get exit value of [0] - PR: https://git.openjdk.java.net/jdk/pull/8661
Re: RFR: 8285838: DST not applying properly with zone id offset set with TZ env variable [v5]
> This fix ensures that when a lookup for a custom TZ code fails, and an > attempt is made to find the GMT offset in order to get the current time, > Daylight savings rules are applied correctly. Gaurav Chaudhari has updated the pull request incrementally with one additional commit since the last revision: 8285838: Corrected month comparison check for TZ DST - Changes: - all: https://git.openjdk.java.net/jdk/pull/8661/files - new: https://git.openjdk.java.net/jdk/pull/8661/files/d7831659..9e3b1bb4 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8661=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8661=03-04 Stats: 3 lines in 1 file changed: 1 ins; 1 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8661.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8661/head:pull/8661 PR: https://git.openjdk.java.net/jdk/pull/8661
Re: RFR: 8285838: DST not applying properly with zone id offset set with TZ env variable [v4]
On Tue, 31 May 2022 17:21:50 GMT, Naoto Sato wrote:
>> Gaurav Chaudhari has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - Merge branch '8285838' of github.com:Deigue/jdk into 8285838
>> - Merge branch '8285838' of github.com:Deigue/jdk into 8285838
>
> test/jdk/java/util/TimeZone/CustomTzIDCheckDST.java line 67:
>
>> 65: if ((month > Month.MARCH.getValue() && month <
>> Month.OCTOBER.getValue()) ||
>> 66: (month == Month.MARCH.getValue() &&
>> date.isAfter(getLastSundayOfMonth(date))) ||
>> 67: (month == Month.OCTOBER.getValue() &&
>> date.isBefore(getLastSundayOfMonth(date {
>
> I don't think these conditions are correct, as `month` is zero-based, and
> comparing it with `Month.MARCH` will be incorrect. The same holds for October.
Oh thats true, just verified this by printing out the values. Will fix by
adding 1 to the month for sensible direct comparison against the singleton
instances.
-
PR: https://git.openjdk.java.net/jdk/pull/8661
Re: RFR: 8285838: DST not applying properly with zone id offset set with TZ env variable [v4]
On Mon, 30 May 2022 15:40:37 GMT, Gaurav Chaudhari
wrote:
>> This fix ensures that when a lookup for a custom TZ code fails, and an
>> attempt is made to find the GMT offset in order to get the current time,
>> Daylight savings rules are applied correctly.
>
> Gaurav Chaudhari has updated the pull request incrementally with two
> additional commits since the last revision:
>
> - Merge branch '8285838' of github.com:Deigue/jdk into 8285838
> - Merge branch '8285838' of github.com:Deigue/jdk into 8285838
test/jdk/java/util/TimeZone/CustomTzIDCheckDST.java line 67:
> 65: if ((month > Month.MARCH.getValue() && month <
> Month.OCTOBER.getValue()) ||
> 66: (month == Month.MARCH.getValue() &&
> date.isAfter(getLastSundayOfMonth(date))) ||
> 67: (month == Month.OCTOBER.getValue() &&
> date.isBefore(getLastSundayOfMonth(date {
I don't think these conditions are correct, as `month` is zero-based, and
comparing it with `Month.MARCH` will be incorrect. The same holds for October.
-
PR: https://git.openjdk.java.net/jdk/pull/8661
Re: RFR: 8285838: DST not applying properly with zone id offset set with TZ env variable [v4]
> This fix ensures that when a lookup for a custom TZ code fails, and an > attempt is made to find the GMT offset in order to get the current time, > Daylight savings rules are applied correctly. Gaurav Chaudhari has updated the pull request incrementally with two additional commits since the last revision: - Merge branch '8285838' of github.com:Deigue/jdk into 8285838 - Merge branch '8285838' of github.com:Deigue/jdk into 8285838 - Changes: - all: https://git.openjdk.java.net/jdk/pull/8661/files - new: https://git.openjdk.java.net/jdk/pull/8661/files/b80af8bf..d7831659 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8661=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8661=02-03 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8661.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8661/head:pull/8661 PR: https://git.openjdk.java.net/jdk/pull/8661
Re: RFR: 8285838: DST not applying properly with zone id offset set with TZ env variable [v3]
> This fix ensures that when a lookup for a custom TZ code fails, and an > attempt is made to find the GMT offset in order to get the current time, > Daylight savings rules are applied correctly. Gaurav Chaudhari has updated the pull request incrementally with two additional commits since the last revision: - Merge branch '8285838' of github.com:Deigue/jdk into 8285838 - 8285838: Revised changes for Custom TZ DST test fixes. - Changes: - all: https://git.openjdk.java.net/jdk/pull/8661/files - new: https://git.openjdk.java.net/jdk/pull/8661/files/41e75494..b80af8bf Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8661=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8661=01-02 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/8661.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8661/head:pull/8661 PR: https://git.openjdk.java.net/jdk/pull/8661
Re: RFR: 8285838: DST not applying properly with zone id offset set with TZ env variable [v2]
On Tue, 17 May 2022 20:19:28 GMT, Naoto Sato wrote: >> Gaurav Chaudhari has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 8285838: Revised changes for Custom TZ DST test fixes. > > src/java.base/unix/native/libjava/TimeZone_md.c line 609: > >> 607: } >> 608: >> 609: offset = (gmt.tm_hour - localtm.tm_hour)*3600 + (gmt.tm_min - >> localtm.tm_min)*60; > > Since it is not using `timezone` anymore, we can reverse the subtraction, > i.e., `localtime` - `gmtime` so that the weird sign switch below can be > eliminated. I've reversed it and reverted the weird sign switching below this code snippet. > test/jdk/java/util/TimeZone/CustomTzIDCheckDST.java line 2: > >> 1: /* >> 2: * Copyright (c) 2001, 2022, Oracle and/or its affiliates. All rights >> reserved. > > This is a new test case, the year should be only 2022. Thanks, corrected this in the following commit > test/jdk/java/util/TimeZone/runCustomTzIDCheckDST.sh line 1: > >> 1: # > > I'd change this script into a java test case (using `.sh` is not > recommended). In fact, this causes a failure invoking `javac` with `-Xmx768m` > (from TESTVMOPTS) > There are libraries under `jdk/test/lib/` for building test jvm process and > tools. I did away with this .sh script and combined the shell script and java test into one. Initially the only purpose of this shell script was in order to set up the environment variable for the proceeding test. However, I learnt that all this could be done within java, and I consolidated both the files into one test now. > test/jdk/java/util/TimeZone/runCustomTzIDCheckDST.sh line 40: > >> 38: >> 39: OS=`uname -s` >> 40: case "$OS" in > > In case other than Linux/AIX, it would be helpful to emit some message that > the test is ignored. All moved to one java test now. - PR: https://git.openjdk.java.net/jdk/pull/8661
Re: RFR: 8285838: DST not applying properly with zone id offset set with TZ env variable [v2]
> This fix ensures that when a lookup for a custom TZ code fails, and an > attempt is made to find the GMT offset in order to get the current time, > Daylight savings rules are applied correctly. Gaurav Chaudhari has updated the pull request incrementally with one additional commit since the last revision: 8285838: Revised changes for Custom TZ DST test fixes. - Changes: - all: https://git.openjdk.java.net/jdk/pull/8661/files - new: https://git.openjdk.java.net/jdk/pull/8661/files/dcf762a5..41e75494 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8661=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8661=00-01 Stats: 94 lines in 3 files changed: 20 ins; 62 del; 12 mod Patch: https://git.openjdk.java.net/jdk/pull/8661.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8661/head:pull/8661 PR: https://git.openjdk.java.net/jdk/pull/8661
Re: RFR: 8285838: DST not applying properly with zone id offset set with TZ env variable
On Wed, 11 May 2022 18:00:31 GMT, Gaurav Chaudhari wrote: > This fix ensures that when a lookup for a custom TZ code fails, and an > attempt is made to find the GMT offset in order to get the current time, > Daylight savings rules are applied correctly. Thanks for contributing the fix. This issue has been reported in the past, but closed as will not fix (https://bugs.openjdk.java.net/browse/JDK-6992725), as implementing POSIX TZ fully requires a sizable amount of work for a rare situation. Returning a fixed offset ID with the `TZ` set to observing DST is thus a compromised solution. Even if the fix would return a custom zone observing the DST does not mean the zone is correct in winter. Having said that I agree that the derived fixed offset zone should reflect the *current* offset, as well as macOS's implementation. src/java.base/unix/native/libjava/TimeZone_md.c line 609: > 607: } > 608: > 609: offset = (gmt.tm_hour - localtm.tm_hour)*3600 + (gmt.tm_min - > localtm.tm_min)*60; Since it is not using `timezone` anymore, we can reverse the subtraction, i.e., `localtime` - `gmtime` so that the weird sign switch below can be eliminated. test/jdk/java/util/TimeZone/CustomTzIDCheckDST.java line 2: > 1: /* > 2: * Copyright (c) 2001, 2022, Oracle and/or its affiliates. All rights > reserved. This is a new test case, the year should be only 2022. test/jdk/java/util/TimeZone/CustomTzIDCheckDST.java line 27: > 25: * Specifically called by runCustomTzIDCheckDST.sh to check if Daylight > savings is > 26: * properly followed with a custom TZ code set through environment > variables. > 27: * */ Nit: Need a new line. test/jdk/java/util/TimeZone/runCustomTzIDCheckDST.sh line 1: > 1: # I'd change this script into a java test case (using `.sh` is not recommended). In fact, this causes a failure invoking `javac` with `-Xmx768m` (from TESTVMOPTS) There are libraries under `jdk/test/lib/` for building test jvm process and tools. test/jdk/java/util/TimeZone/runCustomTzIDCheckDST.sh line 40: > 38: > 39: OS=`uname -s` > 40: case "$OS" in In case other than Linux/AIX, it would be helpful to emit some message that the test is ignored. - PR: https://git.openjdk.java.net/jdk/pull/8661
Re: RFR: 8285838: DST not applying properly with zone id offset set with TZ env variable
On Wed, 11 May 2022 21:20:52 GMT, Dalibor Topic wrote: >> This fix ensures that when a lookup for a custom TZ code fails, and an >> attempt is made to find the GMT offset in order to get the current time, >> Daylight savings rules are applied correctly. > > Hi, please send me an e-mail at dalibor.to...@oracle.com, so that I can mark > your account as verified. I've sent an email yesterday @robilad - PR: https://git.openjdk.java.net/jdk/pull/8661
Re: RFR: 8285838: DST not applying properly with zone id offset set with TZ env variable
On Wed, 11 May 2022 18:00:31 GMT, Gaurav Chaudhari wrote: > This fix ensures that when a lookup for a custom TZ code fails, and an > attempt is made to find the GMT offset in order to get the current time, > Daylight savings rules are applied correctly. Hi, please send me an e-mail at dalibor.to...@oracle.com, so that I can mark your account as verified. - PR: https://git.openjdk.java.net/jdk/pull/8661
RFR: 8285838: DST not applying properly with zone id offset set with TZ env variable
This fix ensures that when a lookup for a custom TZ code fails, and an attempt is made to find the GMT offset in order to get the current time, Daylight savings rules are applied correctly. - Commit messages: - 8285838: Fix for TZ environment variable DST rules Changes: https://git.openjdk.java.net/jdk/pull/8661/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8661=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8285838 Stats: 139 lines in 3 files changed: 138 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8661.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8661/head:pull/8661 PR: https://git.openjdk.java.net/jdk/pull/8661
