RFR: 8015666: test/tools/pack200/TimeStamp.java failing

[Kumar Srinivasan]

Some nits while reading the changes:
1. ZipEntry.java
    a. typo:

+     * Sets the laste access time of the entry.


    b. extra space

+                case EXTID_ZIP64 :

2. ZipOutputStream.java
I think it would be nice to have the flags 0x1, 0x2 and 0x4 defined
as constants, this will also help a casual reader as to what this means.


Besides my previous concern with finish(), everything else appears to be ok.

Kumar


> On 06/27/2013 10:04 AM, Kumar Srinivasan wrote:
>> Hi Sherman,
>>
>> I started looking at this, my initial comment, the Unpacker.unpack
>> does not close its output and  we allow multiple pack  files to be 
>> concatenated,
>> I am assuming out.finish() will allow further jar files to be appended ?
>> or would this cause a problem ?
>
> No, out.finish() will not allow further entry appending.  Then, it 
> appears
> we need to have a different approach to "finish" the Jar/ZipOutputStream.
> What need to be done here is that either out.close/finish() need to be
> invoked under the UTC locale as well (to output the time stamps in cen
> table correctly).  This is another "incompatible" change of the previous
> change, in which the msdosTime<->javaTime conversion no longer
> occurs during the ZipEntry.set/getTime(), but during the read and write
> the ZipEntry from/to the zip file.
>
> -Sherman
>
>
>>
>> Kumar
>>
>>> Hi,
>>>
>>> The zip time related changes[1] I pushed back last month appears
>>> to have the compatibility risk of breaking existing code. The main
>>> idea in that changeset is to use the more accurate and timezone
>>> insensitive utc time stored in the extra field for the 
>>> ZipEntry.set/getTime()
>>> if possible. However it turns out the reality is that the code out 
>>> there
>>> might have already had some interesting workaround/hack solution
>>> to workaround the problem that the time stamp stored in the "standard'
>>> zip entry header is a MS-DOS standard date/time, which is a "local
>>> date/time" and sensitive to timezone, in which, if the zip is archived
>>> in time zone A (our implementation converts the "java" time to dos
>>> time by using the default tz A) and then transferred/un-archived in
>>> a different zone B (use default tz B to convert back to java time), we
>>> have a time stamp mess up. The "workaround" from pack200 for this
>>> issue when pack/unpacking a jar file is to "specify/recommend/suggest"
>>> in its spec that the "time zone" in a jar file entry is assumed to 
>>> be "UTC",
>>> so the pack/unpack200 implementation set the "default time" to utc
>>> before the pack/unpack and set it back to the original after that. 
>>> It worked
>>> "perfectly" for a roundtrip pack/unpacking, until the changeset [2], in
>>> which ZipEntry.getTime() (packing) returns a real utc time and the 
>>> following
>>> ZipEntry.setTime() (unpacking), then mess up the MS-DOS date/time
>>> entry, this is the root cause of this regression.
>>>
>>> Given the facts that
>>> (1) there are actually two real physical time stamps in a zip file 
>>> header,
>>> one is in the date/time fields, which is MS-DOS-local-date/time-with-2-
>>> seconds-granularity , one is in the extra data field, which is 
>>> UTC-1-second
>>> -granularity
>>> (2) and there are applications over there that have dependency on the
>>> MS-DOS date/time stamp.
>>>
>>> I'm proposing the following approach to add the functionality of 
>>> supporting
>>> the "utc-date/time-with-1-second granularity" and keep the old behavior
>>> of the get/setTime() of the ZipEntry.
>>>
>>> (1) keep the time/setTime()/getTime() for the MS-DOS standard 
>>> date/time.
>>>      To set via the old setTime() will only store the time into 
>>> zip's standard
>>>      date/time field, which is in MS-DOS date/time. And getTime() 
>>> only returns
>>>      the date/time from that field, when read from the zip file/stream.
>>> (2) add mtime/set/getLastModifiedTime() to work on the UTC time fields,
>>>      and the last modified time set via the new method will also set 
>>> the "time",
>>>      and the getLastModifiedTime() also returns the "time", if the 
>>> UTC time
>>>      stamp fields are not set in the zip file header. The idea is 
>>> that for the new
>>>      application, the recommendation is to use 
>>> ZipEntry.set/getLastModifiedTime()
>>>      for better/correct time stamp, but the existing apps keep the 
>>> same behavior.
>>> (3) jar and ZipOutputStream are updated to use the 
>>> set/getLastModifiedTime().
>>> (4) Pack/unpack continues to use the set/getTime(), so the current 
>>> workaround
>>>      continues work. I will leave this to Kuma to decide how it 
>>> should be handled
>>>      going forward. (there are two facts need to be considered here, 
>>> a) the
>>>      existing jar file might not have the utc time instored, and b) 
>>> all "extra" data
>>>      are wiped out during the pack/unpacking process)
>>> (5) additionally add another pair of atime/get/setLastAccessTime and
>>>      ctime/get/setCreationTime().
>>> (6) The newly added 3 pairs of the m/a/ctime get/set methods use the 
>>> "new"
>>>      nio FileTime, instead of the "long". This may add some 
>>> additional cost of
>>>      conversion when working with them, but may also help improve the
>>>      performance if the time stamps are directly from nio file 
>>> system when
>>>      get/set XYZTime. Good/bad?
>>>
>>> http://cr.openjdk.java.net/~sherman/8015666/webrev/
>>>
>>> Comment, option and suggestion are appreciated.
>>>
>>> -Sherman
>>>
>>> [1] http://hg.openjdk.java.net/jdk8/tl/jdk/rev/90df6756406f
>>
>