Update Kotlin, KSP, KotlinPoet, and Gradle #195
Conversation
|
I should document this somewhere, but plan to be somewhat conservative on kotlin/ksp updates as the user can update them themselves. Not against updating, but it just needs to be for something we need instead of bumping arbitrarily. Looks like kotlinpoet: 1.10.2 didn't have a kotlin version bump so that should be safe to do. |
|
I'd say that it might be worth it to keep up to date so that if there is a breaking change that requires a library update it'll be closer to that version. But there's no guarantee on that so ¯_(ツ)_/¯ |
|
For the libs we are using those should be few and far between |
f1ca5a3 to
9c9d4e3
Compare
|
@evant I'll be keeping this updated until it is time to merge 😄 |
|
@evant I updated to Gradle 8.2.1 and Kotlin 1.9 This will probably help with bridging the gap towards Kotlin 2.0 |
290047f to
89e3ed4
Compare
|
Any idea why the linux build is failing? Does it need more memory? |
e4e3c2d to
1618e1a
Compare
|
Fyi I was testing this locally and was running into issues with the kapt backend. After a bunch of debugging turns out it's running with the embeded gradle kotlin instead of the kotlin version you pull in. This means it was using kotlin 1.8.10 and kotlinx-metadata 0.5.0 which won't work with kotlin 1.9.0. I think I'm just going to go ahead of get rid of the kapt backend, it's deprecated anyway. Was planning on doing a bugfix release before then but it's getting more and more of a pain to deal with |
| buildscript { | ||
| dependencies { | ||
| with(libs.kotlin.gradle.get()) { | ||
| classpath("$group:$name:$embeddedKotlinVersion") | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Forcing Gradle to use Kotlin 1.9 results in errors, which I think has to do with https://kotlinlang.org/docs/whatsnew19.html#separate-compiler-plugins-for-official-kotlin-libraries but in any case Gradle has always said not to force a different version of Kotlin for their runtime because they offer no guarantees (Gradle 8.3 should have Kotlin 1.9 support).
There was a problem hiding this comment.
ah so this overrides the version of kotlin uses when compiling the build script logic right?
There was a problem hiding this comment.
https://docs.gradle.org/8.3-rc-1/release-notes.html
The rc releases sometimes have issues, but I've used them in the past with no problems. It should allow kapt to stay in if you really don't want to remove it yet.
If you do want to remove kapt, I have a stashed changeset that's most of the way there, and I can make a PR. Unless you want the cathartic experience of ripping it out 😅
|
|
||
| plugins { | ||
| kotlin("jvm") | ||
| } | ||
|
|
||
| val libs = the<LibrariesForLibs>() | ||
|
|
||
| kotlin { | ||
| jvmToolchain(17) |
There was a problem hiding this comment.
I don't quite understand the distinction between the jvmToolchain and the target version. Is there a reason we need both and that they should be different?
There was a problem hiding this comment.
The toolchain version specifies which version of Java should be used to compile the project. The target version specifies the least compatible version of bytecode that will be produced (i.e. projects using at least Java 8 can use this library).
|
Are the kapt tests not run on CI? I didn't notice that they were failing locally. I pushed a new commit addressing your comments. |
Hm, I wonder if it has something to do with the toolchain support. I'm running with java 20 so it's jumping to java 17. When I run with it works fine. Ok yeah it does have to do with toolchains. If I remove the toolchain lines and add back setting the source compatibility it works fine. I do wonder if the toolchian thing is worth it? In theory forcing a specific version of javac is good for consistency but running newer versions seems to work fine without it? |
| implementation(files(libs.javaClass.superclass.protectionDomain.codeSource.location)) | ||
| } | ||
|
|
||
| java { |
There was a problem hiding this comment.
note: to run with java 20 I had to add
java {
targetCompatibility = JavaVersion.VERSION_11
}
tasks.withType<KotlinCompile>().configureEach {
compilerOptions.jvmTarget = JvmTarget.JVM_11
}
(17 probably would work as well)
but either way I think it's good to have for consistency.
There was a problem hiding this comment.
Using a toolchain here would probably be a valid use case (and I tested that it works), but the alternative is to not use java 20 until Gradle supports it.
There was a problem hiding this comment.
but the alternative is to not use java 20 until Gradle supports it
yeah true, I might end up adding a target here because it just means you'd be generating older class files than needed so it's not a huge issue, but it's not a priority or anything.
|
TIL that toolchains aren't as good as I thought they were 😅 |
|
I pushed an update cleaning up some of the source and target usage, and removed the toolchains. Could possibly add the toolchain back conditionally in the future to aid in having a minimum required version of java. The kapt failure when using java 20 with the toolchain set is odd; I filed KT-60563 with JetBrains. |
No description provided.