8342576: [macos] AppContentTest still fails after JDK-8341443 for same reason on older macOS versions#21698
8342576: [macos] AppContentTest still fails after JDK-8341443 for same reason on older macOS versions#21698sashamatveev wants to merge 1 commit intoopenjdk:masterfrom
Conversation
…e reason on older macOS versions
|
👋 Welcome back almatvee! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
|
@sashamatveev The following label will be automatically applied to this pull request:
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. |
|
The test does nothing on OSX, right? We just run jpackage and don't verify the output. What is the point then? How come this test does signing on OSX? I don't see how |
|
Test used to do nothing before this fix on macOS. Before we forced exit code to 1, jpackage failed and we did not verify anything. With new approach we will generate application image with additional content and test will verify that additional content exists in application image. Generating PKG and DMG is not possible due to potential failure of codesign. Also, it is unlikely that something can go wrong with PKG and DMG generation with additional content, since PKG and DMG does not really care about content of application image. We always do code sign on macOS since sometime already. If |
|
Another possible solution to codesign failure is to add additional CLI command to jpackage |
|
Mailing list message from Michael Hall on core-libs-dev:
So these both still return the failing error code? Are these changes only related to the test or do they apply somehow to normal usage? I?m not sure what the additional files are. Extra files that go into the app directory? Or where do they go? |
|
|
Mailing list message from Michael Hall on core-libs-dev:
Thanks
OK. I?m not sure how applications are accessing files here. Being able to have everything that?s in the input directory end up in the app directory I think would be able to serve the same purpose without needing an additional cli arg that requires special handling. Java apps can locate files there with a java option of something like, -Dapp.path=$APPDIR. Or figure it out from something in java.class.path. They could directly resource load from here if the app directory itself was in class path? I don?t remember if I enhancement requested that or not. They would also, files in the app directory, all be automatically code signed I think wouldn?t they? |
|
Closing this PR. This issue will be fixed under new PR #21741. |
|
Mailing list message from Alexander Matveev on core-libs-dev: Hi Michael,
Yes, files under app directory will be signed as well. Thanks, From: Michael Hall <mik3hall at gmail.com>
Thanks
OK. I?m not sure how applications are accessing files here. Being able to have everything that?s in the input directory end up in the app directory I think would be able to serve the same purpose without needing an additional cli arg that requires special handling. Java apps can locate files there with a java option of something like, -Dapp.path=$APPDIR. Or figure it out from something in java.class.path. They could directly resource load from here if the app directory itself was in class path? I don?t remember if I enhancement requested that or not. They would also, files in the app directory, all be automatically code signed I think wouldn?t they? -------------- next part -------------- |
|
Mailing list message from Michael Hall on core-libs-dev:
Sorry, I was going to leave it at this but did a little more googling out of curiosity. https://bugs.openjdk.org/browse/JDK-8274717 https://bugs.openjdk.org/browse/JDK-8274346 I am sort of gathering that this might make more sense on other platforms? Putting files in an application Contents directory doesn?t seem like a sound practice to me. Are you aware of any actual use cases of this? Maybe someone has a situation where this makes sense on MacOS but I am still not thinking of any. The app directory provides a way to do usual java resource loading. There is no such normal provision for accessing something off of an Application's Contents directory that I am aware of. --mac-dmg-content makes perfect sense for dmg?s. I am not really familiar with package installs maybe it works better there? For an application ?app-image if you wanted to allow additional files I think they would be better off extermal to the application. Maybe someplace like the ~/Library/Application\ Support directory. Again maybe I?m missing something and this is commonly used functionality. If so, feel free to ignore this. -------------- next part -------------- |
|
Google directed me here. We updated from JDK23 to JDK24 We have JDK23: Without any issue JDK24: Full example: at JabRef/jabref#13032 |
|
We had a very similar issue in one of jpackage tests when we tried to copy random files in the "Contents" directory. See a comment at AppContentTest.java#L62. It turned out that you can have only "Info.plist" file in the "Contents" directory. Any other files should be placed in the "Contents/Resources" directory instead. This aligns with Apple's recommendations for bundle directory structure - https://developer.apple.com/library/archive/documentation/CoreFoundation/Conceptual/CFBundles/BundleTypes/BundleTypes.html#//apple_ref/doc/uid/10000123i-CH101-SW19 jpackage is not quite compliant with these recommendations, as it places the Java runtime in the "Contents/runtime" directory and the application jar files in the "Contents/app" directory. The justification is that this is legacy behavior originating from the JFX's jpackager. Fortunately, this hasn't caused problems so far. Bottom line: don't copy files or directories with periods (.) in their names to the "Contents" directory. Copy them to the "Contents/Resources" directory instead. Otherwise, codesign will fail. |
|
This is unfortunate. However, then how do I add content to the app image before? |
You can add custom app content to the app image, but be sure to copy it to the correct location. |
|
The workaround at workflows/deployment.yml doesn't look right, I'm surprised "codesign" works in the script. Maybe it runs on older macos with a permissive codesign version? If you want to build app image and create pkg and dmg packages from it in separate steps, you can leverage jpackage's
No explicit codesign, hdiutil, and productbuild invocations. |
|
Followed the stepa and built an unsigned image first and then signed the jpackage build and created the pkg/dmg files. However, notarization fails as it says it's not signed at all: e.g.:
Testing now with a presigned image. |
|
When supplying mac sign to building the image, it does not work either and then applying. And now I remember that probably this was the initial reason for using the original approach mentioned earlier with specifcying the app content |
This means the two-stage packaging with signing is completely broken in jpackage. The output can not be notarized. I suspect the same applies to one-stage packaging also. Not good. Can you share jpackage logs? |
|
From the GitHub actions run with double signing: (mac os 14 runner) signed_image_signed_dmg_macOS (ARM64) installer and portable version.txt Run with only signing the pkg/dmg: |
|
It looks like jpackage sign bundles in a way that they can not be notarized. |
|
@sashamatveev can you comment? |
|
Not sure what is going on. I tried generating app-image with signing modifying it and then generating DMG with --mac-sign and application image is signed correctly. I did not tried to notarize it. When DMG is mounted what is output of Also from provided logs:
|
This is just a mask to hide the real value.
Well, the problem is with notarization. Signing in jpackage passed. But looks like it produces a bundle that can not be notarized. |
|
Mailing list message from Michael Hall on core-libs-dev: I had successfully notarized a jpackage app sometime back. As I remember it was a rather involved process where you had to be online to an Apple server? This might be from when I did that |
|
It might be possible that "codesign --display -vvvv" does not check signature well enough as notarization. If I am reading issue correctly signed app-image is being generated, then it will be post-process and then it is used to generate DMG. See JabRef/jabref#13032. In this case we will package app-image as is since we will assume it is signed. See JDK-8293462. My suggestion is to generate unsigned image if post-processing is required or sign application image separately after it was modified. See JDK-8286850. |
|
I managed to get it working now.
|
|
Mailing list message from Michael Hall on core-libs-dev:
Possibly I missed something if this was the fix. I don?t remember any issues with the existing signing when I tried notarization. I do see? --deep (DEPRECATED for signing as of macOS 13.0) When I man codesign. I?m also a little curious how you came up with the ?options runtime. I?m not familiar and even after looking at the man I?m not quite sure what it's purpose is. Make the requirements extra strict exempted by entitlements maybe? Also why you decided to use ?timestamp? I looked to see if jpackage uses any options like this but verbose output only shows codesign being invoked, many times, and not with what parameters. If this is sufficient, or required, for a valid, notarizable application, maybe jpackage could be simplified to just do an invocation like yours at the end rather than the numerous invocations it appears to do. |
|
Jpackage does include the options as well from the logs: After creating the app image we put additional content in it under Resources. That probably affects the integrity? of the whole stuff Runtime options is for Hardened Runtime https://developer.apple.com/documentation/security/hardened-runtime Timestamp is also required https://developer.apple.com/documentation/security/resolving-common-notarization-issues#Include-a-secure-timestamp Otherwise, notarization fails with no timestamp or invalid timestamp. Deep is like going recursively through the files. But should be avoided. I will try without as well To upload a macOS app to be notarized, you must enable the Hardened Runtime capability. For more information about notarization, see Notarizing macOS software before distribution. |
|
Mailing list message from Michael Hall on core-libs-dev:
Not for me with ${PACKAGER} \ All I see are the [10:05:03.044] Running /usr/bin/codesign
Hardened runtime sounds sort of familiar, it?s been a while. Other requirements might of changed. |
|
Mailing list message from Michael Hall on core-libs-dev:
Sorry missed it. /usr/bin/codesign -s Developer ID Application: Michael Hall (5X6BXQB3Q7) -vvvv --timestamp --options runtime --prefix us.hall.hp.common. --entitlements entitlements.xml --force /var/folders/mp/64527rf1501726r7t53qpx0w0000gn/T/jdk.jpackage8582845157994500039/images/image-17727033358038913514/HalfPipe.app -------------- next part -------------- |
|
Mailing list message from Michael Hall on core-libs-dev:
There was something in the man about behavior when using deep in combination with force. I?m not sure what eliminating one will do. |
Right, if you modify the app image, you need to resign it. I'm glad you figured out a workaround. But ideally, you should be able to pass additional content to jpackage with |
|
So the takeaway from this fruitful discussion is:
|
Actually, this is a regression as it worked in JDK23 where we just used it recently Just changing from 23 to 24 led to this error as reported by @koppor earlier in this thread The binaries of the release were successfully notarized both on macOS13 and macOS14 (About dialog of JabRef shows the jdk version used as well, 23.0.1) |
|
Mailing list message from Michael Hall on core-libs-dev:
I was mistaken on this. The codesign parameters appear to be shown once. Then there are many messages indicating just that codesign is being run. I may of missed this as well. I thought the original poster indicated that changing the signing parameters somehow gave them a fix. I would think that only executable content would need signing? ?app-content I don?t think adds anything for that. But I don?t know. Other than testing some time back I don?t notarize. |
This looks like a regression, but it is not. You use |
The same parameters don't need to be logged, but having the complete list of files signed by jpackage in the log would be good. We don't have it now.
This is a negative test; it configures jpackage to exit with an error. Not very helpful in the context of this discussion |
|
Just tried it with putting the additional files into the Resources folder. Same error as originally reported. Run: https://github.com/JabRef/jabref/actions/runs/14823769155/job/41614205909 Changes from PR: https://github.com/JabRef/jabref/pull/13032/files#diff-5a17873aec4eae6b52b00959d8f9e17672912858f63181d39de8c3a713e90018R135-R144 |
|
Mailing list message from Michael Hall on core-libs-dev:
If this is the current error log the file ***Host.py (? Strange file name.) doesn?t seem to be in the resource directory but the Contents directory still? But I?ve been off on this a couple times already. Still curious do all the codesign running messages in verbose output indicate multiple invocations or are they the status for a single running process? -------------- next part -------------- |
|
In the log: The command line:
You can have multiple |
|
Thanks all for your support, with a single app-content parameter and the resource directory this worked with codesign and even notarization worked on both macOS13 and macOS14 (arm) |
|
Filed JDK-8356218 to follow up with the documentation |
Progress
Issue
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/21698/head:pull/21698$ git checkout pull/21698Update a local copy of the PR:
$ git checkout pull/21698$ git pull https://git.openjdk.org/jdk.git pull/21698/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 21698View PR using the GUI difftool:
$ git pr show -t 21698Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/21698.diff
Webrev
Link to Webrev Comment