Navigation Menu

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

8281175: Add a -providerPath option to jarsigner #7338

Closed
wants to merge 2 commits into from

Conversation

wangweij
Copy link
Contributor

@wangweij wangweij commented Feb 3, 2022

Add the -providerPath option to jarsigner to be consistent with keytool.


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-8281175: Add a -providerPath option to jarsigner
  • JDK-8281230: Add a -providerPath option to jarsigner (CSR)

Reviewers

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/7338/head:pull/7338
$ git checkout pull/7338

Update a local copy of the PR:
$ git checkout pull/7338
$ git pull https://git.openjdk.java.net/jdk pull/7338/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 7338

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/7338.diff

8281175: Add a -providerPath option to jarsigner
@wangweij
Copy link
Contributor Author

wangweij commented Feb 3, 2022

/csr

@bridgekeeper
Copy link

bridgekeeper bot commented Feb 3, 2022

👋 Welcome back weijun! 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 added rfr Pull request is ready for review csr Pull request needs approved CSR before integration labels Feb 3, 2022
@openjdk
Copy link

openjdk bot commented Feb 3, 2022

@wangweij this pull request will not be integrated until the CSR request JDK-8281230 for issue JDK-8281175 has been approved.

@openjdk
Copy link

openjdk bot commented Feb 3, 2022

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

  • compiler
  • core-libs
  • security

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.

@openjdk openjdk bot added security security-dev@openjdk.org core-libs core-libs-dev@openjdk.org compiler compiler-dev@openjdk.org labels Feb 3, 2022
@mlbridge
Copy link

mlbridge bot commented Feb 3, 2022

Webrevs

@wangweij
Copy link
Contributor Author

wangweij commented Feb 3, 2022

/label remove compiler
/label remove core-libs

@openjdk openjdk bot removed the compiler compiler-dev@openjdk.org label Feb 3, 2022
@openjdk
Copy link

openjdk bot commented Feb 3, 2022

@wangweij
The compiler label was successfully removed.

@openjdk openjdk bot removed the core-libs core-libs-dev@openjdk.org label Feb 3, 2022
@openjdk
Copy link

openjdk bot commented Feb 3, 2022

@wangweij
The core-libs label was successfully removed.

Copy link
Member

@XueleiFan XueleiFan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me except a minor comment. It's fine to me if you don't want to take it.

Comment on lines 254 to 256
String path = null;
path = PathList.appendPath(
path, System.getProperty("java.class.path"));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it more straightforward to use "String path = System.getProperty(...)", by combining line 254 and 255?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems so. I'll fix it.

@haimaychao
Copy link
Contributor

Code change looks good to me.
I have a comment on the CSR. The “Options for jarsigner” section in jarsigner manpage, where it describes -digestalg algorithm and -sigalg algorithm, we would update it to include this new option -providerPath there.

@wangweij
Copy link
Contributor Author

wangweij commented Feb 3, 2022

Code change looks good to me.
I have a comment on the CSR. The “Options for jarsigner” section in jarsigner manpage, where it describes -digestalg algorithm and -sigalg algorithm, we would update it to include this new option -providerPath there.

Correct. I've added some lines in the man page.

@haimaychao
Copy link
Contributor

Updated CSR looks good.

@openjdk openjdk bot removed the csr Pull request needs approved CSR before integration label Feb 4, 2022
@openjdk
Copy link

openjdk bot commented Feb 4, 2022

@wangweij 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:

8281175: Add a -providerPath option to jarsigner

Reviewed-by: xuelei, hchao

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

  • 8e4ef81: 8280767: -XX:ArchiveClassesAtExit does not archive BoundMethodHandle$Species classes
  • f5d6fdd: 8280476: [macOS] : hotspot arm64 bug exposed by latest clang
  • d4b99bc: 8281120: G1: Rename G1BlockOffsetTablePart::alloc_block to update_for_block
  • 66b2c3b: 8280948: [TESTBUG] Write a regression test for JDK-4659800
  • 7207f2a: Merge
  • 01f93dd: 8279385: [test] Adjust sun/security/pkcs12/KeytoolOpensslInteropTest.java after 8278344
  • 3d926dd: 8277795: ldap connection timeout not honoured under contention
  • 51b53a8: 8280913: Create a regression test for JRootPane.setDefaultButton() method
  • 46c6c6f: 8281043: Intrinsify recursive ObjectMonitor locking for PPC64
  • c936e70: 8280593: [PPC64, S390] redundant allocation of MacroAssembler in StubGenerator ctor
  • ... and 111 more: https://git.openjdk.java.net/jdk/compare/295c0474c43484e793b67a70af316aaae49fe361...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 openjdk bot added the ready Pull request is ready to be integrated label Feb 4, 2022
@wangweij
Copy link
Contributor Author

wangweij commented Feb 7, 2022

/integrate

@openjdk
Copy link

openjdk bot commented Feb 7, 2022

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

  • a0f6f24: 8280890: Cannot use '-Djava.system.class.loader' with class loader in signed JAR
  • 22a1a32: 8268387: Rename maximum compaction to maximal compaction in G1
  • 7667771: 8281114: G1: Remove PreservedMarks::init_forwarded_mark
  • 4c16949: 8272996: JNDI DNS provider fails to resolve SRV entries when IPV6 stack is enabled
  • f3e8242: 8280965: Tests com/sun/net/httpserver/simpleserver fail with FileSystemException on Windows 11
  • 95fd9d2: 8281243: Test java/lang/instrument/RetransformWithMethodParametersTest.java is failing
  • f5e0870: 8281117: Add regression test for JDK-8280587
  • f230282: 8281298: Revise the creation of unmodifiable list
  • 5dfff74: 8166050: partialArray is not created in javax.swing.text.html.parser.NPrintWriter.println(...) method
  • 2f48a3f: 8279878: java/awt/font/JNICheck/JNICheck.sh test fails on Ubuntu 21.10
  • ... and 125 more: https://git.openjdk.java.net/jdk/compare/295c0474c43484e793b67a70af316aaae49fe361...master

Your commit was automatically rebased without conflicts.

@openjdk openjdk bot added the integrated Pull request has been integrated label Feb 7, 2022
@openjdk openjdk bot closed this Feb 7, 2022
@openjdk openjdk bot removed the ready Pull request is ready to be integrated label Feb 7, 2022
@openjdk openjdk bot removed the rfr Pull request is ready for review label Feb 7, 2022
@openjdk
Copy link

openjdk bot commented Feb 7, 2022

@wangweij Pushed as commit 2ed1f4c.

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

@wangweij wangweij deleted the 8281175 branch February 7, 2022 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integrated Pull request has been integrated security security-dev@openjdk.org
3 participants