-
Notifications
You must be signed in to change notification settings - Fork 73
Post Java 11 version bumps #1087
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
core/api/core.api
Outdated
@@ -4742,6 +4742,10 @@ public final class org/jetbrains/kotlinx/dataframe/columns/ColumnPath : java/uti | |||
public fun add (Ljava/lang/String;)Z | |||
public fun addAll (ILjava/util/Collection;)Z | |||
public fun addAll (Ljava/util/Collection;)Z | |||
public synthetic fun addFirst (Ljava/lang/Object;)V |
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.
Interesting, why it was added
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 not sure either, probably because it extends List
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.
Hmm.. It seems that target jdk is now Java 21 too: https://openjdk.org/jeps/431 and not 11 as we intend. Try Xjdk-release=11 in compileKotlin freeCompilerArgs
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 not sure actually, do you think jvmToolChain overrides
tasks.withType<KotlinCompile> {
compilerOptions {
jvmTarget = JvmTarget.JVM_11
}
}
tasks.withType<JavaCompile> {
sourceCompatibility = JavaVersion.VERSION_11.toString()
targetCompatibility = JavaVersion.VERSION_11.toString()
}
?
It could also be that apiDump
simply runs with jvm 21 because it's a gradle plugin. I'll test this
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.
made an issue for it Kotlin/binary-compatibility-validator#290
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.
https://kotlinlang.org/docs/compiler-reference.html#jvm-target-version
JVM target only seems to affect bytecode. jdk release limits API too. I think binary validator is correct here and our library compiled against newer JDK API, while targeting JVM 11 bytecode
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.
Ah I see, but if the target jvm is 11, then it's okay to compile it against a newer version, right? Having the target at 11 allows our users to set their projects to 11 and DF still works
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 believe we need to set jdk release to 11 and toolchain to 21
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 think release and target overlap. I'll set it, but I think setting the target and sourceCompatibility does the same.
Edit: actually, I think the apiDump now works correctly :)
okay seems Kotlin 2.1 is sneaking in from some dependency, need to figure out which, because it makes our jupyter tests fail |
2fd1172
to
f05e29e
Compare
Follow up for: #700
Fixes #696
Closes #699
Fixes #595
Unblocks #771
Bumps various versions that were previously blocked, like Jupyter, Kotest, etc.
Bumps java toolchain version to 21 to be able to update simple-git. I also bumped TC to 21