From be399dd5441d522568ac04a3b11f3c219dc36117 Mon Sep 17 00:00:00 2001 From: Ian Smith Botsford Date: Fri, 20 May 2022 17:42:37 +0000 Subject: [PATCH 1/3] feat: enhance generic codegen to be more KMP-friendly --- .../0011f293-6d95-46ab-ac8c-59d11ef54559.json | 8 + .../sdk/kotlin/codegen/AwsKotlinDependency.kt | 4 +- .../aws/sdk/kotlin/codegen/GradleGenerator.kt | 24 ++- .../core/QueryHttpBindingProtocolGenerator.kt | 7 +- services/build.gradle.kts | 200 ++++++++---------- services/s3/e2eTest/S3TestUtils.kt | 2 + 6 files changed, 118 insertions(+), 127 deletions(-) create mode 100644 .changes/0011f293-6d95-46ab-ac8c-59d11ef54559.json diff --git a/.changes/0011f293-6d95-46ab-ac8c-59d11ef54559.json b/.changes/0011f293-6d95-46ab-ac8c-59d11ef54559.json new file mode 100644 index 00000000000..d918ada713b --- /dev/null +++ b/.changes/0011f293-6d95-46ab-ac8c-59d11ef54559.json @@ -0,0 +1,8 @@ +{ + "id": "0011f293-6d95-46ab-ac8c-59d11ef54559", + "type": "feature", + "description": "Enhance generic codegen to be more KMP-friendly", + "issues": [ + "awslabs/aws-sdk-kotlin#460" + ] +} \ No newline at end of file diff --git a/codegen/smithy-aws-kotlin-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/AwsKotlinDependency.kt b/codegen/smithy-aws-kotlin-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/AwsKotlinDependency.kt index 1630061f982..d57d9bc97b5 100644 --- a/codegen/smithy-aws-kotlin-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/AwsKotlinDependency.kt +++ b/codegen/smithy-aws-kotlin-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/AwsKotlinDependency.kt @@ -62,8 +62,8 @@ internal fun KotlinDependency.dependencyNotation(allowProjectNotation: Boolean = val dep = this return if (allowProjectNotation && sameProjectDeps.contains(dep)) { val projectNotation = sameProjectDeps[dep] - "${dep.config}($projectNotation)" + "${dep.config.kmpName}($projectNotation)" } else { - "${dep.config}(\"${dep.group}:${dep.artifact}:${dep.version}\")" + "${dep.config.kmpName}(\"${dep.group}:${dep.artifact}:${dep.version}\")" } } diff --git a/codegen/smithy-aws-kotlin-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/GradleGenerator.kt b/codegen/smithy-aws-kotlin-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/GradleGenerator.kt index e80dca82e1f..e6379a1728f 100644 --- a/codegen/smithy-aws-kotlin-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/GradleGenerator.kt +++ b/codegen/smithy-aws-kotlin-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/GradleGenerator.kt @@ -49,16 +49,26 @@ class GradleGenerator : KotlinIntegration { writer.write("val kotlinVersion: String by project") - val dependencies = delegator.dependencies.mapNotNull { it.properties["dependency"] as? KotlinDependency }.distinct() + val allDependencies = delegator.dependencies.mapNotNull { it.properties["dependency"] as? KotlinDependency }.distinct() - writer.write("") - .withBlock("dependencies {", "}") { - val orderedDependencies = dependencies.sortedWith(compareBy({ it.config }, { it.artifact })) - for (dependency in orderedDependencies) { - write(dependency.dependencyNotation()) + writer + .write("") + .withBlock("kotlin {", "}") { + withBlock("sourceSets {", "}") { + allDependencies + .sortedWith(compareBy({ it.config }, { it.artifact })) + .groupBy { it.config.sourceSet } + .forEach { (sourceSet, dependencies) -> + withBlock("$sourceSet {", "}") { + withBlock("dependencies {", "}") { + dependencies + .map { it.dependencyNotation() } + .forEach(::write) + } + } + } } } - .write("") val contents = writer.toString() delegator.fileManifest.writeFile("build.gradle.kts", contents) diff --git a/codegen/smithy-aws-kotlin-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/protocols/core/QueryHttpBindingProtocolGenerator.kt b/codegen/smithy-aws-kotlin-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/protocols/core/QueryHttpBindingProtocolGenerator.kt index b832e997cfa..8a245b47727 100644 --- a/codegen/smithy-aws-kotlin-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/protocols/core/QueryHttpBindingProtocolGenerator.kt +++ b/codegen/smithy-aws-kotlin-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/protocols/core/QueryHttpBindingProtocolGenerator.kt @@ -155,10 +155,9 @@ abstract class AbstractQueryFormUrlSerializerGenerator( ) { // render the serde descriptors descriptorGenerator(ctx, shape, members, writer).render() - if (shape.isUnionShape) { - SerializeUnionGenerator(ctx, members, writer, defaultTimestampFormat).render() - } else { - SerializeStructGenerator(ctx, members, writer, defaultTimestampFormat).render() + when (shape) { + is UnionShape -> SerializeUnionGenerator(ctx, shape, members, writer, defaultTimestampFormat).render() + else -> SerializeStructGenerator(ctx, members, writer, defaultTimestampFormat).render() } } diff --git a/services/build.gradle.kts b/services/build.gradle.kts index 3ddf075b187..2baa85c485b 100644 --- a/services/build.gradle.kts +++ b/services/build.gradle.kts @@ -1,14 +1,19 @@ +import org.jetbrains.kotlin.gradle.targets.jvm.tasks.KotlinJvmTest +import org.jetbrains.kotlin.gradle.tasks.KotlinTest + /* * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. * SPDX-License-Identifier: Apache-2.0. * */ plugins { - kotlin("jvm") + kotlin("multiplatform") `maven-publish` id("org.jetbrains.dokka") } +val platforms = listOf("common", "jvm") + val sdkVersion: String by project val kotlinVersion: String by project val coroutinesVersion: String by project @@ -17,149 +22,116 @@ val kotestVersion: String by project val optinAnnotations = listOf( "aws.smithy.kotlin.runtime.util.InternalApi", "aws.sdk.kotlin.runtime.InternalSdkApi", + "kotlin.RequiresOptIn", ) +kotlin { + jvm() // Create a JVM target with the default name 'jvm' +} + subprojects { group = "aws.sdk.kotlin" version = sdkVersion apply { - plugin("org.jetbrains.kotlin.jvm") + plugin("org.jetbrains.kotlin.multiplatform") plugin("org.jetbrains.dokka") } - // have generated sdk's opt-in to internal runtime features - kotlin.sourceSets.all { - optinAnnotations.forEach { languageSettings.optIn(it) } - } + logger.info("configuring: $project") - kotlin { - sourceSets.getByName("main") { - kotlin.srcDir("common/src") - kotlin.srcDir("generated-src/main/kotlin") - } - sourceSets.getByName("test") { - kotlin.srcDir("common/test") - kotlin.srcDir("generated-src/test") - - dependencies { - implementation(kotlin("test-junit5")) - implementation("org.jetbrains.kotlin:kotlin-test-common:$kotlinVersion") - implementation("org.jetbrains.kotlinx:kotlinx-coroutines-test:$coroutinesVersion") - implementation(project(":aws-runtime:testing")) - implementation("io.kotest:kotest-assertions-core:$kotestVersion") - } + platforms.forEach { platform -> + configure(listOf(project)) { + apply(from = rootProject.file("gradle/$platform.gradle")) } } - tasks.withType().configureEach { - dokkaSourceSets { - named("main") { - platform.set(org.jetbrains.dokka.Platform.jvm) - sourceRoots.from(kotlin.sourceSets.getByName("main").kotlin.srcDirs) + kotlin { + sourceSets { + all { + val srcDir = if (name.endsWith("Main")) "src" else "test" + val resourcesPrefix = if (name.endsWith("Test")) "test-" else "" + // the name is always the platform followed by a suffix of either "Main" or "Test" (e.g. jvmMain, commonTest, etc) + val platform = name.substring(0, name.length - 4) + kotlin.srcDir("$platform/$srcDir") + resources.srcDir("$platform/${resourcesPrefix}resources") + + languageSettings.progressiveMode = true + + // have generated sdk's opt-in to internal runtime features + optinAnnotations.forEach { languageSettings.optIn(it) } } - } - } - - tasks.test { - useJUnitPlatform() - testLogging { - events("passed", "skipped", "failed") - showStandardStreams = true - showStackTraces = true - showExceptions = true - exceptionFormat = org.gradle.api.tasks.testing.logging.TestExceptionFormat.FULL - } - } - - - tasks.compileKotlin { - kotlinOptions { - jvmTarget = "1.8" // this is the default but it's better to be explicit (e.g. it may change in Kotlin 1.5) - allWarningsAsErrors = false // FIXME Tons of errors occur in generated code - } - } - tasks.compileTestKotlin { - kotlinOptions { - jvmTarget = "1.8" // this is the default but it's better to be explicit (e.g. it may change in Kotlin 1.5) - allWarningsAsErrors = false // FIXME Tons of errors occur in generated code - // Enable coroutine runTests in 1.6.10 - // NOTE: may be removed after coroutines-test runTests becomes stable - freeCompilerArgs = freeCompilerArgs + "-opt-in=kotlin.RequiresOptIn" - } - } - // FIXME - we can remove this when we implement generated services as multiplatform. - setOutgoingVariantMetadata() + getByName("commonMain") { + kotlin.srcDir("generated-src/main/kotlin") + } - val sourcesJar by tasks.creating(Jar::class) { - group = "publishing" - description = "Assembles Kotlin sources jar" - classifier = "sources" - from(sourceSets.getByName("main").allSource) - } + getByName("commonTest") { + kotlin.srcDir("generated-src/test") - // FIXME - kotlin multiplatform configures publications for you so when we switch we can remove this - // and just apply "publish.gradle" from the set of root gradle scripts (just like we do for the runtime) - plugins.apply("maven-publish") - publishing { - publications { - create("sdk"){ - from(components["java"]) - artifact(sourcesJar) + dependencies { + implementation("org.jetbrains.kotlinx:kotlinx-coroutines-test:$coroutinesVersion") + implementation(project(":aws-runtime:testing")) + } } } - } - - apply(from = rootProject.file("gradle/publish.gradle")) - if (project.file("e2eTest").exists()) { - - kotlin.target.compilations { - val main by getting - val e2eTest by creating { - defaultSourceSet { - kotlin.srcDir("e2eTest") - dependencies { - implementation(main.compileDependencyFiles + main.runtimeDependencyFiles + main.output.classesDirs) + if (project.file("e2eTest").exists()) { + jvm().compilations { + val main by getting + val e2eTest by creating { + defaultSourceSet { + kotlin.srcDir("e2eTest") + + dependencies { + // Compile against the main compilation's compile classpath and outputs: + implementation(main.compileDependencyFiles + main.output.classesDirs) + + implementation(kotlin("test")) + implementation(kotlin("test-junit5")) + implementation(project(":aws-runtime:testing")) + implementation(project(":tests:e2e-test-util")) + } + } - implementation(kotlin("test")) - implementation(kotlin("test-junit5")) - implementation(project(":aws-runtime:testing")) - implementation(project(":tests:e2e-test-util")) + kotlinOptions { + // Enable coroutine runTests in 1.6.10 + // NOTE: may be removed after coroutines-test runTests becomes stable + freeCompilerArgs = freeCompilerArgs + "-opt-in=kotlin.RequiresOptIn" } - } - kotlinOptions { - // Enable coroutine runTests in 1.6.10 - // NOTE: may be removed after coroutines-test runTests becomes stable - freeCompilerArgs = freeCompilerArgs + "-opt-in=kotlin.RequiresOptIn" - } - tasks.register("e2eTest") { - description = "Run e2e service tests" - group = "verification" - classpath = compileDependencyFiles + runtimeDependencyFiles - testClassesDirs = output.classesDirs - useJUnitPlatform() - testLogging { - events("passed", "skipped", "failed") - showStandardStreams = true - showStackTraces = true - showExceptions = true - exceptionFormat = org.gradle.api.tasks.testing.logging.TestExceptionFormat.FULL + tasks.register("e2eTest") { + description = "Run e2e service tests" + group = "verification" + + // Run the tests with the classpath containing the compile dependencies (including 'main'), + // runtime dependencies, and the outputs of this compilation: + classpath = compileDependencyFiles + runtimeDependencyFiles + output.allOutputs + + // Run only the tests from this compilation's outputs: + testClassesDirs = output.classesDirs + + useJUnitPlatform() + testLogging { + events("passed", "skipped", "failed") + showStandardStreams = true + showStackTraces = true + showExceptions = true + exceptionFormat = org.gradle.api.tasks.testing.logging.TestExceptionFormat.FULL + } } } } } } -} + dependencies { + dokkaPlugin(project(":dokka-aws")) + } -// fixes outgoing variant metadata: https://github.com/awslabs/smithy-kotlin/issues/258 -fun Project.setOutgoingVariantMetadata() { - tasks.withType() { - val javaVersion = JavaVersion.VERSION_1_8.toString() - sourceCompatibility = javaVersion - targetCompatibility = javaVersion + tasks.withType { + kotlinOptions.allWarningsAsErrors = false // FIXME Tons of errors occur in generated code } + + apply(from = rootProject.file("gradle/publish.gradle")) } diff --git a/services/s3/e2eTest/S3TestUtils.kt b/services/s3/e2eTest/S3TestUtils.kt index 792fc35c850..eb9f1beaf44 100644 --- a/services/s3/e2eTest/S3TestUtils.kt +++ b/services/s3/e2eTest/S3TestUtils.kt @@ -34,6 +34,8 @@ object S3TestUtils { if (testBucket == null) { testBucket = prefix + UUID.randomUUID() + println("Creating S3 bucket: $testBucket") + client.createBucket { bucket = testBucket createBucketConfiguration { From 0a4a4da4a102d84c84a3bc3384b48ebb8a0d0df3 Mon Sep 17 00:00:00 2001 From: Ian Smith Botsford Date: Mon, 23 May 2022 22:56:45 +0000 Subject: [PATCH 2/3] addressing PR feedback --- services/build.gradle.kts | 13 ++++++++----- services/s3/e2eTest/S3TestUtils.kt | 2 +- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/services/build.gradle.kts b/services/build.gradle.kts index 2baa85c485b..86e48273b74 100644 --- a/services/build.gradle.kts +++ b/services/build.gradle.kts @@ -1,11 +1,11 @@ -import org.jetbrains.kotlin.gradle.targets.jvm.tasks.KotlinJvmTest -import org.jetbrains.kotlin.gradle.tasks.KotlinTest - /* * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. * SPDX-License-Identifier: Apache-2.0. - * */ + +import org.jetbrains.kotlin.gradle.targets.jvm.tasks.KotlinJvmTest +import org.jetbrains.kotlin.gradle.tasks.KotlinTest + plugins { kotlin("multiplatform") `maven-publish` @@ -130,7 +130,10 @@ subprojects { } tasks.withType { - kotlinOptions.allWarningsAsErrors = false // FIXME Tons of errors occur in generated code + kotlinOptions { + allWarningsAsErrors = false // FIXME Tons of errors occur in generated code + jvmTarget = "1.8" // fixes outgoing variant metadata: https://github.com/awslabs/smithy-kotlin/issues/258 + } } apply(from = rootProject.file("gradle/publish.gradle")) diff --git a/services/s3/e2eTest/S3TestUtils.kt b/services/s3/e2eTest/S3TestUtils.kt index eb9f1beaf44..02995381080 100644 --- a/services/s3/e2eTest/S3TestUtils.kt +++ b/services/s3/e2eTest/S3TestUtils.kt @@ -44,7 +44,7 @@ object S3TestUtils { } client.waitUntilBucketExists { bucket = testBucket } - } + } else println("Using existing S3 bucket: $testBucket") client.putBucketLifecycleConfiguration { bucket = testBucket From 88cdf919a3f00209a1d5583f035a50f548e772d5 Mon Sep 17 00:00:00 2001 From: Ian Smith Botsford Date: Mon, 23 May 2022 23:06:45 +0000 Subject: [PATCH 3/3] denote breaking change and describe effect more in changelog --- .changes/0011f293-6d95-46ab-ac8c-59d11ef54559.json | 2 +- gradle.properties | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.changes/0011f293-6d95-46ab-ac8c-59d11ef54559.json b/.changes/0011f293-6d95-46ab-ac8c-59d11ef54559.json index d918ada713b..8ccb4ab052e 100644 --- a/.changes/0011f293-6d95-46ab-ac8c-59d11ef54559.json +++ b/.changes/0011f293-6d95-46ab-ac8c-59d11ef54559.json @@ -1,7 +1,7 @@ { "id": "0011f293-6d95-46ab-ac8c-59d11ef54559", "type": "feature", - "description": "Enhance generic codegen to be more KMP-friendly", + "description": "Enhance generic codegen to be more KMP-friendly. This is a **breaking change** which means service client artifacts will now include their platform name (e.g., `s3-jvm-.jar` vs `s3-.jar`). Users consuming dependencies through the Gradle Kotlin plugin will have this handled automatically for them.", "issues": [ "awslabs/aws-sdk-kotlin#460" ] diff --git a/gradle.properties b/gradle.properties index 0c6a3d2262a..d53b3539580 100644 --- a/gradle.properties +++ b/gradle.properties @@ -6,7 +6,7 @@ kotlin.native.ignoreDisabledTargets=true org.gradle.jvmargs=-Xmx6g -XX:MaxPermSize=6g -XX:MaxMetaspaceSize=1G # sdk -sdkVersion=0.15.3-SNAPSHOT +sdkVersion=0.16.0-SNAPSHOT # codegen smithyVersion=1.17.0