Skip to content

Commit

Permalink
Fix computation of runtime crate versions in generic codegen
Browse files Browse the repository at this point in the history
When the stable/unstable crate versions were split, this caused a regression in determining the crate version during codegen. This enhances our crate-version forwarding code to publish all the crate versions. For impact, see codegen-client-test.
  • Loading branch information
rcoh committed Nov 28, 2023
1 parent 57c3d63 commit 613a588
Show file tree
Hide file tree
Showing 8 changed files with 56 additions and 146 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import software.amazon.smithy.rust.codegen.core.smithy.RuntimeConfig
import software.amazon.smithy.rust.codegen.core.smithy.crateLocation

fun RuntimeConfig.awsRuntimeCrate(name: String, features: Set<String> = setOf()): CargoDependency =
CargoDependency(name, awsRoot().crateLocation(null), features = features)
CargoDependency(name, awsRoot().crateLocation(name), features = features)

object AwsCargoDependency {
fun awsConfig(runtimeConfig: RuntimeConfig) = runtimeConfig.awsRuntimeCrate("aws-config")
Expand Down
2 changes: 2 additions & 0 deletions buildSrc/src/main/kotlin/CrateSet.kt
Original file line number Diff line number Diff line change
Expand Up @@ -79,4 +79,6 @@ object CrateSet {
)

val ENTIRE_SMITHY_RUNTIME = (AWS_SDK_SMITHY_RUNTIME + SERVER_SMITHY_RUNTIME).toSortedSet(compareBy { it.name })

val ALL_CRATES = AWS_SDK_RUNTIME + ENTIRE_SMITHY_RUNTIME
}
19 changes: 16 additions & 3 deletions codegen-core/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
*/

import org.gradle.api.tasks.testing.logging.TestExceptionFormat
import software.amazon.smithy.model.node.Node
import software.amazon.smithy.model.node.ObjectNode
import java.io.ByteArrayOutputStream

plugins {
Expand Down Expand Up @@ -58,13 +60,24 @@ val generateSmithyRuntimeCrateVersion by tasks.registering {
val resourcesDir = "$buildDir/resources/main/software/amazon/smithy/rust/codegen/core"
val versionFile = file("$resourcesDir/runtime-crate-version.txt")
outputs.file(versionFile)
val crateVersion = project.properties["smithy.rs.runtime.crate.version"].toString()
val crateVersion = project.properties["smithy.rs.runtime.crate.stable.version"].toString()
inputs.property("crateVersion", crateVersion)
// version format must be in sync with `software.amazon.smithy.rust.codegen.core.Version`
val version = "$crateVersion\n${gitCommitHash()}"
val version = ObjectNode.builder()
.withMember("githash", gitCommitHash())
.withMember("stableVersion", crateVersion)

val runtimeCrates = ObjectNode.builder()

CrateSet.ENTIRE_SMITHY_RUNTIME.forEach { crate ->
runtimeCrates.withMember(crate.name, project.project.properties[crate.versionPropertyName]!!.toString())
}

version.withMember("runtimeCrates", runtimeCrates.build())

sourceSets.main.get().output.dir(resourcesDir)
doLast {
versionFile.writeText(version)
versionFile.writeText(Node.prettyPrintJson(version.build()))
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,27 +6,35 @@
package software.amazon.smithy.rust.codegen.core

import software.amazon.smithy.codegen.core.CodegenException
import software.amazon.smithy.model.node.Node

// generated as part of the build, see codegen-core/build.gradle.kts
private const val VERSION_FILENAME = "runtime-crate-version.txt"

data class Version(val fullVersion: String, val crateVersion: String) {
data class Version(val fullVersion: String, val stableCrateVersion: String, val crates: Map<String, String>) {
companion object {
// Version must be in the "{smithy_rs_version}\n{git_commit_hash}" format
fun parse(content: String): Version {
val lines = content.lines()
if (lines.size != 2) {
throw IllegalArgumentException("Invalid version format, it should contain `2` lines but contains `${lines.size}` line(s)")
}
return Version(lines.joinToString("-"), lines.first())
val node = Node.parse(content).expectObjectNode()
val githash = node.expectStringMember("githash").value
val stableVersion = node.expectStringMember("stableVersion").value
return Version(
"$githash-$stableVersion", stableVersion,
node.expectObjectMember("runtimeCrates").members.map {
it.key.value to it.value.expectStringNode().value
}
.toMap(),
)
}

// Returns full version in the "{smithy_rs_version}-{git_commit_hash}" format
fun fullVersion(): String =
fromDefaultResource().fullVersion

fun crateVersion(): String =
fromDefaultResource().crateVersion
fun stableCrateVersion(): String =
fromDefaultResource().stableCrateVersion

fun crateVersion(crate: String): String? = fromDefaultResource().crates[crate]

private fun fromDefaultResource(): Version = parse(
Version::class.java.getResource(VERSION_FILENAME)?.readText()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,19 +40,21 @@ data class RuntimeCrateLocation(val path: String?, val versions: CrateVersionMap
}
}

fun RuntimeCrateLocation.crateLocation(crateName: String?): DependencyLocation {
val version = crateName.let { versions.map[crateName] } ?: versions.map[DEFAULT_KEY]
fun RuntimeCrateLocation.crateLocation(crateName: String): DependencyLocation {
val version = crateName.let {
versions.map[crateName]
} ?: Version.crateVersion(crateName)
return when (this.path) {
// CratesIo needs an exact version. However, for local runtime crates we do not
// provide a detected version unless the user explicitly sets one via the `versions` map.
null -> CratesIo(version ?: defaultRuntimeCrateVersion())
else -> Local(this.path, version)
null -> CratesIo(version ?: throw CodegenException("a version must be specified for $crateName"))
else -> Local(this.path)
}
}

fun defaultRuntimeCrateVersion(): String {
try {
return Version.crateVersion()
return Version.stableCrateVersion()
} catch (ex: Exception) {
throw CodegenException("failed to get crate version which sets the default client-runtime version", ex)
}
Expand Down Expand Up @@ -94,10 +96,6 @@ data class RuntimeConfig(
}
}

val crateSrcPrefix: String = cratePrefix.replace("-", "_")

fun runtimeCratesPath(): String? = runtimeCrateLocation.path

fun smithyRuntimeCrate(
runtimeCrateName: String,
optional: Boolean = false,
Expand Down Expand Up @@ -324,7 +322,9 @@ data class RuntimeType(val path: String, val dependency: RustDependency? = null)
fun smithyQuery(runtimeConfig: RuntimeConfig) = CargoDependency.smithyQuery(runtimeConfig).toType()
fun smithyRuntime(runtimeConfig: RuntimeConfig) = CargoDependency.smithyRuntime(runtimeConfig).toType()
fun smithyRuntimeApi(runtimeConfig: RuntimeConfig) = CargoDependency.smithyRuntimeApi(runtimeConfig).toType()
fun smithyRuntimeApiClient(runtimeConfig: RuntimeConfig) = CargoDependency.smithyRuntimeApiClient(runtimeConfig).toType()
fun smithyRuntimeApiClient(runtimeConfig: RuntimeConfig) =
CargoDependency.smithyRuntimeApiClient(runtimeConfig).toType()

fun smithyTypes(runtimeConfig: RuntimeConfig) = CargoDependency.smithyTypes(runtimeConfig).toType()
fun smithyXml(runtimeConfig: RuntimeConfig) = CargoDependency.smithyXml(runtimeConfig).toType()
private fun smithyProtocolTest(runtimeConfig: RuntimeConfig) =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class VersionTest {
) {
val version = Version.parse(content)
version.fullVersion shouldBe fullVersion
version.crateVersion shouldBe crateVersion
version.stableCrateVersion shouldBe crateVersion
}

@ParameterizedTest()
Expand All @@ -36,30 +36,15 @@ class VersionTest {
@JvmStatic
fun versionProvider() = listOf(
Arguments.of(
"0.47.0\n0198d26096eb1af510ce24766c921ffc5e4c191e",
"""{ "stableVersion": "0.47.0", "githash": "0198d26096eb1af510ce24766c921ffc5e4c191e" }""",
"0.47.0-0198d26096eb1af510ce24766c921ffc5e4c191e",
"0.47.0",
),
Arguments.of(
"release-2022-08-04\ndb48039065bec890ef387385773b37154b555b14",
"""{ "stableVersion": "release-2022-08-04", "githash": "db48039065bec890ef387385773b37154b555b14" }""",
"release-2022-08-04-db48039065bec890ef387385773b37154b555b14",
"release-2022-08-04",
),
Arguments.of(
"0.30.0-alpha\na1dbbe2947de3c8bbbef9446eb442e298f83f200",
"0.30.0-alpha-a1dbbe2947de3c8bbbef9446eb442e298f83f200",
"0.30.0-alpha",
),
Arguments.of(
"0.6-rc1.cargo\nc281800a185b34600b05f8b501a0322074184123",
"0.6-rc1.cargo-c281800a185b34600b05f8b501a0322074184123",
"0.6-rc1.cargo",
),
Arguments.of(
"0.27.0-alpha.1\n643f2ee",
"0.27.0-alpha.1-643f2ee",
"0.27.0-alpha.1",
),
)

@JvmStatic
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ import org.junit.jupiter.params.ParameterizedTest
import org.junit.jupiter.params.provider.Arguments
import org.junit.jupiter.params.provider.MethodSource
import software.amazon.smithy.model.node.Node
import software.amazon.smithy.rust.codegen.core.rustlang.CratesIo
import software.amazon.smithy.rust.codegen.core.rustlang.DependencyLocation
import software.amazon.smithy.rust.codegen.core.Version
import software.amazon.smithy.rust.codegen.core.rustlang.Local
import java.util.Optional

Expand All @@ -35,17 +34,12 @@ class RuntimeTypesTest {
cfg.runtimeCrateLocation shouldBe RuntimeCrateLocation(null, CrateVersionMap(mapOf()))
}

@ParameterizedTest
@MethodSource("runtimeCrateLocationProvider")
fun `runtimeCrateLocation provides dependency location`(
path: String?,
versions: CrateVersionMap,
crateName: String?,
expectedDependencyLocation: DependencyLocation,
) {
val crateLoc = RuntimeCrateLocation(path, versions)
val depLoc = crateLoc.crateLocation(crateName)
depLoc shouldBe expectedDependencyLocation
@Test
fun `runtimeCrateLocation provides dependency location`() {
val crateLoc = RuntimeCrateLocation("/foo", CrateVersionMap(mapOf("aws-smithy-runtime-api" to "999.999")))
crateLoc.crateLocation("aws-smithy-runtime") shouldBe Local("/foo", Version.stableCrateVersion())
crateLoc.crateLocation("aws-smithy-runtime-api") shouldBe Local("/foo", "999.999")
crateLoc.crateLocation("aws-smithy-http") shouldBe Local("/foo", Version.crateVersion("aws-smithy-http"))
}

companion object {
Expand Down Expand Up @@ -90,97 +84,5 @@ class RuntimeTypesTest {
RuntimeCrateLocation("/path", CrateVersionMap(mapOf("a" to "1.0", "b" to "2.0"))),
),
)

@JvmStatic
fun runtimeCrateLocationProvider() = listOf(
// If user specifies `relativePath` in `runtimeConfig`, then that always takes precedence over versions.
Arguments.of(
"/path",
mapOf<String, String>(),
null,
Local("/path"),
),
Arguments.of(
"/path",
mapOf("a" to "1.0", "b" to "2.0"),
null,
Local("/path"),
),
Arguments.of(
"/path",
mapOf("DEFAULT" to "0.1", "a" to "1.0", "b" to "2.0"),
null,
Local("/path", "0.1"),
),

// User does not specify the versions object.
// The version number of the code-generator should be used as the version for all runtime crates.
Arguments.of(
null,
mapOf<String, String>(),
null,
CratesIo(defaultVersion),
),
Arguments.of(
null,
mapOf<String, String>(),
"a",
CratesIo(defaultVersion),
),

// User specifies versions object, setting explicit version numbers for some runtime crates.
// Then the rest of the runtime crates use the code-generator's version as their version.
Arguments.of(
null,
mapOf("a" to "1.0", "b" to "2.0"),
null,
CratesIo(defaultVersion),
),
Arguments.of(
null,
mapOf("a" to "1.0", "b" to "2.0"),
"a",
CratesIo("1.0"),
),
Arguments.of(
null,
mapOf("a" to "1.0", "b" to "2.0"),
"b",
CratesIo("2.0"),
),
Arguments.of(
null,
mapOf("a" to "1.0", "b" to "2.0"),
"c",
CratesIo(defaultVersion),
),

// User specifies versions object, setting DEFAULT and setting version numbers for some runtime crates.
// Then the specified version in DEFAULT is used for all runtime crates, except for those where the user specified a value for in the map.
Arguments.of(
null,
mapOf("DEFAULT" to "0.1", "a" to "1.0", "b" to "2.0"),
null,
CratesIo("0.1"),
),
Arguments.of(
null,
mapOf("DEFAULT" to "0.1", "a" to "1.0", "b" to "2.0"),
"a",
CratesIo("1.0"),
),
Arguments.of(
null,
mapOf("DEFAULT" to "0.1", "a" to "1.0", "b" to "2.0"),
"b",
CratesIo("2.0"),
),
Arguments.of(
null,
mapOf("DEFAULT" to "0.1", "a" to "1.0", "b" to "2.0"),
"c",
CratesIo("0.1"),
),
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ class PythonServerModuleGenerator(

// Render the codegeneration version as module attribute.
private fun RustWriter.renderCodegenVersion() {
rust("""m.add("CODEGEN_VERSION", "${Version.crateVersion()}")?;""")
rust("""m.add("CODEGEN_VERSION", "${Version.stableCrateVersion()}")?;""")
}

// Convert to symbol and check the namespace to figure out where they should be imported from.
Expand Down

0 comments on commit 613a588

Please sign in to comment.