8327466: ct.sym zip not reproducible across build environment timezones

[jaikiran]

Can I please get a review of this change in the JDK build tool class CreateSymbols which addresses an issue related to the reproducibility of the generated ct.sym file? This addresses https://bugs.openjdk.org/browse/JDK-8327466.

Even before this change, in order to support reproducibility of the generated ct.sym file, this internal CreateSymbols program takes a timestamp argument. The value for the timestamp is considered to be the number of seconds since epoch and maps directly to the SOURCE_DATE_EPOCH construct that's used by several tools (even outside the JDK) to provide reproducible output.

The ct.sym file generated by the CreateSymbols program is a ZIP file. CreateSymbols uses the passed timestamp value to set the last modified time on each of the ZIP entries of the generated ZIP file. That way, a constant value for the timestamp argument implies that without anything else having changed for a subsequent build, the subsequently generated ct.sym will also have the exact same value for the timestamp for each of the ZIP entries.

Like many other things in the ZIP specification, a timestamp for a ZIP entry can be set in more than one place within the ZIP structure and the value thus set can be interpreted differently depending on where it is set. That also results in Java SE's java.util.zip APIs having more than one way of setting that timestamp.

The CreateSymbols program uses the java.util.zip.ZipEntry.setTime(...) API to set this timestamp on the entry. However, that API is specified to be affected by the local system default timezone. This effectively means that if CreateSymbols is triggered more than once with the same timestamp value but with a different timezone, then the generated ct.sym files from each run will have a different value for the ZIP entry timestamps. This defeats the purpose of the timestamp agrument.

The fix is to use an alternate API java.util.zip.ZipEntry.setTimeLocal(...) which isn't affected by the local system timezone. This API was introduced in Java 9 to solve issues like this. The decade old original RFR, when this API was introduced, does a very good job of explaining the necessity of this API and how it differs from the setTime(...) method https://mail.openjdk.org/pipermail/core-libs-dev/2015-June/034426.html.

The commit in this PR also introduces a regression test which reproduces the issue and verifies the fix.

I have run this change in tier1 and tier5 and this and other tests continue to pass.

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8327466: ct.sym zip not reproducible across build environment timezones (Bug - P4)

Reviewers

• Erik Joelsson (@erikj79 - Reviewer)
• Jan Lahoda (@lahodaj - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/25207/head:pull/25207
$ git checkout pull/25207

Update a local copy of the PR:
$ git checkout pull/25207
$ git pull https://git.openjdk.org/jdk.git pull/25207/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 25207

View PR using the GUI difftool:
$ git pr show -t 25207

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/25207.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back jpai! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@jaikiran This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8327466: ct.sym zip not reproducible across build environment timezones

Reviewed-by: erikj, jlahoda

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 4 new commits pushed to the master branch:

• 48d2acb: 8356783: CompilerTask hot_method is redundant
• de10644: 8356631: OopHandle replacement methods should not be called on empty handles
• 14d372b: 8356816: JFR: Move printing of metadata into separate class
• ... and 1 more: https://git.openjdk.org/jdk/compare/7dcc386af9831bea21def6763512c9fba13b5064...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[openjdk]

@jaikiran The following labels will be automatically applied to this pull request:

• build
• compiler

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 00: Full (9126117d)

[lahodaj]

Looks good to me, thanks!

[jaikiran]

Thank you Erik and Jan for the reviews.

[jaikiran]

/integrate

[openjdk]

Going to push as commit a989245.
Since your change was applied there have been 34 commits pushed to the master branch:

• 3270b00: 8356173: Remove ThreadCritical
• 5c73dfc: 8352533: Report useful IOExceptions when jspawnhelper fails
• 86f39ab: 8249825: Tests sun/security/ssl/SSLSocketImpl/SetClientMode.java and NonAutoClose.java marked with @ignore
• ... and 31 more: https://git.openjdk.org/jdk/compare/7dcc386af9831bea21def6763512c9fba13b5064...master

Your commit was automatically rebased without conflicts.

[openjdk]

@jaikiran Pushed as commit a989245.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.