[#1179] feat(build): Make Gravitino build and run against JDK8, 11, 17 #1199
Conversation
| @@ -48,7 +48,7 @@ public static EntitySerDe createEntitySerDe(String name) { | |||
| String className = ENTITY_SERDES.getOrDefault(name, name); | |||
|
|
|||
| try { | |||
| return (EntitySerDe) Class.forName(className).newInstance(); | |||
| return (EntitySerDe) Class.forName(className).getDeclaredConstructor().newInstance(); | |||
There was a problem hiding this comment.
This is deprecated in Java 11+
| @@ -253,7 +253,7 @@ public <T> Builder hiddenImpl(Class<T> targetClass, Class<?>... types) { | |||
|
|
|||
| try { | |||
| Constructor<T> hidden = targetClass.getDeclaredConstructor(types); | |||
| AccessController.doPrivileged(new MakeAccessible(hidden)); | |||
| java.security.AccessController.doPrivileged(new MakeAccessible(hidden)); | |||
There was a problem hiding this comment.
AccessController is deprecated in JDK17 with no replacement, so currently suppress the warning as a workaround.
|
How do we ensure the PRs by other development works for JDK 11 and 17 since GitHub action runs with JDK 8. Do we verify it before a release? |
|
I will change the CI to support running on JDK11 and 17 in the next PR. |
|
|
||
| #jdkVersion is used to specify the version of JDK to build and test against Gravitino, current | ||
| # supported version is 8, 11, and 17. | ||
| jdkVersion = 8 |
There was a problem hiding this comment.
What if the value of jdkVersion differs from that of the JDK installed actually?
There was a problem hiding this comment.
It is the same as before, if the specified jdkVersion is not installed locally, it will download automatically by gradle.
There was a problem hiding this comment.
I mean if the value of jdkVersion is 8 and the JDK you installed is JDK11 or JDK17, so the final behavior is?
There was a problem hiding this comment.
it will build, test against jdk8, no matter which version you installed in your environment.
I'm afraid of CI time bombs, I prefer to test on JDK 11 and 17 before release since there're few JDK compatiable issues. |
Agreed, we can test locally and make sure it's okay to use 8, 11, and 17. Adding them to CI may make it difficult to develop in Git Hub. |
|
I will make the
I will take a try to enable all the CI matrix, if it is too long, I'll see how to handle this. |
What changes were proposed in this pull request?
This PR proposes to add multiple JDK version support for building and running Gravitino server, including JDK8, 11, and 17.
Users could specify the version they want to build in
gradle.properties, like:Or users could specify the version by
./gradlew build -PjdkVersion=8.Our generated jars are JDK 8 version compatible, so it can run against Java runtime 8, 11, and 17.
Why are the changes needed?
To make Gravitino build, test and run against multiple JDK versions.
Fix: #1179
Does this PR introduce any user-facing change?
No.
How was this patch tested?
This was compiled, tested, run on JDK8, 11, and 17 locally.