-
Notifications
You must be signed in to change notification settings - Fork 6.2k
8286122: [macos]: App bundle cannot upload to Mac App Store due to info.plist embedded in java exe #8666
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
Conversation
…fo.plist embedded in java exe
|
👋 Welcome back almatvee! A progress list of the required criteria for merging this PR into |
|
@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. |
Webrevs
|
| @Test | ||
| public static void test() throws Exception { | ||
| JPackageCommand cmd = JPackageCommand.helloAppImage(); | ||
| cmd.addArguments("--mac-app-store", "--runtime-image", getRuntimeImage(true)); | ||
|
|
||
| cmd.executeAndAssertHelloAppImageCreated(); | ||
| } | ||
|
|
||
| @Test | ||
| public static void test2() throws Exception { | ||
| JPackageCommand cmd = JPackageCommand.helloAppImage(); | ||
| cmd.addArguments("--mac-app-store", "--runtime-image", getRuntimeImage(false)); | ||
|
|
||
| cmd.execute(1); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Test
@Parameter("true")
@Parameter("false")
public static void test(boolean stripNativeCommands) throws Exception {
JPackageCommand cmd = JPackageCommand.helloAppImage();
cmd.addArguments("--mac-app-store", "--runtime-image", getRuntimeImage(stripNativeCommands));
if (stripNativeCommands) {
cmd.executeAndAssertHelloAppImageCreated();
} else {
cmd.execute(1);
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
| public class MacAppStoreJLinkOptionsTest { | ||
|
|
||
| @Test | ||
| public static void test() throws Exception { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd give some more descriptive names to test functions than test and test2. Something like testWithStripNativeCommands and testWithoutStripNativeCommands maybe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
|
Mailing list message from Michael Hall on core-libs-dev: Is this restricted somehow to Mac App Store applications? Is a warning issued as stripping native commands may break application functionality? Is it not possible for the user to provide their own Info.plist with a different bundle identifier that doesn?t collide? I?m not real familiar with the build process. But would it be possible for the user to build their own jdk that substitutes something else for the colliding identifier that gets embedded? |
|
Mailing list message from Michael Hall on core-libs-dev:
Or just change it in the current build so it doesn?t collide? With the application or whatever one it is colliding with. |
…fo.plist embedded in java exe [v2]
|
Mailing list message from Alexander Matveev on core-libs-dev: Hi Michael,
Yes, helper tools (in our case JDK native commands) in Mac App Store applications cannot use same bundle ID as another application. Since we have bundle ID embedded in native commands multiple applications cannot use it without updating each native command with unique bundle ID and since they embedded inside executable I do not see how it would be possible to change during packaging.
No, error will be issues and jpackage will exit with error if ?mac-app-store arguments is used and provided runtime has native tools or user did not specified --strip-native-commands to jlink.
Not possible due to native commands have embedded Info.plist.
Yes, it should be possible and in theory such JDK with native commands can be used. However, current fix will not allow such JDK build, since we checking for presence of ?bin? folder and not if ID is actually unique. To work around this limitation user can package without native commands first, then add native commands and re-sign application.
Not possible, since ID should be unique per application. Thanks, |
|
Mailing list message from Michael Hall on core-libs-dev: Alexander
Possibly not during packaging but during the jdk build of the native commands? Somehow the Info.plist must be getting embedded and a bundle id provided. I?m not sure where or how. But one way to get uniqueness if the id is provided to embed the Info.plist in the native command executable compile/link would be to include the command as part of the identifier. Something like CFBundleID = java.native.cmd or CFBundleID = javac.native.cmd.
A warning might be sufficient if the commands are included accidentally by the user unintentionally omitting the ?strip-native-commands with the jlink options. Then just force the ?strip-native-commands option. It might be the application will just lose some non-critical functionality even if they meant to have the native command included.
Maybe I?m misunderstanding the conflict. Are the commands conflicting with the application level Info.plist or with each other? I believe jpackage usually has a mechanism where a substitute Info.plist can be used if the conflict is with the application level. If multiple commands are conflicting with each other this won?t work.
Signing is currently an integrated part of the process. I thought about making an enhancement request to have it possible to do it stepped. First build the application, then allow the user to post-process that. Maybe run a tool/script to modify the application level Info.plist file. Then allow a second final step that does the signing. I don?t remember if I actually made that enhancement request. But currently there is no way with jpackage to standalone sign the application is there? I think this is somewhat involved with the application bundle being iterated and for one thing each jar being separately code signed? I think it took a release or two of the command to get this correct. So I doubt Xcode could manage it. Is there a way to do the signing standalone with jpackage that I am not aware of? If I didn?t do an enhancement request on this I could, if you think jpackage could manage it?
Possibly you are misunderstanding me here I think you already indicated the could be done ?in theory?. It still seems possibly the correct ?fix?. Change the build so uniqueness is not an issue.
Thanks, |
|
Mailing list message from Magnus Ihse Bursie on core-libs-dev: (cc:ing build-dev.) On 2022-05-12 00:17, Michael Hall wrote:
This problem seem to come about as a clash between the the OpenJDK The well-written response from Apple tech support in the original web ----
---- So maybe a better, or at least alternative, way of dealing with this I will need to do some reading up on the macOS bundle format to fully /Magnus |
|
Mailing list message from Michael Hall on core-libs-dev:
I had read this but possibly didn?t grok the issue myself. If I understand correctly now the conflict isn?t within the application but across applications to some other application that has already been added to the Mac App Store that included the commands with the given CFBundleIdentifier. A solution like including a bundle identifier something like net.java.openjdk.MYAPP.java would be possible at packaging time but not at build time. |
|
/label build |
|
Mailing list message from Magnus Ihse Bursie on core-libs-dev: On 2022-05-12 13:17, Michael Hall wrote:
Yes, that is indeed how I also interpret this. Presumably, the very
As you say, we're a bit in a bind since the java executable needs to be I believe what you are describing is exactly solution #2 suggested by 1) When building the JDK, we create an additional copy of the java 2) At jpackage time, this java.template file is installed instead of My primary suggestion would to be to use an UUID for the unique ID. They This seems like a fully workable solution to me. However, I'd really I don't know what a "standard bundle" is, or how you would go around to I also don't understand if that means that the standard bundle needs to /Magnus |
|
@magicus |
|
Mailing list message from Michael Hall on core-libs-dev:
I was thinking jpackage would change the XXX to app name but a fixed size unique field would probably be better.
I may again not understanding but I was thinking they were talking about something like symlinks to a machine installed JDK and this seemed to me to defeat some of the purpose of embedding the jdk. But he could be thinking else. Something external to the application anyhow it seemed. |
|
Mailing list message from erik.joelsson at oracle.com on core-libs-dev: On 2022-05-12 04:58, Magnus Ihse Bursie wrote:
Aren't we embedding this bundle ID in every launcher, so you would need What I think we need to do is first evaluate if we actually need to /Erik |
|
Mailing list message from Scott Palmer on core-libs-dev:
I thought they meant something like the embedded JDK would be like a framework bundle. Since the framework is expected to be the same in multiple apps it would be excluded from the duplicate id check. (I think that is related to the older bug that the Apple guy thought might have come back.) Scott |
|
@sashamatveev 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: 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 153 new commits pushed to the
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 |
|
Mailing list message from Magnus Ihse Bursie on build-dev:
I naively assumed that only the java launcher was needed, since this is
What you say sounds good, although I feel I only partially understand I assume the important point here is that the app get the There seems to be a lot of guessing here. :-) I assume we either need to That solution make take some time to get correct, so the jpackage team /Magnus
|
|
Mailing list message from Michael Hall on build-dev:
If it?s correct that normal applications don?t need the executable included Info.plist and jpackage can somehow get versions that don?t include it that seems the easiest and most hack free solution.
Uh, this is something I again don?t understand. It seems questionable that most java applications would need this Info.plist key granting access to the microphone. It should be up to the developer to include this in their application Info.plist if their application requires it. That is where I thought my earlier suggestion of allowing application post-processing before standalone signing would make sense. The current jpackage solution of allowing the developer to include a separate complete alternate Info.plist in a separate directory I found somewhat awkward. To script the Info.plist changes. I believe this is used by native OS/X applications for this purpose including from XCode.
OK, one last possible misunderstanding on my part. The PR simply throws an error if commands are included. It doesn?t involve a workaround does it? |
|
Mailing list message from Michael Hall on build-dev:
If it?s correct that normal applications don?t need the executable included Info.plist and jpackage can somehow get versions that don?t include it that seems the easiest and most hack free solution.
Uh, this is something I again don?t understand. It seems questionable that most java applications would need this Info.plist key granting access to the microphone. https://developer.apple.com/documentation/bundleresources/information_property_list/nsmicrophoneusagedescription It should be up to the developer to include this in their application Info.plist if their application requires it. That is where I thought my earlier suggestion of allowing application post-processing before doing separate standalone signing would make sense. The current jpackage solution of allowing the developer to include a separate complete alternate Info.plist in a separate directory I found somewhat awkward. https://www.unix.com/man-page/osx/8/PLISTBUDDY/ To script the Info.plist changes. I believe this is used by native OS/X applications for this purpose including from XCode.
OK, one last possible misunderstanding on my part. The PR simply throws an error if commands are included. It doesn?t involve a workaround does it? |
|
We have the Yes, plistbuddy is an official Apple program. My understanding of the PR was that native commands are removed by jlink if the user is packaging on a mac for the App Store. I thought this was a workaround that solved the immediate problem of not being able to submit the app to App Store. (However, I don't know how the app is supposed to be started without a launcher...) |
|
Mailing list message from Michael Hall on core-libs-dev:
I?d have to agree with Apple DTS that this is an interesting exception.
I thought that if the app was indicated as intended for the App Store and native commands were also indicated an error would be thrown and the app not built. It doesn?t allow apps that will fail to attempt the App Store but does nothing for getting them there. Switching from an error to a warning and forcing the native commands to be stripped would allow the app into the app store but with unknown functionality probably not working. My understanding. |
jpackage supplies an alternative launcher that doesn't have plist. |
|
Mailing list message from Alexander Matveev on core-libs-dev: Hi Michael,
There is no a way to do the signing of standalone app image and app image will not get signed if you generating DMG or PKG by providing app image via ?app-image and specifing signing. Only PKG will get signed if signing is specified with ?app-image. Currently jpackage will block following command: I think we can allow app-image type with app image input if signing is requested and command from above will just sign app image in place. Generating DMG or PKG from ?app-image and with signing enabled will not sign app image as it currently do. If no objections for proposed enhancement I will go ahead and file one. Thanks, |
|
Mailing list message from Michael Hall on core-libs-dev:
Alexander I?m not entirely sure I?m following this. I assumed it wasn?t currently possible to do standalone signing against an application bundle but would require an enhancement request. If you are saying you will do that request yourself, that?s fine. Thanks |
|
Mailing list message from Alexander Matveev on core-libs-dev: Hi Michael, I filed following enhancement for signing user provided app image and yes it will be two step process. Invoke jpackage to generate image, then user can modified it and then invoke jpackage again to sign it. For JDK-8286122, I will integrate proposed fix as is. Error will be thrown if user tries to include native commands for Mac App Store image. Still not sure if we will provide solution for including native commands with Mac App Store. Including special versions of native commands with jpackage will work only if runtime is generated by jpackage. It will not solve issue with user provided runtime. Also, it is not clear if app updates will require same unique ID based on UUID across app versions. Also, many functionality provided by native JDK commands can be used via ToolProvider. For example if you include jdk.jpackage module with your app, you should able to use jpackage functionality from your app via ToolProvider without actual jpackage native command. Thanks,
|
|
Mailing list message from Michael Hall on core-libs-dev:
This enhancement doesn?t directly concern this issue. Although given this enhancement it would be possible to provide a temporary ?hack? fix without building it into jpackage. Where the executables are changed to make them unique in the post-processing step. If it is not your intention to address this issue given this enhancement you would probably have to check the provided application bundle to be sure it doesn?t have the colliding Info.plist native commands. What Apple DTS provided may of had commands for this? I don?t remember. Or you would have to just fail any passed application bundles with native java commands.From https://bugs.openjdk.java.net/browse/JDK-8286850 <https://bugs.openjdk.java.net/browse/JDK-8286850>
I am not sure you would want to change this. If the developer doesn?t want to do any post-processing of the application bundle then they should be able to use the current invocations unchanged to do this?
Which might be another way this could be useful. If any 3rd parties want to generate their own application bundles they could still use jpackage as the definitive way to sign those. Thanks |
kevinrushforth
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm just catching up with this thread. Based on the analysis, it's pretty clear that any solution that actually allows bundling native launchers is going to take more research, and is out of scope for this bug. Some combination of JDK-8286850 and/or providing Java launchers that don't have an Info.plist is likely needed to provide a complete solution.
Given that, I think that this PR seems a reasonable fix to avoid creating an app bundle that can't be uploaded to the app store.
|
/integrate |
|
Going to push as commit b523c88.
Your commit was automatically rebased without conflicts. |
|
@sashamatveev Pushed as commit b523c88. 💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored. |
|
Mailing list message from Michael Hall on core-libs-dev:
I was just thinking about this. If you wanted a workaround to suggest to the user on the original issue. You could jar the native executables, extract them to a known accessible location, and then runtime them. Having the commands jar?d would get them past the App Store check. Runtime on the client machine I doubt would object to the duplicate bundle id?s on absolute path execution but I haven?t double checked that. Also avoids issues with quarantine xattr?s. For example? public static void extractArchive(Path archiveFile, Path destPath) throws IOException { @SuppressWarnings("unchecked") // copy or create new or updated Path entryDest = destPath.resolve(fileName); boolean debug = Boolean.getBoolean("R.debug"); |
|
Mailing list message from Alexander Matveev on core-libs-dev: Hi Michael, I think it will be a problem to implement this for native launchers. Thanks, On May 18, 2022, at 6:44 PM, Michael Hall <mik3hall at gmail.com<mailto:mik3hall at gmail.com>> wrote: On May 11, 2022, at 4:39 PM, Alexander Matveev <almatvee at openjdk.java.net<mailto:almatvee at openjdk.java.net>> wrote: - It is not possible to support native JDK commands such as "java" inside Mac App Store bundles due to embedded info.plist. Workarounds suggested in JDK-8286122 does not seems to be visible. I was just thinking about this. If you wanted a workaround to suggest to the user on the original issue. You could jar the native executables, extract them to a known accessible location, and then runtime them. Having the commands jar?d would get them past the App Store check. Runtime on the client machine I doubt would object to the duplicate bundle id?s on absolute path execution but I haven?t double checked that. Also avoids issues with quarantine xattr?s. For example? if (Files.exists(rscriptCmd)) { if (data.equals("N/A")) { // https://stackoverflow.com/questions/1529611/how-to-write-a-java-program-which-can-extract-a-jar-file-and-store-its-data-in-s @SuppressWarnings("unchecked") // copy or create new or updated if (!entry.getName().startsWith("finance/") || !(entry.getName().length() > 8)) { Path entryDest = destPath.resolve(fileName); if (archiveTime.compareTo(destAttr.creationTime()) > 0) { boolean debug = Boolean.getBoolean("R.debug"); |
|
Mailing list message from Michael Hall on core-libs-dev:
Alexander
Each of the native commands is it?s own app launcher? I didn?t know this. I don?t think it was ever really indicated in the discussion why the commands needed the embedded plist. I think Apple DTS guessed it had something to do with a TCC issue.
I thought maybe jpackage unsigned and resigned the jdk itself. Maybe not. I was talking about simply including a jar in input that ends up in the ?app? directory and the app then has the jar contents extracted from there. No additional write permission?s are required.
Again this possibly my not understanding what additional requirements there are for the commands to be ?launchers?. I was thinking if the embedded plist was still a problem the developer could possibly use commands from a compatible linux distribution. OS/X and linux are both Unix right? It did occur to me that extracting native commands out of a jar might lose the executable posix permission. I?m not sure if jar has any builtin meta information to retain these permissions or if nio allows you to set executable. I haven?t had to address this.
I haven?t tried to address this yet myself for my application. I?m not sure a lot of OS/X application?s do. But it would probably be up to the app to provide an uninstall of some kind. Some app?s are obviously going to need external data.
Again I?m suggesting it as a roughed out workaround suggestion for the developer. There might be issues they would need to work out and might even be problems where it won?t work. Thanks |
|
Mailing list message from Michael Hall on core-libs-dev:
Basically you are telling the developer in https://bugs.openjdk.java.net/browse/JDK-8286122 <https://bugs.openjdk.java.net/browse/JDK-8286122> that it isn?t currently possible for their application to get into the Mac App Store as is. |
|
Mailing list message from Michael Hall on core-libs-dev:
I think it can work for java commands as well. You have to set them executable after extraction and point to the embedded jdk lib directory DYLD_LIBRARY_PATH=JavaCommand.app/Contents/runtime/Contents/Home/lib JavaCommand.app/Contents/MacOS/JavaCommand I can provide the full JavaCommand source code and jpackage invocation if of interest. But yes, it works. From? String v = rtexec(new String[] { Paths.get(dataLoc,"java").toString(),"-version" }); You?d have to figure out how to provide the environment variable on the runtime invocation. That I didn?t verify, but it doesn?t seem impossible. JavaCommand.app/Contents/MacOS/JavaCommand |
Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/8666/head:pull/8666$ git checkout pull/8666Update a local copy of the PR:
$ git checkout pull/8666$ git pull https://git.openjdk.java.net/jdk pull/8666/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 8666View PR using the GUI difftool:
$ git pr show -t 8666Using diff file
Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/8666.diff