-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Build: Run Iceberg with JDK 17 #7391
Changes from all commits
f776dca
c6f1195
3fc591d
1e64055
8dd29ca
b51b34c
9bcbc9d
1fd914f
3d2fae1
82fec3d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -68,10 +68,36 @@ try { | |
|
||
if (JavaVersion.current() == JavaVersion.VERSION_1_8) { | ||
project.ext.jdkVersion = '8' | ||
project.ext.extraJvmArgs = [] | ||
} else if (JavaVersion.current() == JavaVersion.VERSION_11) { | ||
project.ext.jdkVersion = '11' | ||
project.ext.extraJvmArgs = [] | ||
} else if (JavaVersion.current() == JavaVersion.VERSION_17) { | ||
project.ext.jdkVersion = '17' | ||
project.ext.extraJvmArgs = ["-XX:+IgnoreUnrecognizedVMOptions", | ||
"--add-opens", "java.base/java.io=ALL-UNNAMED", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to open all these? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I collected these flag by debugging individual failures of ut's from test projects (spark / arrow / aws (kryo)) this is a superset i would say we may define only the relevant one's per package, but since there was no harm in passing additional flags hence passed all the flags in all the projects. please let me know your thoughts if the above makes sense, happy to add required flags per projects |
||
"--add-opens", "java.base/java.lang.invoke=ALL-UNNAMED", | ||
"--add-opens", "java.base/java.lang.reflect=ALL-UNNAMED", | ||
"--add-opens", "java.base/java.lang=ALL-UNNAMED", | ||
"--add-opens", "java.base/java.math=ALL-UNNAMED", | ||
"--add-opens", "java.base/java.net=ALL-UNNAMED", | ||
"--add-opens", "java.base/java.nio=ALL-UNNAMED", | ||
"--add-opens", "java.base/java.text=ALL-UNNAMED", | ||
"--add-opens", "java.base/java.time=ALL-UNNAMED", | ||
"--add-opens", "java.base/java.util.concurrent.atomic=ALL-UNNAMED", | ||
"--add-opens", "java.base/java.util.concurrent=ALL-UNNAMED", | ||
"--add-opens", "java.base/java.util.regex=ALL-UNNAMED", | ||
"--add-opens", "java.base/java.util=ALL-UNNAMED", | ||
"--add-opens", "java.base/jdk.internal.ref=ALL-UNNAMED", | ||
"--add-opens", "java.base/jdk.internal.reflect=ALL-UNNAMED", | ||
"--add-opens", "java.sql/java.sql=ALL-UNNAMED", | ||
"--add-opens", "java.base/sun.util.calendar=ALL-UNNAMED", | ||
"--add-opens", "java.base/sun.nio.ch=ALL-UNNAMED", | ||
"--add-opens", "java.base/sun.nio.cs=ALL-UNNAMED", | ||
"--add-opens", "java.base/sun.security.action=ALL-UNNAMED", | ||
"--add-opens", "java.base/sun.util.calendar=ALL-UNNAMED"] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe we could provide a list of classes to open and generate the |
||
} else { | ||
throw new GradleException("This build must be run with JDK 8 or 11 but was executed with JDK " + JavaVersion.current()) | ||
throw new GradleException("This build must be run with JDK 8 or 11 or 17 but was executed with JDK " + JavaVersion.current()) | ||
} | ||
|
||
apply plugin: 'com.gorylenko.gradle-git-properties' | ||
|
@@ -217,6 +243,8 @@ subprojects { | |
|
||
maxHeapSize = "1500m" | ||
|
||
jvmArgs += project.property('extraJvmArgs') | ||
|
||
testLogging { | ||
events "failed" | ||
exceptionFormat "full" | ||
|
@@ -490,6 +518,7 @@ project(':iceberg-aws') { | |
task integrationTest(type: Test) { | ||
testClassesDirs = sourceSets.integration.output.classesDirs | ||
classpath = sourceSets.integration.runtimeClasspath | ||
jvmArgs += project.property('extraJvmArgs') | ||
} | ||
} | ||
|
||
|
@@ -558,6 +587,7 @@ project(':iceberg-delta-lake') { | |
task integrationTest(type: Test) { | ||
testClassesDirs = sourceSets.integration.output.classesDirs | ||
classpath = sourceSets.integration.runtimeClasspath | ||
jvmArgs += project.property('extraJvmArgs') | ||
singhpk234 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
check.dependsOn integrationTest | ||
} | ||
|
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.
sorry forgot to ask, why is this changed from 6 to 9? Does JDK17 change default precision of timestamp or something like that?
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.
yes with my mac on using JDK 17 existing regex would pass, but with ubuntu with JDK 17, was getting
2023-04-20T19:17:17.748170628Z
more details here: #7391 (comment)