Skip to content
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

chore: extract bootstrap into separate project with tests #1172

Merged
merged 5 commits into from
Jan 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions .brazil.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
{
"dependencies": {
"org.jetbrains.kotlin:kotlin-stdlib-common:1.9.*": "KotlinStdlibCommon-1.9.x",
"org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.9.*": "KotlinStdlibJdk8-1.9.x",
"org.jetbrains.kotlin:kotlin-stdlib:1.9.*": "KotlinStdlib-1.9.x",
"org.jetbrains.kotlinx:atomicfu-jvm:0.23.1": "AtomicfuJvm-0.23.1",
"org.jetbrains.kotlinx:atomicfu:0.23.1": "Atomicfu-0.23.1",
"org.jetbrains.kotlinx:kotlinx-coroutines-core-jvm:1.7.*": "KotlinxCoroutinesCoreJvm-1.7.x",
"org.jetbrains.kotlinx:kotlinx-coroutines-core:1.7.*": "KotlinxCoroutinesCore-1.7.x",
"org.jetbrains.kotlinx:kotlinx-coroutines-jdk8:1.7.*": "KotlinxCoroutinesJdk8-1.7.x"
},
"packageHandlingRules": {
"versioning": {
"defaultVersionLayout": "{MAJOR}.0.x"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: We will have to remember to change this for our next minor version bump. There's a tradeoff between security (not accidentally bumping version) and maintainability (remembering this also needs to change).

Maybe this should be documented somewhere (e.g internal runbook) for bumping versions?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: We will have to remember to change this for our next minor version bump.

If we land where I hope we land (which is to have only one internal major version) then I expect this won't ever change even when we bump minor version externally. This is set for now to preserve the existing major version and not accidentally bump it (introducing a new MV internally).

},
"ignore": [
"aws.sdk.kotlin:bom",
"aws.sdk.kotlin.crt:aws-crt-kotlin-android",
"aws.sdk.kotlin:testing",
"aws.sdk.kotlin:version-catalog"
],
"resolvesConflictDependencies": {
"org.jetbrains.kotlinx:kotlinx-coroutines-core-jvm:1.7.*": [
"KotlinStdlibCommon-1.9.x",
"KotlinStdlibJdk8-1.9.x"
],
"org.jetbrains.kotlinx:kotlinx-coroutines-jdk8:1.7.*": [
"KotlinStdlibJdk8-1.9.x"
]
}
}
}
49 changes: 49 additions & 0 deletions build-support/build.gradle.kts
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/

plugins {
`kotlin-dsl`
`java-gradle-plugin`
alias(libs.plugins.kotlinx.serialization)
}

group = "aws.sdk.kotlin"

repositories {
mavenCentral()
}

dependencies {
compileOnly(kotlin("gradle-plugin"))
compileOnly(kotlin("gradle-plugin-api"))

implementation(libs.smithy.model)
implementation(libs.smithy.aws.traits)
implementation(libs.kotlinx.serialization.json)

testImplementation(libs.junit.jupiter)
testImplementation(libs.junit.jupiter.params)
testImplementation(libs.kotlin.test.junit5)
}

gradlePlugin {
plugins {
create("sdk-bootstrap") {
id = "sdk-bootstrap"
implementationClass = "aws.sdk.kotlin.gradle.sdk.Bootstrap"
}
}
}

tasks.test {
useJUnitPlatform()
testLogging {
events("passed", "skipped", "failed")
showStandardStreams = true
showStackTraces = true
showExceptions = true
exceptionFormat = org.gradle.api.tasks.testing.logging.TestExceptionFormat.FULL
}
}
13 changes: 13 additions & 0 deletions build-support/settings.gradle.kts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/
rootProject.name = "build-support"

dependencyResolutionManagement {
versionCatalogs {
create("libs") {
from(files("../gradle/libs.versions.toml"))
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/
package aws.sdk.kotlin.gradle.sdk

import org.gradle.api.Project
import software.amazon.smithy.model.Model
import software.amazon.smithy.model.shapes.ServiceShape
import java.io.File
import kotlin.streams.toList

/**
* Represents information needed to generate a smithy projection JSON stanza
*/
data class AwsService(
/**
* The service shape ID name
*/
val serviceShapeId: String,

/**
* The package name to use for the service when generating smithy-build.json
*/
val packageName: String,

/**
* The package version (this should match the sdk version of the project)
*/
val packageVersion: String,

/**
* The path to the model file in aws-sdk-kotlin
*/
val modelFile: File,

/**
* The name of the projection to generate
*/
val projectionName: String,

/**
* The sdkId value from the service trait
*/
val sdkId: String,

/**
* The model version from the service shape
*/
val version: String,

/**
* A description of the service (taken from the title trait)
*/
val description: String? = null,

)

/**
* Get the artifact name to use for the service derived from the sdkId. This will be the `A` in the GAV coordinates
* and the directory name under `services/`.
*/
val AwsService.artifactName: String
get() = sdkIdToArtifactName(sdkId)

/**
* Returns a lambda for a service model file that respects the given bootstrap config
*
* @param project the codegen gradle project
* @param bootstrap the [BootstrapConfig] used to include/exclude a service based on the given config
*/
fun fileToService(
project: Project,
bootstrap: BootstrapConfig,
): (File) -> AwsService? = { file: File ->
val sdkVersion = project.findProperty("sdkVersion") as? String ?: error("expected sdkVersion to be set on project ${project.name}")
val filename = file.nameWithoutExtension
// TODO - Can't enable validation without being able to recognize all traits which requires additional deps on classpath
// This is _OK_ for the build because the CLI will do validation with the correct classpath but for unit tests
// it catches some errors that were difficult to track down. Would be nice to enable
val model = Model.assembler()
.discoverModels() // FIXME - why needed in tests but not in actual gradle build?
.addImport(file.absolutePath)
.assemble()
.result
.get()
val services: List<ServiceShape> = model.shapes(ServiceShape::class.java).sorted().toList()
val service = services.singleOrNull() ?: error("Expected one service per aws model, but found ${services.size} in ${file.absolutePath}: ${services.joinToString { it.id.toString() }}")
val protocolName = service.protocolName()

val serviceTrait = service
.findTrait(software.amazon.smithy.aws.traits.ServiceTrait.ID)
.map { it as software.amazon.smithy.aws.traits.ServiceTrait }
.orNull()
?: error("Expected aws.api#service trait attached to model ${file.absolutePath}")

val sdkId = serviceTrait.sdkId
val packageName = packageNameForService(sdkId)
val packageDescription = "The AWS SDK for Kotlin client for $sdkId"

when {
!bootstrap.serviceMembership.isMember(filename, packageName) -> {
project.logger.info("skipping ${file.absolutePath}, $filename/$packageName not a member of ${bootstrap.serviceMembership}")
null
}

!bootstrap.protocolMembership.isMember(protocolName) -> {
project.logger.info("skipping ${file.absolutePath}, $protocolName not a member of $${bootstrap.protocolMembership}")
null
}

else -> {
project.logger.info("discovered service: ${serviceTrait.sdkId}")
AwsService(
serviceShapeId = service.id.toString(),
packageName = packageNamespaceForService(sdkId),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

correctness: should this be packageNameForService(sdkId)?
it's correct, but I had a comment about the naming below

packageVersion = sdkVersion,
modelFile = file,
projectionName = filename,
sdkId = sdkId,
version = service.version,
description = packageDescription,
)
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/
package aws.sdk.kotlin.gradle.sdk

import org.gradle.api.Plugin
import org.gradle.api.Project

// Dummy plugin, we use a plugin because it's easiest with an included build to apply to a buildscript and get
// the buildscript classpath correct.
class Bootstrap : Plugin<Project> {
override fun apply(project: Project) {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/
package aws.sdk.kotlin.gradle.sdk

import org.gradle.kotlin.dsl.provideDelegate

/**
* Settings related to bootstrapping codegen tasks for AWS service code generation.
*
* Services and protocols can be included or excluded by `+` or `-` prefix. If no prefix is found then it is
* considered included (implicit `+`).
*
* @param services the service names to bootstrap. Services are named by either their model filename without
* the extension or by their artifact/package name.
* @param protocols the names of protocols to bootstrap
*/
class BootstrapConfig(
services: String? = null,
protocols: String? = null,
) {
companion object {
/**
* A bootstrap configuration that includes everything by default
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: this does not include everything by default, it just does not exclude anything right? (i.e no services will be bootstrapped with a BootstrapConfig.ALL?)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No it includes everything. Look at the isMember logic, if inclusions is empty then everything is included by default (thats how ./gradlew :codegen:sdk:bootstrap bootstraps The World (TM))

*/
val ALL: BootstrapConfig = BootstrapConfig()
}

val serviceMembership: Membership by lazy { parseMembership(services) }
val protocolMembership: Membership by lazy { parseMembership(protocols) }
override fun toString(): String =
"BootstrapConfig(serviceMembership=$serviceMembership, protocolMembership=$protocolMembership)"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/
package aws.sdk.kotlin.gradle.sdk

/**
* Service and protocol membership for SDK generation
*/
data class Membership(val inclusions: Set<String> = emptySet(), val exclusions: Set<String> = emptySet())

fun Membership.isMember(vararg memberNames: String): Boolean =
memberNames.none(exclusions::contains) && (inclusions.isEmpty() || memberNames.any(inclusions::contains))
fun parseMembership(rawList: String?): Membership {
if (rawList == null) return Membership()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this early return seems not necessary

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much of this was ported as is


val inclusions = mutableSetOf<String>()
val exclusions = mutableSetOf<String>()

rawList.split(",").map { it.trim() }.forEach { item ->
when {
item.startsWith('-') -> exclusions.add(item.substring(1))
item.startsWith('+') -> inclusions.add(item.substring(1))
else -> inclusions.add(item)
}
}

val conflictingMembers = inclusions.intersect(exclusions)
require(conflictingMembers.isEmpty()) { "$conflictingMembers specified both for inclusion and exclusion in $rawList" }

return Membership(inclusions, exclusions)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/
package aws.sdk.kotlin.gradle.sdk

// The root namespace prefix for SDKs
const val SDK_PACKAGE_NAME_PREFIX: String = "aws.sdk.kotlin.services."

/**
* Get the package name to use for a service from it's `sdkId`
*/
fun packageNameForService(sdkId: String): String =
sdkId.replace(" ", "")
.replace("-", "")
.lowercase()
.kotlinNamespace()
Comment on lines +13 to +17
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

simplification: fun packageNameForService(sdkId: String): String = sdkIdToArtifactName(sdkId).kotlinNamespace()

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't want to change anything from what it was (yet anyway) so much of the naming is ported exactly as it was.


/**
* Get the package namespace for a service from it's `sdkId`
*/
fun packageNamespaceForService(sdkId: String): String = "$SDK_PACKAGE_NAME_PREFIX${packageNameForService(sdkId)}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

naming: namespacedPackageNameForService?

while reading this, I was expecting packageNamespace to just return $SDK_PACKAGE_NAME_PREFIX, not the package name as well


/**
* Remove characters invalid for Kotlin package namespace identifier
*/
fun String.kotlinNamespace(): String = split(".")
.joinToString(separator = ".") { segment -> segment.filter { it.isLetterOrDigit() } }

/**
* Convert an sdkId to the module/artifact name to use
*/
internal fun sdkIdToArtifactName(sdkId: String): String = sdkId.replace(" ", "").replace("-", "").lowercase()

/**
* Maps an sdkId from a model to the local filename to use. This logic has to match the logic used by
* catapult! See AwsSdkCatapultWorkspaceTools:lib/source/merge/smithy-model-handler.ts
*/
fun sdkIdToModelFilename(sdkId: String): String = sdkId.trim().replace("""[\s]+""".toRegex(), "-").lowercase()
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/
package aws.sdk.kotlin.gradle.sdk

import kotlinx.serialization.ExperimentalSerializationApi
import kotlinx.serialization.Required
import kotlinx.serialization.Serializable
import kotlinx.serialization.json.Json
import kotlinx.serialization.json.decodeFromStream
import java.io.File

/**
* Container for AWS service package settings.
* Each service can optionally have a `services/<service>/package.json` file that is used
* to control some aspect of code generation specific to that service
*/
@Serializable
data class PackageSettings(
/**
* The sdkId of the service. This is used as a check that the package settings are used on the correct service
*/
@Required
val sdkId: String,

/**
* Whether to enable generating an auth scheme resolver based on endpoint resolution (rare).
*/
val enableEndpointAuthProvider: Boolean = false,
) {
companion object {

/**
* Parse package settings from the given file path if it exists, otherwise return the default settings with
* the given sdkId.
*/
@OptIn(ExperimentalSerializationApi::class)
fun fromFile(sdkId: String, packageSettingsFile: File): PackageSettings {
if (!packageSettingsFile.exists()) return PackageSettings(sdkId)
val settings = Json.decodeFromStream<PackageSettings>(packageSettingsFile.inputStream())
check(sdkId == settings.sdkId) { "${packageSettingsFile.absolutePath} `sdkId` from settings (${settings.sdkId}) does not match expected `$sdkId`" }
return settings
}
}
}
Loading
Loading