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

JDK-8276837: [macos]: Error when signing the additional launcher #6636

Closed
wants to merge 86 commits into from

Conversation

andyherrick
Copy link

@andyherrick andyherrick commented Dec 1, 2021

Before signing, unsign all executables and libraries (not just those in runtime).
Also, run individual file sign and unsign commands in quiet mode.
Also, add test case to SigningAppImageTest to test that signing app that has additional launcher works, and results in validly signed launcher and additional launcher (original reported problem).


Progress

  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • Change must be properly reviewed

Issue

  • JDK-8276837: [macos]: Error when signing the additional launcher

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 6636

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

Using diff file

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

andyherrick and others added 30 commits Dec 1, 2021
…(java_thread == _state->get_thread()) failed: Must be"

Reviewed-by: mdoerr, lmesnik, dcubed
Co-authored-by: Guy Steele <gls@openjdk.org>
Reviewed-by: rriggs
…r when none

Co-authored-by: Guy Steele <gls@openjdk.org>
Reviewed-by: rriggs
Reviewed-by: sviswanathan, dlong
…n.java fails when running with JEP 416

Reviewed-by: sspitsyn, dholmes
Reviewed-by: almatvee, herrick, erikj
… on macosx-X64

8277577: ProblemList compiler/onSpinWait/TestOnSpinWaitAArch64DefaultFlags.java on linux-aarch64
8277578: ProblemList applications/jcstress/acqrel.java on linux-aarch64

Reviewed-by: mikael
…ls with "wrong OOME"

Reviewed-by: shade, stefank
…ts to help diagonize recent intermittent failures

Reviewed-by: almatvee
Reviewed-by: sjohanss, tschatzl
Reviewed-by: lmesnik, tschatzl, sspitsyn
… failed: not last

Reviewed-by: chagedorn, neliasso
…launching jpackage tests to help diagonize recent intermittent failures

Reviewed-by: alanb, stefank
simonis and others added 18 commits Dec 1, 2021
…he interpreter

Reviewed-by: chagedorn, mdoerr
…nt/jfr/RecordingInfo.java

Reviewed-by: mgronlun
…n(...) on Japanese Windows

Reviewed-by: naoto
Co-authored-by: Roberto Castañeda Lozano <rcastanedalo@openjdk.org>
Co-authored-by: Nils Eliasson <neliasso@openjdk.org>
Reviewed-by: chagedorn, thartmann
Co-authored-by: Nick Gasson <ngasson@openjdk.org>
Reviewed-by: aph, neliasso
Reviewed-by: thartmann, chagedorn
@bridgekeeper
Copy link

bridgekeeper bot commented Dec 1, 2021

👋 Welcome back herrick! 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
Copy link

openjdk bot commented Dec 1, 2021

@andyherrick The following label will be automatically applied to this pull request:

  • core-libs

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

@openjdk openjdk bot added the core-libs core-libs-dev@openjdk.org label Dec 1, 2021
@andyherrick andyherrick changed the title JDK-8276837: [macos]: Errorwhen signing the additional launcher JDK-8276837: [macos]: Error when signing the additional launcher Dec 1, 2021
@andyherrick andyherrick marked this pull request as ready for review Dec 1, 2021
@openjdk openjdk bot added the rfr Pull request is ready for review label Dec 1, 2021
@mlbridge
Copy link

mlbridge bot commented Dec 1, 2021

Webrevs


@Test
public static void testAdditionalLauncher() throws Exception {
SigningCheck.checkCertificates();

JPackageCommand cmd = JPackageCommand.helloAppImage();
cmd.addArguments("--mac-sign", "--mac-signing-key-user-name",
SigningBase.DEV_NAME, "--mac-signing-keychain",
SigningBase.KEYCHAIN);
AdditionalLauncher testAL = new AdditionalLauncher("testAL");
testAL.applyTo(cmd);

cmd.executeAndAssertHelloAppImageCreated();
Path launcherPath = cmd.appLauncherPath();
Path testALPath = launcherPath.getParent().resolve("testAL");
SigningBase.verifyCodesign(launcherPath, true);
SigningBase.verifyCodesign(testALPath, true);
}

Choose a reason for hiding this comment

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

Would it make sense to extend the existing test with additional launcher checks instead of creating another one? Packaging of a full JDK with signing probably takes a while

Copy link
Author

@andyherrick andyherrick Dec 1, 2021

Choose a reason for hiding this comment

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

yes - that would be simpler - will do.

@andyherrick
Copy link
Author

andyherrick commented Dec 1, 2021

last push was meant to be only one file - but got lots of unrelated changes.
will close this one and open new PR.

@andyherrick andyherrick closed this Dec 1, 2021
@andyherrick andyherrick deleted the JDK-8276837 branch Dec 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core-libs core-libs-dev@openjdk.org rfr Pull request is ready for review