[14] RFR: 8212970: TZ database in "vanguard" format support

[Roger Riggs]

Hi Naoto,

Thanks for the explanation.

Looks good.
Roger


On 7/25/19 4:04 PM, naoto.sato at 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.sato at 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.sato at 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
>>>>
>>