-
Notifications
You must be signed in to change notification settings - Fork 2
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
misc: Upgrade to Kotlin 2.1.0 #123
Open
lauzadis
wants to merge
8
commits into
main
Choose a base branch
from
misc-kotlin-2.1.0
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
06c06da
Remove duplicate instantiation of JVM platform
lauzadis 0686f6c
Upgrade crt-java version
lauzadis d9e2dbe
Upgrade kotlinx-binary-compatibility-validator version and refactor t…
lauzadis cfccf02
ktlintFormat
lauzadis 5a7062e
Changelog
lauzadis 9a63b55
Remove SNAPSHOT dependency
lauzadis 81536c2
Upgrade to latest aws-kotlin-repo-tools, aws-crt-java
lauzadis 5a77cb3
Expand wildcard match on KotlinStdlib-2.x
lauzadis File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
{ | ||
"id": "337cca7e-6a38-4601-b5d2-9f8d12dd60f7", | ||
"type": "misc", | ||
"description": "Upgrade to Kotlin 2.1.0" | ||
} | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,6 @@ | |
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
import aws.sdk.kotlin.gradle.dsl.configurePublishing | ||
import aws.sdk.kotlin.gradle.kmp.IDEA_ACTIVE | ||
import aws.sdk.kotlin.gradle.kmp.configureKmpTargets | ||
|
||
plugins { | ||
|
@@ -24,42 +23,6 @@ configureKmpTargets() | |
kotlin { | ||
explicitApi() | ||
|
||
jvm { | ||
attributes { | ||
attribute<org.gradle.api.attributes.java.TargetJvmEnvironment>( | ||
TargetJvmEnvironment.TARGET_JVM_ENVIRONMENT_ATTRIBUTE, | ||
objects.named(TargetJvmEnvironment.STANDARD_JVM), | ||
) | ||
} | ||
} | ||
|
||
// KMP doesn't support sharing source sets for multiple JVM targets OR JVM + Android targets. | ||
// We can manually declare a `jvmCommon` target and wire it up. It will compile fine but Intellij does | ||
// not support this and the developer experience is abysmal. Kotlin/Native suffers a similar problem and | ||
// we can use the same solution. Simply, if Intellij is running (i.e. the one invoking this script) then | ||
// assume we are only building for JVM. Otherwise declare the additional JVM target for Android and | ||
// set the sourceSet the same for both but with different runtime dependencies. | ||
// See: | ||
// * https://kotlinlang.org/docs/mpp-share-on-platforms.html#share-code-in-libraries | ||
// * https://kotlinlang.org/docs/mpp-set-up-targets.html#distinguish-several-targets-for-one-platform | ||
if (!IDEA_ACTIVE) { | ||
|
||
// NOTE: We don't actually need the Android plugin. All of the Android specifics are handled in aws-crt-java, | ||
// we just need a variant with a different dependency set + some distinguishing attributes. | ||
jvm("android") { | ||
attributes { | ||
attribute( | ||
org.jetbrains.kotlin.gradle.plugin.KotlinPlatformType.Companion.attribute, | ||
org.jetbrains.kotlin.gradle.plugin.KotlinPlatformType.androidJvm, | ||
) | ||
attribute( | ||
TargetJvmEnvironment.TARGET_JVM_ENVIRONMENT_ATTRIBUTE, | ||
objects.named(TargetJvmEnvironment.ANDROID), | ||
) | ||
} | ||
} | ||
} | ||
Comment on lines
-47
to
-61
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. Removed because of:
|
||
|
||
sourceSets { | ||
val commonMain by getting { | ||
dependencies { | ||
|
@@ -83,30 +46,6 @@ kotlin { | |
implementation(libs.mockserver.netty) | ||
} | ||
} | ||
|
||
if (!IDEA_ACTIVE) { | ||
val androidMain by getting { | ||
// re-use the jvm (desktop) sourceSet. We only really care about declaring a variant with a different set | ||
// of runtime dependencies | ||
kotlin.srcDir("jvm/src") | ||
dependsOn(commonMain) | ||
dependencies { | ||
// we need symbols we can resolve during compilation but at runtime (i.e. on device) we depend on the Android dependency | ||
compileOnly(libs.crt.java) | ||
val crtJavaVersion = libs.versions.crt.java.version.get() | ||
implementation("software.amazon.awssdk.crt:aws-crt-android:$crtJavaVersion@aar") | ||
|
||
// FIXME - temporary integration with CompletableFuture while we work out a POC on the jvm target | ||
implementation(libs.kotlinx.coroutines.jdk8) | ||
} | ||
} | ||
|
||
// disable compilation of android test source set. It is the same as the jvmTest sourceSet/tests. This | ||
// sourceSet only exists to create a new variant that is the same in every way except the runtime | ||
// dependency on aws-crt-android. To test this we would need to run it on device/emulator. | ||
tasks.getByName("androidTest").enabled = false | ||
tasks.getByName("compileTestKotlinAndroid").enabled = false | ||
} | ||
} | ||
|
||
sourceSets.all { | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,7 +15,7 @@ buildscript { | |
} | ||
|
||
plugins { | ||
id("org.jetbrains.kotlinx.binary-compatibility-validator") version "0.13.2" | ||
alias(libs.plugins.kotlinx.binary.compatibility.validator) | ||
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. Nice! |
||
alias(libs.plugins.kotlin.multiplatform) apply false | ||
alias(libs.plugins.aws.kotlin.repo.tools.kmp) | ||
alias(libs.plugins.aws.kotlin.repo.tools.artifactsizemetrics) | ||
|
@@ -54,7 +54,7 @@ subprojects { | |
if (project.typedProp<Boolean>("kotlinWarningsAsErrors") == true) { | ||
allprojects { | ||
tasks.withType<org.jetbrains.kotlin.gradle.tasks.KotlinCompile> { | ||
kotlinOptions.allWarningsAsErrors = true | ||
compilerOptions.allWarningsAsErrors = true | ||
} | ||
} | ||
} | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Question: Are we going to do a minor version bump of the CRT?