fix: explicitly set jvm target compatibility #103
Conversation
|
|
||
| // this is the default but it's better to be explicit (e.g. it may change in Kotlin 1.5) | ||
| tasks.compileKotlin { | ||
| kotlinOptions.jvmTarget = "1.6" |
There was a problem hiding this comment.
question
Why not set this to 1.8 if that's what the outgoing variant thing will be of? Or, what does keeping at 1.6 get us?
There was a problem hiding this comment.
1.6 is the default the kotlin compiler already was using.
It gets us 100% Android compatibility and honestly there's no reason to change it since we don't miss out on anything in Kotlin with it turned on right now (that may change in the future). As far as I can tell the kotlin compiler options are taking precedence here w.r.t bytecode compatibility. The java settings are affecting the gradle metadata we are publishing though.
I wasn't able to set the java settings to 1.6 as the dependency selection failed (other libraries we are using have a 1.8 outgoing variant). FWIW amplify-android has these set to 1.8 and use the R8 desugaring capabilities. I'm not 100% sure what the right answer is on this one yet and because of that I'm actually happy to punt this down the line a bit further if we want.
The jvm 11 setting we were seeing is because when we compiled and published the artifact gradle was running with JVM 11 and defaulted the metadata to that. That's not the right setting of course though since clearly we support JVM versions < 11.
KMP projects don't seem to suffer the same default behavior for some reason (which is both good and bad) so this is currently only affecting the generated services because they are JVM only still. When we generate them as actual multiplatform projects we will have to revisit this.
@kiiadi do you have any gradle/java wisdom to share here?
There was a problem hiding this comment.
are we able to have different jvmTargets per "target" ? I'd imagine the android package we want to target 1.6 for compatibility there - but for all others we probably want to min-ver on 1.8 or (even better) 11.
There was a problem hiding this comment.
Android and JVM share the same implementation, there is no separate Android source set. They can't be shared in KMP. The stdlib, coroutines, ktor, okhttp, etc all just have a single JVM target that just works on Android. We don't want this headache anyway, using a single target makes the most sense for maintenance and sanity reasons.
I disagree with 11. We should support as far back as reasonable and comfortable and there is nothing we need in 11 (or 8 for that matter). The issue here, and where I'm not an expert, is how all this relates to gradle telling you what does and doesn't work together. I had to set this to 1.8 for jvmTarget because some of the libraries report compatibility as 1.8+.
This whole issue stems from (1) the outgoing variant metadata was being inferred from the JVM version gradle was invoked with. That is clearly not the right default. (2) we had some initial customer feedback that hit issues when trying to build with JVM 1.8 (note I'm actually fairly positive it would work fine but gradle reports an issue that it wouldn't work because our metadata was saying it wouldn't. The actual compiled bytecode was 1.6 so this is all just a build system issue currently...or at least that is my understanding).
| } | ||
| tasks.compileTestKotlin { | ||
| kotlinOptions.jvmTarget = "1.6" | ||
| } |
There was a problem hiding this comment.
FYI you could avoid a repeated block with this:
tasks.withType<org.jetbrains.kotlin.gradle.tasks.KotlinCompile> {
kotlinOptions.jvmTarget = "1.6"
}
Explicilty set the java source/target compatibility so that the outgoing gradle variant metadata isn't inferred from the JVM version gradle is running with. see: https://docs.gradle.org/current/userguide/variant_attributes.html#sub:jvm_default_attributes fixes: smithy-lang/smithy-kotlin#258
Issue #, if available:
awslabs/smithy-kotlin#258
Description of changes:
Explicilty set the java source/target compatibility to 1.8 so that the outgoing gradle variant metadata isn't inferred from the JVM version gradle is running with. See: Gradle JVM Default Attributes.
I'm not positive this is the right way to fix this as I'm not an expert on gradle variants or the possible interactions between the kotlin plugin and the java plugin as far as source/target compatibility. At any rate this is likely temporary anyway as KMP handles this slightly differently but our services are generated as normal Kotlin JVM projects (for now). 1.8 was chosen based on what amplify-android currently targets in their build file.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.