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

[Joe Wang]

Looks all good Naoto :-)

-Joe

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