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

[Xueming Shen]

Kumar, Alan,

Here is the latest webrev of the changes based on the latest CCC.

http://cr.openjdk.java.net/~sherman/8015666/webrev

-Sherman


On 07/02/2013 11:29 AM, Xueming Shen wrote:
> On 06/28/2013 07:47 AM, Kumar Srinivasan wrote:
>> 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,
>
> I have the dostime "cached" in XEntry, so the writeCEN() and writeLOC() will
> always write out the same local msdos time. The cache should help the perf
> a little, as the javaToDosTime() now only invoked once for the same entry.
>
> Nothing needs to be updated in unpack side now. (I took a look at the API,
> it appears there is no way to do anything on unpack side to workaround
> this issue, without the possibility of breaking someone's code)
>
> http://cr.openjdk.java.net/~sherman/8015666/webrev/
>
> -Sherman
>
>>
>> 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
>>>>
>>>
>>
>