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

Tue Nov 30 15:23:14 UTC 2021

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.
>> 
>> Signed-off-by: Andrew Leonard <anleonar at redhat.com>
>
> Andrew Leonard has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - 8276766: Enable jar and jmod to produce deterministic timestamped content
>    
>    Signed-off-by: Andrew Leonard <anleonar at redhat.com>
>  - 8276766: Enable jar and jmod to produce deterministic timestamped content
>    
>    Signed-off-by: Andrew Leonard <anleonar at redhat.com>

Thanks for taking a look Jaikiran,
I concur with the above. I'm currently looking to make the rest of jar/jmod
deterministic as there
are several currentTimeMillis() used... i'll probably update this to be at
least consistent
with the others.
Thanks
Andrew

On Tue, Nov 30, 2021 at 3:02 PM mlbridge[bot] ***@***.***>
wrote:

> *Mailing list message from Jaikiran Pai ***@***.***> on
> core-libs-dev ***@***.***>:*
>
> 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/
> /pull/5486 <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//pull/5486 <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 ***@***.*** date and time fields} of the zip file
> ???? * entry and encoded in standard ***@***.*** MS-DOS date and time format}.
> ???? * The ***@***.*** 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//pull/5486 <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
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/openjdk/jdk/pull/6481#issuecomment-982719481>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AHQDDN7UQMX4PV46ZZCALE3UOTRRJANCNFSM5IMSJY4Q>
> .
> Triage notifications on the go with GitHub Mobile for iOS
> <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
> or Android
> <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
>
>

-------------

PR: https://git.openjdk.java.net/jdk/pull/6481