-
Notifications
You must be signed in to change notification settings - Fork 26
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
java17 runtime support for launcher when using a jdk17 build and run env #893
Comments
To repro, use a JDK17 for a mvn appengine:run command on a GAE project and you will see: --- appengine-maven-plugin:2.4.3:run (default-cli) @ courteline --- |
As a side effect, these flags can be removed: -Duse_jetty9_runtime=true -D--enable_all_permissions=true as we are only on jetty9 now, and we do not have a security manager for java8, 11 and 17. |
Confirmed that for command line mode, adding |
Ah, I see. I've added documentation for users to add the extra flags in GoogleCloudPlatform/app-maven-plugin#455, but we can definitely make the changes ourselves in core. Long-term, it would be best if devserver did not modify JDK classes visibility... |
The core plugin still refers to isSandboxEnforced() which can be deleted entirely for java7 which is not supported (everything with java7 string can be deleted including tests
But when using a JDK17 for the java17 runtime (same logic as java8), the devappserver launcher does not pass a needed flag for the devappserver process very similar to the .sh and .cmd launcher, as seen as in GoogleCloudPlatform/appengine-java-standard#13
I will fix this one by adding the 3:
--add-opens java.base/java.net=ALL-UNNAMED
--add-opens java.base/java.base/sun.net.www.protocol.http=ALL-UNNAMED
--add-opens java.base/java.base/sun.net.www.protocol.https=ALL-UNNAMED
flags in the scripts but we also need to add the flag in com.google.cloud.tools.appengine.operations.DevServer.java which is also a process launcher written in java which does not create the good command line when running on jdk17.
Thanks!
The text was updated successfully, but these errors were encountered: