Re: [14] RFR: 8212970: TZ database in "vanguard" format support
Hi Naoto, Thanks for the explanation. Looks good. Roger On 7/25/19 4:04 PM, naoto.s...@oracle.com wrote: Hi Roger, On 7/25/19 7:47 AM, Roger Riggs wrote: Hi Naoto, TestZoneInfo310.java: With the composition of the tzdir path up and over to the make directory for the tzdir it might be useful to do an explicit check that the directory exists. That way if the directory structure on the build side changes, there will be a test failure makine it obvious that the dependency has changed. If the input tz data files, either in "test" tree or "make" tree, cannot be located, the test will fail which effectively reports if there is a repo structure change. So I believe it is ok as it is. Aside from it, the latest changes to eliminate the duplicates caused that regression test fail. The reason was that there were actually two "jdk11_backward" data files each in "tzdata" and "tzdata_jdk" test directories, and the contents differ! I am not sure the reason why there are two files this way (seems to be so for years), so I reverted that exact file as before. Here is the webrev reflected that: http://cr.openjdk.java.net/~naoto/8212970/webrev.12/ Naoto Looks fine. Thanks, Roger On 7/24/19 6:24 PM, naoto.s...@oracle.com wrote: Hi Joe, Thank you for the review. On 7/24/19 2:57 PM, Joe Wang wrote: Hi Naoto, The method findNegativeSavings method in TzdbZoneRulesProvider.java states that it "Find the minimum negative savings". While the result is correct since the rules all have the same value for SAVE, I wonder if that's ideal conceptually. Given a start LDT, shouldn't it be looking for the SAVE in the exact (narrower) date range (e.g. 1981 - 1989 vs 1981 - max)?. I believe it is working as such. The end year is retrieved within the method (line 879) and only the minimum negative saving values within the window is filtered. NegativeDSTTest verifies the tzdata, that is the adjusted data after import, is that correct? I wonder a comment and a bit of details in the test summary would be helpful since there is no negative data in the test itself. Good point. It is confusing. I supplied summary text in the test (also the similar line in TestZoneRules.java) Here is the updated webrev: http://cr.openjdk.java.net/~naoto/8212970/webrev.11/ Naoto Best, Joe On 7/23/19 3:15 PM, naoto.s...@oracle.com wrote: Hi, Please review the fix to the following enhancement: https://bugs.openjdk.java.net/browse/JDK-8212970 The proposed changeset is located at: https://cr.openjdk.java.net/~naoto/8212970/webrev.09/ This change aims to support the "vanguard" IANA time zone data format, which uses the negative savings and transition time beyond a day period. The change basically translates those negative savings and transition times, such as 25:00, into the ones that the current JDK recognizes, then produces the data file "tzdb.dat" at the build time. At the run time, the data file is read and interpreted as before. This way the produced tzdb.dat is compatible with the prior JDK releases so that the TZ Updater can also distribute it as a time zone update. I have also refactored redundant copy of ZoneRules file in the build directory, by dynamically importing the file under src. Thus some build related files are modified. I am hoping folks on the build-dev can review those changes. Naoto
Re: [14] RFR: 8212970: TZ database in "vanguard" format support
Thanks for these changes, +1 Stephen On Tue, 23 Jul 2019 at 23:18, wrote: > > Hi, > > Please review the fix to the following enhancement: > > https://bugs.openjdk.java.net/browse/JDK-8212970 > > The proposed changeset is located at: > > https://cr.openjdk.java.net/~naoto/8212970/webrev.09/ > > This change aims to support the "vanguard" IANA time zone data format, > which uses the negative savings and transition time beyond a day period. > The change basically translates those negative savings and transition > times, such as 25:00, into the ones that the current JDK recognizes, > then produces the data file "tzdb.dat" at the build time. At the run > time, the data file is read and interpreted as before. This way the > produced tzdb.dat is compatible with the prior JDK releases so that the > TZ Updater can also distribute it as a time zone update. > > I have also refactored redundant copy of ZoneRules file in the build > directory, by dynamically importing the file under src. Thus some build > related files are modified. I am hoping folks on the build-dev can > review those changes. > > Naoto
Re: [14] RFR: 8212970: TZ database in "vanguard" format support
Looks all good Naoto :-) -Joe On 7/25/19 2:35 PM, naoto.s...@oracle.com wrote: Hi Joe, Yes, I only removed not in-use files, i.e., 2 & 4. I sent the previous email just after confirming that all tests (open/closed) passed on a platform :-) Naoto On 7/25/19 2:24 PM, Joe Wang wrote: Hi Naoto, The legacy trap :-) Relevant files: 1. make/data/tzdata/jdk11_backward 2. test/jdk/sun/util/calendar/zi/tzdata/jdk11_backward 3. test/jdk/sun/util/calendar/zi/tzdata_jdk/jdk11_backward 4. test/jdk/sun/util/calendar/zi/tzdata_jdk/jdk11_full_backward I see you reverted changes to (1) plus removing (2) and (4). (3) and (4) differs slightly, but contains the changes previously made in (1). Probably not something to worry about since only (3) is referenced, and (4) not. Just wonder whether you've checked on this one. I assume your build and test passed without any issues. Best, Joe On 7/25/19 1:04 PM, naoto.s...@oracle.com wrote: Hi Roger, On 7/25/19 7:47 AM, Roger Riggs wrote: Hi Naoto, TestZoneInfo310.java: With the composition of the tzdir path up and over to the make directory for the tzdir it might be useful to do an explicit check that the directory exists. That way if the directory structure on the build side changes, there will be a test failure makine it obvious that the dependency has changed. If the input tz data files, either in "test" tree or "make" tree, cannot be located, the test will fail which effectively reports if there is a repo structure change. So I believe it is ok as it is. Aside from it, the latest changes to eliminate the duplicates caused that regression test fail. The reason was that there were actually two "jdk11_backward" data files each in "tzdata" and "tzdata_jdk" test directories, and the contents differ! I am not sure the reason why there are two files this way (seems to be so for years), so I reverted that exact file as before. Here is the webrev reflected that: http://cr.openjdk.java.net/~naoto/8212970/webrev.12/ Naoto Looks fine. Thanks, Roger On 7/24/19 6:24 PM, naoto.s...@oracle.com wrote: Hi Joe, Thank you for the review. On 7/24/19 2:57 PM, Joe Wang wrote: Hi Naoto, The method findNegativeSavings method in TzdbZoneRulesProvider.java states that it "Find the minimum negative savings". While the result is correct since the rules all have the same value for SAVE, I wonder if that's ideal conceptually. Given a start LDT, shouldn't it be looking for the SAVE in the exact (narrower) date range (e.g. 1981 - 1989 vs 1981 - max)?. I believe it is working as such. The end year is retrieved within the method (line 879) and only the minimum negative saving values within the window is filtered. NegativeDSTTest verifies the tzdata, that is the adjusted data after import, is that correct? I wonder a comment and a bit of details in the test summary would be helpful since there is no negative data in the test itself. Good point. It is confusing. I supplied summary text in the test (also the similar line in TestZoneRules.java) Here is the updated webrev: http://cr.openjdk.java.net/~naoto/8212970/webrev.11/ Naoto Best, Joe On 7/23/19 3:15 PM, naoto.s...@oracle.com wrote: Hi, Please review the fix to the following enhancement: https://bugs.openjdk.java.net/browse/JDK-8212970 The proposed changeset is located at: https://cr.openjdk.java.net/~naoto/8212970/webrev.09/ This change aims to support the "vanguard" IANA time zone data format, which uses the negative savings and transition time beyond a day period. The change basically translates those negative savings and transition times, such as 25:00, into the ones that the current JDK recognizes, then produces the data file "tzdb.dat" at the build time. At the run time, the data file is read and interpreted as before. This way the produced tzdb.dat is compatible with the prior JDK releases so that the TZ Updater can also distribute it as a time zone update. I have also refactored redundant copy of ZoneRules file in the build directory, by dynamically importing the file under src. Thus some build related files are modified. I am hoping folks on the build-dev can review those changes. Naoto
Re: [14] RFR: 8212970: TZ database in "vanguard" format support
Hi Joe, Yes, I only removed not in-use files, i.e., 2 & 4. I sent the previous email just after confirming that all tests (open/closed) passed on a platform :-) Naoto On 7/25/19 2:24 PM, Joe Wang wrote: Hi Naoto, The legacy trap :-) Relevant files: 1. make/data/tzdata/jdk11_backward 2. test/jdk/sun/util/calendar/zi/tzdata/jdk11_backward 3. test/jdk/sun/util/calendar/zi/tzdata_jdk/jdk11_backward 4. test/jdk/sun/util/calendar/zi/tzdata_jdk/jdk11_full_backward I see you reverted changes to (1) plus removing (2) and (4). (3) and (4) differs slightly, but contains the changes previously made in (1). Probably not something to worry about since only (3) is referenced, and (4) not. Just wonder whether you've checked on this one. I assume your build and test passed without any issues. Best, Joe On 7/25/19 1:04 PM, naoto.s...@oracle.com wrote: Hi Roger, On 7/25/19 7:47 AM, Roger Riggs wrote: Hi Naoto, TestZoneInfo310.java: With the composition of the tzdir path up and over to the make directory for the tzdir it might be useful to do an explicit check that the directory exists. That way if the directory structure on the build side changes, there will be a test failure makine it obvious that the dependency has changed. If the input tz data files, either in "test" tree or "make" tree, cannot be located, the test will fail which effectively reports if there is a repo structure change. So I believe it is ok as it is. Aside from it, the latest changes to eliminate the duplicates caused that regression test fail. The reason was that there were actually two "jdk11_backward" data files each in "tzdata" and "tzdata_jdk" test directories, and the contents differ! I am not sure the reason why there are two files this way (seems to be so for years), so I reverted that exact file as before. Here is the webrev reflected that: http://cr.openjdk.java.net/~naoto/8212970/webrev.12/ Naoto Looks fine. Thanks, Roger On 7/24/19 6:24 PM, naoto.s...@oracle.com wrote: Hi Joe, Thank you for the review. On 7/24/19 2:57 PM, Joe Wang wrote: Hi Naoto, The method findNegativeSavings method in TzdbZoneRulesProvider.java states that it "Find the minimum negative savings". While the result is correct since the rules all have the same value for SAVE, I wonder if that's ideal conceptually. Given a start LDT, shouldn't it be looking for the SAVE in the exact (narrower) date range (e.g. 1981 - 1989 vs 1981 - max)?. I believe it is working as such. The end year is retrieved within the method (line 879) and only the minimum negative saving values within the window is filtered. NegativeDSTTest verifies the tzdata, that is the adjusted data after import, is that correct? I wonder a comment and a bit of details in the test summary would be helpful since there is no negative data in the test itself. Good point. It is confusing. I supplied summary text in the test (also the similar line in TestZoneRules.java) Here is the updated webrev: http://cr.openjdk.java.net/~naoto/8212970/webrev.11/ Naoto Best, Joe On 7/23/19 3:15 PM, naoto.s...@oracle.com wrote: Hi, Please review the fix to the following enhancement: https://bugs.openjdk.java.net/browse/JDK-8212970 The proposed changeset is located at: https://cr.openjdk.java.net/~naoto/8212970/webrev.09/ This change aims to support the "vanguard" IANA time zone data format, which uses the negative savings and transition time beyond a day period. The change basically translates those negative savings and transition times, such as 25:00, into the ones that the current JDK recognizes, then produces the data file "tzdb.dat" at the build time. At the run time, the data file is read and interpreted as before. This way the produced tzdb.dat is compatible with the prior JDK releases so that the TZ Updater can also distribute it as a time zone update. I have also refactored redundant copy of ZoneRules file in the build directory, by dynamically importing the file under src. Thus some build related files are modified. I am hoping folks on the build-dev can review those changes. Naoto
Re: [14] RFR: 8212970: TZ database in "vanguard" format support
Hi Naoto, The legacy trap :-) Relevant files: 1. make/data/tzdata/jdk11_backward 2. test/jdk/sun/util/calendar/zi/tzdata/jdk11_backward 3. test/jdk/sun/util/calendar/zi/tzdata_jdk/jdk11_backward 4. test/jdk/sun/util/calendar/zi/tzdata_jdk/jdk11_full_backward I see you reverted changes to (1) plus removing (2) and (4). (3) and (4) differs slightly, but contains the changes previously made in (1). Probably not something to worry about since only (3) is referenced, and (4) not. Just wonder whether you've checked on this one. I assume your build and test passed without any issues. Best, Joe On 7/25/19 1:04 PM, naoto.s...@oracle.com wrote: Hi Roger, On 7/25/19 7:47 AM, Roger Riggs wrote: Hi Naoto, TestZoneInfo310.java: With the composition of the tzdir path up and over to the make directory for the tzdir it might be useful to do an explicit check that the directory exists. That way if the directory structure on the build side changes, there will be a test failure makine it obvious that the dependency has changed. If the input tz data files, either in "test" tree or "make" tree, cannot be located, the test will fail which effectively reports if there is a repo structure change. So I believe it is ok as it is. Aside from it, the latest changes to eliminate the duplicates caused that regression test fail. The reason was that there were actually two "jdk11_backward" data files each in "tzdata" and "tzdata_jdk" test directories, and the contents differ! I am not sure the reason why there are two files this way (seems to be so for years), so I reverted that exact file as before. Here is the webrev reflected that: http://cr.openjdk.java.net/~naoto/8212970/webrev.12/ Naoto Looks fine. Thanks, Roger On 7/24/19 6:24 PM, naoto.s...@oracle.com wrote: Hi Joe, Thank you for the review. On 7/24/19 2:57 PM, Joe Wang wrote: Hi Naoto, The method findNegativeSavings method in TzdbZoneRulesProvider.java states that it "Find the minimum negative savings". While the result is correct since the rules all have the same value for SAVE, I wonder if that's ideal conceptually. Given a start LDT, shouldn't it be looking for the SAVE in the exact (narrower) date range (e.g. 1981 - 1989 vs 1981 - max)?. I believe it is working as such. The end year is retrieved within the method (line 879) and only the minimum negative saving values within the window is filtered. NegativeDSTTest verifies the tzdata, that is the adjusted data after import, is that correct? I wonder a comment and a bit of details in the test summary would be helpful since there is no negative data in the test itself. Good point. It is confusing. I supplied summary text in the test (also the similar line in TestZoneRules.java) Here is the updated webrev: http://cr.openjdk.java.net/~naoto/8212970/webrev.11/ Naoto Best, Joe On 7/23/19 3:15 PM, naoto.s...@oracle.com wrote: Hi, Please review the fix to the following enhancement: https://bugs.openjdk.java.net/browse/JDK-8212970 The proposed changeset is located at: https://cr.openjdk.java.net/~naoto/8212970/webrev.09/ This change aims to support the "vanguard" IANA time zone data format, which uses the negative savings and transition time beyond a day period. The change basically translates those negative savings and transition times, such as 25:00, into the ones that the current JDK recognizes, then produces the data file "tzdb.dat" at the build time. At the run time, the data file is read and interpreted as before. This way the produced tzdb.dat is compatible with the prior JDK releases so that the TZ Updater can also distribute it as a time zone update. I have also refactored redundant copy of ZoneRules file in the build directory, by dynamically importing the file under src. Thus some build related files are modified. I am hoping folks on the build-dev can review those changes. Naoto
Re: [14] RFR: 8212970: TZ database in "vanguard" format support
Hi Roger, On 7/25/19 7:47 AM, Roger Riggs wrote: Hi Naoto, TestZoneInfo310.java: With the composition of the tzdir path up and over to the make directory for the tzdir it might be useful to do an explicit check that the directory exists. That way if the directory structure on the build side changes, there will be a test failure makine it obvious that the dependency has changed. If the input tz data files, either in "test" tree or "make" tree, cannot be located, the test will fail which effectively reports if there is a repo structure change. So I believe it is ok as it is. Aside from it, the latest changes to eliminate the duplicates caused that regression test fail. The reason was that there were actually two "jdk11_backward" data files each in "tzdata" and "tzdata_jdk" test directories, and the contents differ! I am not sure the reason why there are two files this way (seems to be so for years), so I reverted that exact file as before. Here is the webrev reflected that: http://cr.openjdk.java.net/~naoto/8212970/webrev.12/ Naoto Looks fine. Thanks, Roger On 7/24/19 6:24 PM, naoto.s...@oracle.com wrote: Hi Joe, Thank you for the review. On 7/24/19 2:57 PM, Joe Wang wrote: Hi Naoto, The method findNegativeSavings method in TzdbZoneRulesProvider.java states that it "Find the minimum negative savings". While the result is correct since the rules all have the same value for SAVE, I wonder if that's ideal conceptually. Given a start LDT, shouldn't it be looking for the SAVE in the exact (narrower) date range (e.g. 1981 - 1989 vs 1981 - max)?. I believe it is working as such. The end year is retrieved within the method (line 879) and only the minimum negative saving values within the window is filtered. NegativeDSTTest verifies the tzdata, that is the adjusted data after import, is that correct? I wonder a comment and a bit of details in the test summary would be helpful since there is no negative data in the test itself. Good point. It is confusing. I supplied summary text in the test (also the similar line in TestZoneRules.java) Here is the updated webrev: http://cr.openjdk.java.net/~naoto/8212970/webrev.11/ Naoto Best, Joe On 7/23/19 3:15 PM, naoto.s...@oracle.com wrote: Hi, Please review the fix to the following enhancement: https://bugs.openjdk.java.net/browse/JDK-8212970 The proposed changeset is located at: https://cr.openjdk.java.net/~naoto/8212970/webrev.09/ This change aims to support the "vanguard" IANA time zone data format, which uses the negative savings and transition time beyond a day period. The change basically translates those negative savings and transition times, such as 25:00, into the ones that the current JDK recognizes, then produces the data file "tzdb.dat" at the build time. At the run time, the data file is read and interpreted as before. This way the produced tzdb.dat is compatible with the prior JDK releases so that the TZ Updater can also distribute it as a time zone update. I have also refactored redundant copy of ZoneRules file in the build directory, by dynamically importing the file under src. Thus some build related files are modified. I am hoping folks on the build-dev can review those changes. Naoto
Re: [14] RFR: 8212970: TZ database in "vanguard" format support
Build change looks good. /Erik On 2019-07-24 15:24, naoto.s...@oracle.com wrote: Hi Joe, Thank you for the review. On 7/24/19 2:57 PM, Joe Wang wrote: Hi Naoto, The method findNegativeSavings method in TzdbZoneRulesProvider.java states that it "Find the minimum negative savings". While the result is correct since the rules all have the same value for SAVE, I wonder if that's ideal conceptually. Given a start LDT, shouldn't it be looking for the SAVE in the exact (narrower) date range (e.g. 1981 - 1989 vs 1981 - max)?. I believe it is working as such. The end year is retrieved within the method (line 879) and only the minimum negative saving values within the window is filtered. NegativeDSTTest verifies the tzdata, that is the adjusted data after import, is that correct? I wonder a comment and a bit of details in the test summary would be helpful since there is no negative data in the test itself. Good point. It is confusing. I supplied summary text in the test (also the similar line in TestZoneRules.java) Here is the updated webrev: http://cr.openjdk.java.net/~naoto/8212970/webrev.11/ Naoto Best, Joe On 7/23/19 3:15 PM, naoto.s...@oracle.com wrote: Hi, Please review the fix to the following enhancement: https://bugs.openjdk.java.net/browse/JDK-8212970 The proposed changeset is located at: https://cr.openjdk.java.net/~naoto/8212970/webrev.09/ This change aims to support the "vanguard" IANA time zone data format, which uses the negative savings and transition time beyond a day period. The change basically translates those negative savings and transition times, such as 25:00, into the ones that the current JDK recognizes, then produces the data file "tzdb.dat" at the build time. At the run time, the data file is read and interpreted as before. This way the produced tzdb.dat is compatible with the prior JDK releases so that the TZ Updater can also distribute it as a time zone update. I have also refactored redundant copy of ZoneRules file in the build directory, by dynamically importing the file under src. Thus some build related files are modified. I am hoping folks on the build-dev can review those changes. Naoto
Re: [14] RFR: 8212970: TZ database in "vanguard" format support
Hi Naoto, TestZoneInfo310.java: With the composition of the tzdir path up and over to the make directory for the tzdir it might be useful to do an explicit check that the directory exists. That way if the directory structure on the build side changes, there will be a test failure makine it obvious that the dependency has changed. Looks fine. Thanks, Roger On 7/24/19 6:24 PM, naoto.s...@oracle.com wrote: Hi Joe, Thank you for the review. On 7/24/19 2:57 PM, Joe Wang wrote: Hi Naoto, The method findNegativeSavings method in TzdbZoneRulesProvider.java states that it "Find the minimum negative savings". While the result is correct since the rules all have the same value for SAVE, I wonder if that's ideal conceptually. Given a start LDT, shouldn't it be looking for the SAVE in the exact (narrower) date range (e.g. 1981 - 1989 vs 1981 - max)?. I believe it is working as such. The end year is retrieved within the method (line 879) and only the minimum negative saving values within the window is filtered. NegativeDSTTest verifies the tzdata, that is the adjusted data after import, is that correct? I wonder a comment and a bit of details in the test summary would be helpful since there is no negative data in the test itself. Good point. It is confusing. I supplied summary text in the test (also the similar line in TestZoneRules.java) Here is the updated webrev: http://cr.openjdk.java.net/~naoto/8212970/webrev.11/ Naoto Best, Joe On 7/23/19 3:15 PM, naoto.s...@oracle.com wrote: Hi, Please review the fix to the following enhancement: https://bugs.openjdk.java.net/browse/JDK-8212970 The proposed changeset is located at: https://cr.openjdk.java.net/~naoto/8212970/webrev.09/ This change aims to support the "vanguard" IANA time zone data format, which uses the negative savings and transition time beyond a day period. The change basically translates those negative savings and transition times, such as 25:00, into the ones that the current JDK recognizes, then produces the data file "tzdb.dat" at the build time. At the run time, the data file is read and interpreted as before. This way the produced tzdb.dat is compatible with the prior JDK releases so that the TZ Updater can also distribute it as a time zone update. I have also refactored redundant copy of ZoneRules file in the build directory, by dynamically importing the file under src. Thus some build related files are modified. I am hoping folks on the build-dev can review those changes. Naoto
Re: [14] RFR: 8212970: TZ database in "vanguard" format support
Thanks Naoto. Looks good. -Joe On 7/24/19 3:24 PM, naoto.s...@oracle.com wrote: Hi Joe, Thank you for the review. On 7/24/19 2:57 PM, Joe Wang wrote: Hi Naoto, The method findNegativeSavings method in TzdbZoneRulesProvider.java states that it "Find the minimum negative savings". While the result is correct since the rules all have the same value for SAVE, I wonder if that's ideal conceptually. Given a start LDT, shouldn't it be looking for the SAVE in the exact (narrower) date range (e.g. 1981 - 1989 vs 1981 - max)?. I believe it is working as such. The end year is retrieved within the method (line 879) and only the minimum negative saving values within the window is filtered. NegativeDSTTest verifies the tzdata, that is the adjusted data after import, is that correct? I wonder a comment and a bit of details in the test summary would be helpful since there is no negative data in the test itself. Good point. It is confusing. I supplied summary text in the test (also the similar line in TestZoneRules.java) Here is the updated webrev: http://cr.openjdk.java.net/~naoto/8212970/webrev.11/ Naoto Best, Joe On 7/23/19 3:15 PM, naoto.s...@oracle.com wrote: Hi, Please review the fix to the following enhancement: https://bugs.openjdk.java.net/browse/JDK-8212970 The proposed changeset is located at: https://cr.openjdk.java.net/~naoto/8212970/webrev.09/ This change aims to support the "vanguard" IANA time zone data format, which uses the negative savings and transition time beyond a day period. The change basically translates those negative savings and transition times, such as 25:00, into the ones that the current JDK recognizes, then produces the data file "tzdb.dat" at the build time. At the run time, the data file is read and interpreted as before. This way the produced tzdb.dat is compatible with the prior JDK releases so that the TZ Updater can also distribute it as a time zone update. I have also refactored redundant copy of ZoneRules file in the build directory, by dynamically importing the file under src. Thus some build related files are modified. I am hoping folks on the build-dev can review those changes. Naoto
Re: [14] RFR: 8212970: TZ database in "vanguard" format support
Hi Joe, Thank you for the review. On 7/24/19 2:57 PM, Joe Wang wrote: Hi Naoto, The method findNegativeSavings method in TzdbZoneRulesProvider.java states that it "Find the minimum negative savings". While the result is correct since the rules all have the same value for SAVE, I wonder if that's ideal conceptually. Given a start LDT, shouldn't it be looking for the SAVE in the exact (narrower) date range (e.g. 1981 - 1989 vs 1981 - max)?. I believe it is working as such. The end year is retrieved within the method (line 879) and only the minimum negative saving values within the window is filtered. NegativeDSTTest verifies the tzdata, that is the adjusted data after import, is that correct? I wonder a comment and a bit of details in the test summary would be helpful since there is no negative data in the test itself. Good point. It is confusing. I supplied summary text in the test (also the similar line in TestZoneRules.java) Here is the updated webrev: http://cr.openjdk.java.net/~naoto/8212970/webrev.11/ Naoto Best, Joe On 7/23/19 3:15 PM, naoto.s...@oracle.com wrote: Hi, Please review the fix to the following enhancement: https://bugs.openjdk.java.net/browse/JDK-8212970 The proposed changeset is located at: https://cr.openjdk.java.net/~naoto/8212970/webrev.09/ This change aims to support the "vanguard" IANA time zone data format, which uses the negative savings and transition time beyond a day period. The change basically translates those negative savings and transition times, such as 25:00, into the ones that the current JDK recognizes, then produces the data file "tzdb.dat" at the build time. At the run time, the data file is read and interpreted as before. This way the produced tzdb.dat is compatible with the prior JDK releases so that the TZ Updater can also distribute it as a time zone update. I have also refactored redundant copy of ZoneRules file in the build directory, by dynamically importing the file under src. Thus some build related files are modified. I am hoping folks on the build-dev can review those changes. Naoto
Re: [14] RFR: 8212970: TZ database in "vanguard" format support
Hi Naoto, The method findNegativeSavings method in TzdbZoneRulesProvider.java states that it "Find the minimum negative savings". While the result is correct since the rules all have the same value for SAVE, I wonder if that's ideal conceptually. Given a start LDT, shouldn't it be looking for the SAVE in the exact (narrower) date range (e.g. 1981 - 1989 vs 1981 - max)?. NegativeDSTTest verifies the tzdata, that is the adjusted data after import, is that correct? I wonder a comment and a bit of details in the test summary would be helpful since there is no negative data in the test itself. Best, Joe On 7/23/19 3:15 PM, naoto.s...@oracle.com wrote: Hi, Please review the fix to the following enhancement: https://bugs.openjdk.java.net/browse/JDK-8212970 The proposed changeset is located at: https://cr.openjdk.java.net/~naoto/8212970/webrev.09/ This change aims to support the "vanguard" IANA time zone data format, which uses the negative savings and transition time beyond a day period. The change basically translates those negative savings and transition times, such as 25:00, into the ones that the current JDK recognizes, then produces the data file "tzdb.dat" at the build time. At the run time, the data file is read and interpreted as before. This way the produced tzdb.dat is compatible with the prior JDK releases so that the TZ Updater can also distribute it as a time zone update. I have also refactored redundant copy of ZoneRules file in the build directory, by dynamically importing the file under src. Thus some build related files are modified. I am hoping folks on the build-dev can review those changes. Naoto
Re: [14] RFR: 8212970: TZ database in "vanguard" format support
Hi Roger, On 7/24/19 1:09 PM, naoto.s...@oracle.com wrote: Thanks for the review, Roger. On 7/24/19 12:47 PM, Roger Riggs wrote: Hi Naoto, Adjusting the data during import looks fine. Typo: TzdbZoneRulesProvider.java:504 "ususally" -> "usually" Will fix it. Removing the source duplication is good. Is there a way to remove the duplication of the TZDATA files themselves? make/data/tzdata/* and test/jdk/sun/til/calendar/zi/* I thought about it, but it turned out that the copyright is different (with/without classpath exception). So I just leave them as they are. I was wrong about this. Since we are trying to eliminate the duplicates, copyright difference does not matter :-) Here is the update: http://cr.openjdk.java.net/~naoto/8212970/webrev.10/ Naoto Naoto Looks good, Roger On 7/23/19 6:15 PM, naoto.s...@oracle.com wrote: Hi, Please review the fix to the following enhancement: https://bugs.openjdk.java.net/browse/JDK-8212970 The proposed changeset is located at: https://cr.openjdk.java.net/~naoto/8212970/webrev.09/ This change aims to support the "vanguard" IANA time zone data format, which uses the negative savings and transition time beyond a day period. The change basically translates those negative savings and transition times, such as 25:00, into the ones that the current JDK recognizes, then produces the data file "tzdb.dat" at the build time. At the run time, the data file is read and interpreted as before. This way the produced tzdb.dat is compatible with the prior JDK releases so that the TZ Updater can also distribute it as a time zone update. I have also refactored redundant copy of ZoneRules file in the build directory, by dynamically importing the file under src. Thus some build related files are modified. I am hoping folks on the build-dev can review those changes. Naoto
Re: [14] RFR: 8212970: TZ database in "vanguard" format support
Thanks for the review, Roger. On 7/24/19 12:47 PM, Roger Riggs wrote: Hi Naoto, Adjusting the data during import looks fine. Typo: TzdbZoneRulesProvider.java:504 "ususally" -> "usually" Will fix it. Removing the source duplication is good. Is there a way to remove the duplication of the TZDATA files themselves? make/data/tzdata/* and test/jdk/sun/til/calendar/zi/* I thought about it, but it turned out that the copyright is different (with/without classpath exception). So I just leave them as they are. Naoto Looks good, Roger On 7/23/19 6:15 PM, naoto.s...@oracle.com wrote: Hi, Please review the fix to the following enhancement: https://bugs.openjdk.java.net/browse/JDK-8212970 The proposed changeset is located at: https://cr.openjdk.java.net/~naoto/8212970/webrev.09/ This change aims to support the "vanguard" IANA time zone data format, which uses the negative savings and transition time beyond a day period. The change basically translates those negative savings and transition times, such as 25:00, into the ones that the current JDK recognizes, then produces the data file "tzdb.dat" at the build time. At the run time, the data file is read and interpreted as before. This way the produced tzdb.dat is compatible with the prior JDK releases so that the TZ Updater can also distribute it as a time zone update. I have also refactored redundant copy of ZoneRules file in the build directory, by dynamically importing the file under src. Thus some build related files are modified. I am hoping folks on the build-dev can review those changes. Naoto
Re: [14] RFR: 8212970: TZ database in "vanguard" format support
Hi Naoto, Adjusting the data during import looks fine. Typo: TzdbZoneRulesProvider.java:504 "ususally" -> "usually" Removing the source duplication is good. Is there a way to remove the duplication of the TZDATA files themselves? make/data/tzdata/* and test/jdk/sun/til/calendar/zi/* Looks good, Roger On 7/23/19 6:15 PM, naoto.s...@oracle.com wrote: Hi, Please review the fix to the following enhancement: https://bugs.openjdk.java.net/browse/JDK-8212970 The proposed changeset is located at: https://cr.openjdk.java.net/~naoto/8212970/webrev.09/ This change aims to support the "vanguard" IANA time zone data format, which uses the negative savings and transition time beyond a day period. The change basically translates those negative savings and transition times, such as 25:00, into the ones that the current JDK recognizes, then produces the data file "tzdb.dat" at the build time. At the run time, the data file is read and interpreted as before. This way the produced tzdb.dat is compatible with the prior JDK releases so that the TZ Updater can also distribute it as a time zone update. I have also refactored redundant copy of ZoneRules file in the build directory, by dynamically importing the file under src. Thus some build related files are modified. I am hoping folks on the build-dev can review those changes. Naoto
[14] RFR: 8212970: TZ database in "vanguard" format support
Hi, Please review the fix to the following enhancement: https://bugs.openjdk.java.net/browse/JDK-8212970 The proposed changeset is located at: https://cr.openjdk.java.net/~naoto/8212970/webrev.09/ This change aims to support the "vanguard" IANA time zone data format, which uses the negative savings and transition time beyond a day period. The change basically translates those negative savings and transition times, such as 25:00, into the ones that the current JDK recognizes, then produces the data file "tzdb.dat" at the build time. At the run time, the data file is read and interpreted as before. This way the produced tzdb.dat is compatible with the prior JDK releases so that the TZ Updater can also distribute it as a time zone update. I have also refactored redundant copy of ZoneRules file in the build directory, by dynamically importing the file under src. Thus some build related files are modified. I am hoping folks on the build-dev can review those changes. Naoto
