RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v9]

Jaikiran Pai jai.forums2013 at gmail.com
Tue Nov 30 15:00:22 UTC 2021


Hello Andrew,

On 30/11/21 7:32 pm, Andrew Leonard wrote:
> On Mon, 29 Nov 2021 16:22:30 GMT, Andrew Leonard <aleonard at openjdk.org> wrote:
>
>>> Add a new --source-date <TIMESTAMP> (epoch seconds) option to jar and jmod to allow specification of time to use for created/updated jar/jmod entries. This then allows the ability to make the content deterministic.
>>>
>>>
> However, it looks like this behavior to not set extended mtime within the xdostime range has just been changed by a recent PR: https://github.com/openjdk/jdk/pull/5486/files which has changed jartool.Main using zipEntry.setTime(time) to use zipEntry.setLastModifiedTime(time), the later sets mtime always regardless of xdostime.

When I changed the implementation in sun.tools.jar.Main to use 
setLastModifiedTime(FileTime) instead of the previous setTime(long), I 
hadn't paid close attention to the implementation of these methods in 
java.util.zip.ZipEntry. Now that you brought this up, I had a more 
closer look at their implementation.

So it looks like the implementation of setTime(long) conditionally sets 
the extended timestamp fields in optional extra data of the zip entry. 
That condition checks to see if the time value being set is before 1980 
or after 2099 and if it passes this condition, it goes and sets the 
extended timestamp fields. So in practice, for jar creation/updation in 
sun.tools.jar.Main, the previous code using setTime(long) wouldn't have 
set the extended timestamp fields, because the current year(s) aren't 
before 1980 or after 2099. The implementation of 
java.util.zip.ZipEntry.setLastModifiedTime(FileTime) on the other hand 
has no such conditional checks and (as noted in the javadoc of that 
method) goes ahead and sets the time value in the extended timestamp 
fields of that zip entry.

So yes, the change that went in to 
https://github.com/openjdk/jdk/pull/5486 did introduce this subtle 
change in the generated zip/jar entries. Having said that, the 
setTime(long) on java.util.zip.ZipEntry makes no mention of these 
conditional checks. It currently states:

      * Sets the last modification time of the entry.
      *
      * <p> If the entry is output to a ZIP file or ZIP file formatted
      * output stream the last modification time set by this method will
      * be stored into the {@code date and time fields} of the zip file
      * entry and encoded in standard {@code MS-DOS date and time format}.
      * The {@link java.util.TimeZone#getDefault() default TimeZone} is
      * used to convert the epoch time to the MS-DOS data and time.

So it doesn't even make a mention of setting extended timestamp fields, 
let along setting them on a particular condition. So I'm unsure whether 
switching to setLastModifiedTime(FileTime) instead of setTime(long) was 
a bad thing.

> The implications of https://bugs.openjdk.java.net/browse/JDK-8073497 might also apply to FileTime unnecessary initialization slowing VM startup, however if FileTime is already regularly referenced during startup, then it wont.. If this is the case then way forward (1) would be ok...
> @AlanBateman was that an intentional change? @jaikiran

I had a look at that JBS issue now. From what I understand in its 
description and goal, the idea was to prevent initialization of 
java.util.Date utilities very early on during the startup. I had a quick 
look at the code in ZipEntry and how it behaves when it constructs a 
ZipEntry instance out of the zip file data. With the change in 
https://github.com/openjdk/jdk/pull/5486, the "mtime" (which represents 
extended timestamp field) will be set in the zip file entry, so there 
would be an attempt to create a FileTime out of it. The call that does 
that appears to be "unixTimeToFileTime" in ZipEntry and as far as I can 
see it doesn't end up using any java.util.Date classes. Of course, I 
haven't yet looked or experimented to verify and be absolutely sure 
about it, but from a cursory check it doesn't look like this would 
contribute to pulling in java.util.Date utilities.

-Jaikiran



More information about the compiler-dev mailing list