Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8276766: Enable jar and jmod to produce deterministic timestamped content #82

Conversation

andrew-m-leonard
Copy link

@andrew-m-leonard andrew-m-leonard commented Jan 12, 2022

This backport (along with others in-progress) enables reproducible building of jar's and jmod's with the addition of a new --date option. Enabling reproducible timestamped jar & jmod content.

jdk-17.0.3 CSR: https://bugs.openjdk.java.net/browse/JDK-8279926


Progress

  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • Change must be properly reviewed
  • Change requires a CSR request to be approved

Issues

  • JDK-8276766: Enable jar and jmod to produce deterministic timestamped content
  • JDK-8279453: Disable tools/jar/ReproducibleJar.java on 32-bit platforms
  • JDK-8277755: Enable jar and jmod to produce deterministic timestamped content (CSR)

Reviewers

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk17u-dev pull/82/head:pull/82
$ git checkout pull/82

Update a local copy of the PR:
$ git checkout pull/82
$ git pull https://git.openjdk.java.net/jdk17u-dev pull/82/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 82

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk17u-dev/pull/82.diff

@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented Jan 12, 2022

👋 Welcome back aleonard! 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 openjdk bot changed the title Backport db68a0ce1ce152345320e70acb7e9842d2f1ece4 8276766: Enable jar and jmod to produce deterministic timestamped content Jan 12, 2022
@openjdk
Copy link

@openjdk openjdk bot commented Jan 12, 2022

This backport pull request has now been updated with issue from the original commit.

@openjdk openjdk bot added backport rfr labels Jan 12, 2022
@mlbridge
Copy link

@mlbridge mlbridge bot commented Jan 12, 2022

Webrevs

Signed-off-by: Andrew Leonard <anleonar@redhat.com>
Signed-off-by: Andrew Leonard <anleonar@redhat.com>
@andrew-m-leonard
Copy link
Author

@andrew-m-leonard andrew-m-leonard commented Jan 12, 2022

Also adds disablement of test/jdk/tools/jar/ReproducibleJar.java on 32bit platforms for https://bugs.openjdk.java.net/browse/JDK-8279453

@andrew-m-leonard
Copy link
Author

@andrew-m-leonard andrew-m-leonard commented Jan 12, 2022

/issues JDK-8279453

@openjdk
Copy link

@openjdk openjdk bot commented Jan 12, 2022

@andrew-m-leonard Unknown command issues - for a list of valid commands use /help.

@openjdk
Copy link

@openjdk openjdk bot commented Jan 12, 2022

@andrew-m-leonard Available commands:

  • cc - add or remove an additional classification label
  • clean - Mark the backport pull request as a clean backport
  • contributor - adds or removes additional contributors for a PR
  • covered - used when employer has signed the OCA
  • csr - require a compatibility and specification request (CSR) for this pull request
  • help - shows this text
  • integrate - performs integration of the changes in the PR
  • issue - edit the list of issues that this PR solves
  • label - add or remove an additional classification label
  • open - Set the pull request state to "open"
  • reviewer - manage additional reviewers for a PR
  • reviewers - set the number of additional required reviewers for this PR
  • signed - used after signing the OCA
  • solves - edit the list of issues that this PR solves
  • sponsor - performs integration of a PR that is authored by a non-committer
  • summary - updates the summary in the commit message
  • test - used to run tests

@andrew-m-leonard
Copy link
Author

@andrew-m-leonard andrew-m-leonard commented Jan 12, 2022

/issue add JDK-8279453

@openjdk
Copy link

@openjdk openjdk bot commented Jan 12, 2022

@andrew-m-leonard
Adding additional issue to issue list: 8279453: Disable tools/jar/ReproducibleJar.java on 32-bit platforms.

@andrew-m-leonard andrew-m-leonard changed the title 8276766: Enable jar and jmod to produce deterministic timestamped content 8279925: Enable jar and jmod to produce deterministic timestamped content Jan 12, 2022
@openjdk openjdk bot added the csr label Jan 12, 2022
@andrew-m-leonard andrew-m-leonard changed the title 8279925: Enable jar and jmod to produce deterministic timestamped content 8276766: Enable jar and jmod to produce deterministic timestamped content Jan 12, 2022
@openjdk openjdk bot removed the csr label Jan 12, 2022
@GoeLin
Copy link
Member

@GoeLin GoeLin commented Jan 13, 2022

Hi @andrew-m-leonard
next time, I would prefer two backports for seperate issues as JDK-8276766 and JDK-8279453.
You can use dependent Pull Request to stack changes if needed.
Add in the fix request comments that such changes need to be pushed at the same time.
I assume this would have saved you from looking for a reviewer.
Please also elaborate why the change is not clean when you make the Pull Request. This simplifies the task of reviewing.
Please only add the jfk17u-fix-request label once the change is reviewed.
Thanks,
Goetz.

@andrew-m-leonard
Copy link
Author

@andrew-m-leonard andrew-m-leonard commented Jan 13, 2022

@andrew-m-leonard
Copy link
Author

@andrew-m-leonard andrew-m-leonard commented Jan 13, 2022

This PR had a merge conflict when applying the original patch, due to jdk PR: openjdk/jdk@a81e4fc
which is not required for this PR.
The setZipEntryTime() logic was applied to the jdk17u logic.

@andrew-m-leonard
Copy link
Author

@andrew-m-leonard andrew-m-leonard commented Jan 13, 2022

jdk-17u CSR for this has now been Approved: https://bugs.openjdk.java.net/browse/JDK-8279926

@andrew-m-leonard
Copy link
Author

@andrew-m-leonard andrew-m-leonard commented Jan 13, 2022

tier1 and tooling tests all pass.

Copy link

@LanceAndersen LanceAndersen left a comment

Looks good to me

@openjdk
Copy link

@openjdk openjdk bot commented Jan 13, 2022

@andrew-m-leonard 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:

8276766: Enable jar and jmod to produce deterministic timestamped content
8279453: Disable tools/jar/ReproducibleJar.java on 32-bit platforms

Reviewed-by: lancea, clanger

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 8 new commits pushed to the master branch:

  • d3eb0a2: 8279998: PPC64 debug builds fail with "untested: RangeCheckStub: predicate_failed_trap_id"
  • f749fc7: 8280002: jmap -histo may leak stream
  • 2137e83: 8277069: [REDO] JDK-8276743 Make openjdk build Zip Archive generation "reproducible"
  • 69d296d: 8279833: Loop optimization issue in String.encodeUTF8_UTF16
  • 6f644e0: 8273277: C2: Move conditional negation into rc_predicate
  • d1d0b08: 8279412: [JVMCI] failed speculations list must outlive any nmethod that refers to it
  • 3354244: 8271202: C1: assert(false) failed: live_in set of first block must be empty
  • e965881: 8263567: gtests don't terminate the VM safely

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch.
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.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@LanceAndersen, @RealCLanger) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

@openjdk openjdk bot added the ready label Jan 13, 2022
@andrew-m-leonard
Copy link
Author

@andrew-m-leonard andrew-m-leonard commented Jan 13, 2022

thank you @LanceAndersen

@andrew-m-leonard
Copy link
Author

@andrew-m-leonard andrew-m-leonard commented Jan 13, 2022

/integrate
i'm happy this is ready

@openjdk openjdk bot added the sponsor label Jan 13, 2022
@openjdk
Copy link

@openjdk openjdk bot commented Jan 13, 2022

@andrew-m-leonard
Your change (at version 5f5ac9a) is now ready to be sponsored by a Committer.

@GoeLin
Copy link
Member

@GoeLin GoeLin commented Jan 14, 2022

Hi @andrew-m-leonard, backports are only ready if there is the jdk*u-fix-yes label on the bug. Please don't integrate before you see that label on the bug. Unfortunatley there is no automatic check for this.

@andrew-m-leonard
Copy link
Author

@andrew-m-leonard andrew-m-leonard commented Jan 14, 2022

Thanks @GoeLin, I did realize that, I got confused with the other bug that had been marked

@openjdk openjdk bot removed the sponsor label Jan 14, 2022
@andrew-m-leonard
Copy link
Author

@andrew-m-leonard andrew-m-leonard commented Jan 17, 2022

/integrate

@openjdk openjdk bot added the sponsor label Jan 17, 2022
@openjdk
Copy link

@openjdk openjdk bot commented Jan 17, 2022

@andrew-m-leonard
Your change (at version 854742e) is now ready to be sponsored by a Committer.

@RealCLanger
Copy link
Contributor

@RealCLanger RealCLanger commented Jan 17, 2022

/reviewers 2

@openjdk
Copy link

@openjdk openjdk bot commented Jan 17, 2022

@RealCLanger
The number of required reviews for this PR is now set to 2 (with at least 1 of role reviewers).

@openjdk openjdk bot removed sponsor ready labels Jan 17, 2022
@RealCLanger
Copy link
Contributor

@RealCLanger RealCLanger commented Jan 17, 2022

@andrew-m-leonard, can you please remove the issue JDK-8277755 from this PR via the "/issue remove" PR command?
JDK-8277755 is the original CSR and won't be backported by the resulting commit from this PR.

As for JDK-8279453, I can see that it doesn't have the jdk17u-fix-request label/comment yet. Can you please add this for the sake of completeness?

Thanks!

@andrew-m-leonard
Copy link
Author

@andrew-m-leonard andrew-m-leonard commented Jan 18, 2022

/issue remove JDK-8277755

@openjdk
Copy link

@openjdk openjdk bot commented Jan 18, 2022

@andrew-m-leonard The issue 8277755 was not found in the list of additional solved issues. The list currently contains these issues: 8279453

@andrew-m-leonard
Copy link
Author

@andrew-m-leonard andrew-m-leonard commented Jan 18, 2022

@RealCLanger I can't remove 8277755, as the Skara bot processor keeps adding it back in. It seems the Skara interface doesn't handle CSR's for backports, as it makes the assumption the corresponding CSR is the CSR for the bug determined from the PR summary prefix, which as we know must be the original bug, hence Skara chooses the original CSR and won't let you remove it....

@andrew-m-leonard
Copy link
Author

@andrew-m-leonard andrew-m-leonard commented Jan 18, 2022

I've added the jdk17u-fix-request to https://bugs.openjdk.java.net/browse/JDK-8279453 for completeness.

@openjdk openjdk bot added sponsor ready labels Jan 18, 2022
@RealCLanger
Copy link
Contributor

@RealCLanger RealCLanger commented Jan 18, 2022

@RealCLanger I can't remove 8277755, as the Skara bot processor keeps adding it back in. It seems the Skara interface doesn't handle CSR's for backports, as it makes the assumption the corresponding CSR is the CSR for the bug determined from the PR summary prefix, which as we know must be the original bug, hence Skara chooses the original CSR and won't let you remove it....

Ah, ok. Seems like a new feature of Skara to record the CSR issue. Didn't know that. So go for it.

/sponsor

@openjdk
Copy link

@openjdk openjdk bot commented Jan 18, 2022

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

  • d3eb0a2: 8279998: PPC64 debug builds fail with "untested: RangeCheckStub: predicate_failed_trap_id"
  • f749fc7: 8280002: jmap -histo may leak stream
  • 2137e83: 8277069: [REDO] JDK-8276743 Make openjdk build Zip Archive generation "reproducible"
  • 69d296d: 8279833: Loop optimization issue in String.encodeUTF8_UTF16
  • 6f644e0: 8273277: C2: Move conditional negation into rc_predicate
  • d1d0b08: 8279412: [JVMCI] failed speculations list must outlive any nmethod that refers to it
  • 3354244: 8271202: C1: assert(false) failed: live_in set of first block must be empty
  • e965881: 8263567: gtests don't terminate the VM safely

Your commit was automatically rebased without conflicts.

@openjdk openjdk bot added the integrated label Jan 18, 2022
@openjdk openjdk bot closed this Jan 18, 2022
@openjdk openjdk bot removed ready rfr sponsor labels Jan 18, 2022
@openjdk
Copy link

@openjdk openjdk bot commented Jan 18, 2022

@RealCLanger @andrew-m-leonard Pushed as commit 761e298.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport integrated
4 participants