GoogleCloudPlatform / appengine-plugins

A client Java library to manage App Engine Java applications for any project that performs App Engine Java application management. For example, the Maven, Gradle and Eclipse App Engine plugins, custom user tools, etc.
Apache License 2.0
36 stars 25 forks source link

"--add-opens" flags are being added to JDK 8 invocations #916

Closed etanshaul closed 1 year ago

etanshaul commented 1 year ago

This was found by a Cloud Code IntelliJ user of App Engine standard Java 8. Details here: https://github.com/GoogleCloudPlatform/cloud-code-intellij/issues/3136#issuecomment-1522283942

This PR introduced some conditional logic to add --add-opens flags required for JDK11: https://github.com/GoogleCloudPlatform/appengine-plugins-core/pull/894/files

However, despite having a JDK8 configured, and invoking the appengine runner in this lib, --add-opens is still being added (leading to failure due to non-support in JDK8):

com.google.cloud.tools.appengine.operations.DevAppServerRunner - submitting command: 
/Library/Java/JavaVirtualMachines/jdk1.8.0_251.jdk/Contents/Home/bin/java --add-opens 
java.base/java.net=ALL-UNNAMED --add-opens java.base/sun.net.www.protocol.http=ALL-UNNAMED 
--add-opens java.base/sun.net.www.protocol.https=ALL-UNNAMED -Duse_jetty9_runtime=true -D--
enable_all_permissions=true -Dappengine.sdk.root=/Users/.../google-cloud-
sdk/platform/google_appengine/google/appengine/tools/java -cp /Users/.../google-cloud-
sdk/platform/google_appengine/google/appengine/tools/java/lib/appengine-tools-api.jar 
com.google.appengine.tools.development.DevAppServerMain --address=localhost --port=8080 --
allow_remote_shutdown --disable_update_check --no_java_agent /Users/...
etanshaul commented 1 year ago

Looks to be because the runtime of the IDE plugin is different than the runtime of of the user's configured project (for which the local emulator runs). For example, I set up an app engine project to use JDK8, but the runtime of the plugin, which calls this lib, is JDK17 which is what is fetched by StandardSystemProperty.

The plugin sets the java home path to use here (pulling it from what the project has configured): https://github.com/GoogleCloudPlatform/appengine-plugins-core/blob/8948d05369cfa432d9512bce02f90b81e9cb1522/src/main/java/com/google/cloud/tools/appengine/operations/CloudSdk.java#L287

etanshaul commented 1 year ago

One possible approach here would be to have the callers pass in the JDK version rather than have the lib try to infer it. Alternatively, maybe the library can figure it out based on what is configured in javaHome (see previous comment).

suztomo commented 1 year ago

@etanshaul Doesn't "the runtime of the plugin" (or Cloud Code) use JAVA_HOME too?

etanshaul commented 1 year ago

Doesn't "the runtime of the plugin" (or Cloud Code) use JAVA_HOME too?

The project the user has open in their IDE may use a completely different runtime from what is used to run the IDE and plugins. In this case, Cloud Code passes in the java home path configured by the user's project to this library so it knows which runtime to use (e.g. when emulating a user's app engine application). I'm not sure if that answered the question though?

When I say the users project JDK in IntelliJ I'm talking about this:

Screenshot 2023-04-25 at 9 34 12 PM

in our case, the user configures their project to use jdk8, but the IDE/plugins are on 17

suztomo commented 1 year ago

Thank you for explanation.

I read that "the runtime of the plugin" is the JVM on which IntelliJ is running. That makes sense.