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

Support granular control of specifying runtime crate versions #1635

Merged
merged 13 commits into from
Aug 17, 2022
Merged
44 changes: 44 additions & 0 deletions CHANGELOG.next.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,47 @@ message = "Add codegen version to generated package metadata"
references = ["smithy-rs#1612"]
meta = { "breaking" = false, "tada" = false, "bug" = false }
author = "unexge"

[[smithy-rs]]
message = """
Support granular control of specifying runtime crate versions.

For code generation, the field `runtimeConfig.version` in smithy-build.json has been removed.
The new field `runtimeConfig.versions` is an object whose keys are runtime crate names (e.g. `aws-smithy-http`),
and values are user-specified versions.

If you previously set `version = "DEFAULT"`, the migration path is simple.
By setting `versions` with an empty object or just not setting it at all,
the version number of the code generator will be used as the version for all runtime crates.

If you specified a certain version such as `version = "0.47.0", you can migrate to a special reserved key `DEFAULT`.
weihanglo marked this conversation as resolved.
Show resolved Hide resolved
The equivalent JSON config would look like:

```json
{
"runtimeConfig": {
"versions": {
"DEFAULT": "0.47.0"
}
}
}
```

Then all runtime crates are set with version 0.47.0 by default unless overridden by specific crates. For example,

```json
{
"runtimeConfig": {
"versions": {
"DEFAULT": "0.47.0",
"aws-smithy-http": "0.47.1"
}
}
}
```

implies that we're using `aws-smithy-http` 0.47.1 specifically. For the rest of the crates, it will default to 0.47.0.
"""
references = ["smithy-rs#1635", "smithy-rs#1416"]
meta = { "breaking" = true, "tada" = true, "bug" = false }
Copy link
Collaborator

Choose a reason for hiding this comment

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

For breaking changes, add instructions to the message on what changes need to be made when upgrading. You can switch the message to triple quotes if it needs to be multi-line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated. Please review again. Thanks!

author = "weihanglo"
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ fun RuntimeConfig.awsRoot(): RuntimeCrateLocation {
path
}
return runtimeCrateLocation.copy(
path = updatedPath, version = runtimeCrateLocation.version?.let { defaultSdkVersion() },
path = updatedPath, versions = runtimeCrateLocation.versions,
)
}

Expand All @@ -61,7 +61,7 @@ object AwsRuntimeType {
}

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

fun RuntimeConfig.awsHttp(): CargoDependency = awsRuntimeDependency("aws-http")
fun RuntimeConfig.awsTypes(): CargoDependency = awsRuntimeDependency("aws-types")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package software.amazon.smithy.rust.codegen.smithy

import software.amazon.smithy.codegen.core.CodegenException
import software.amazon.smithy.codegen.core.Symbol
import software.amazon.smithy.model.node.Node
import software.amazon.smithy.model.node.ObjectNode
import software.amazon.smithy.model.traits.TimestampFormatTrait
import software.amazon.smithy.rust.codegen.rustlang.CargoDependency
Expand All @@ -23,26 +24,27 @@ import software.amazon.smithy.rust.codegen.rustlang.asType
import software.amazon.smithy.rust.codegen.util.orNull
import java.util.Optional

private const val DEFAULT_KEY = "DEFAULT"

/**
* Location of the runtime crates (aws-smithy-http, aws-smithy-types etc.)
*
* This can be configured via the `runtimeConfig.version` field in smithy-build.json
* This can be configured via the `runtimeConfig.versions` field in smithy-build.json
*/
data class RuntimeCrateLocation(val path: String?, val version: String?) {
init {
check(path != null || version != null) {
"path ($path) or version ($version) must not be null"
}
}

data class RuntimeCrateLocation(val path: String?, val versions: CrateVersionMap) {
companion object {
fun Path(path: String) = RuntimeCrateLocation(path, null)
fun Path(path: String) = RuntimeCrateLocation(path, CrateVersionMap(emptyMap()))
}
}

fun RuntimeCrateLocation.crateLocation(): DependencyLocation = when (this.path) {
null -> CratesIo(this.version!!)
else -> Local(this.path, this.version)
fun RuntimeCrateLocation.crateLocation(crateName: String?): DependencyLocation {
val version = crateName.let { versions.map[crateName] } ?: versions.map[DEFAULT_KEY]
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())
weihanglo marked this conversation as resolved.
Show resolved Hide resolved
else -> Local(this.path, version)
}
}

fun defaultRuntimeCrateVersion(): String {
Expand All @@ -53,6 +55,14 @@ fun defaultRuntimeCrateVersion(): String {
}
}

/**
* A mapping from crate name to a user-specified version.
*/
@JvmInline
value class CrateVersionMap(
Copy link
Contributor

@david-perez david-perez Aug 16, 2022

Choose a reason for hiding this comment

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

TIL about value class. Very interesting.

This is the first time we introduce them in the codebase. I wonder why we're not using them? They seem to be better at encoding "transparent" newtypes, like here, something for which we are relying on data class.

cc someone who actually knows Kotlin @rcoh.

Copy link
Collaborator

Choose a reason for hiding this comment

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

no reason not to use them, TIL about them TBH, they seem handy!

We often end up with more than one field which is why data class is usually useful but for single-member types this seems great!

val map: Map<String, String>,
)

/**
* Prefix & crate location for the runtime crates.
*/
Expand All @@ -67,13 +77,12 @@ data class RuntimeConfig(
*/
fun fromNode(node: Optional<ObjectNode>): RuntimeConfig {
return if (node.isPresent) {
val resolvedVersion = when (val configuredVersion = node.get().getStringMember("version").orNull()?.value) {
"DEFAULT" -> defaultRuntimeCrateVersion()
null -> null
else -> configuredVersion
val crateVersionMap = node.get().getObjectMember("versions").orElse(Node.objectNode()).members.entries.let { members ->
val map = members.associate { it.key.toString() to it.value.expectStringNode().value }
CrateVersionMap(map)
}
val path = node.get().getStringMember("relativePath").orNull()?.value
val runtimeCrateLocation = RuntimeCrateLocation(path = path, version = resolvedVersion)
val runtimeCrateLocation = RuntimeCrateLocation(path = path, versions = crateVersionMap)
RuntimeConfig(
node.get().getStringMemberOrDefault("cratePrefix", "aws-smithy"),
runtimeCrateLocation = runtimeCrateLocation,
Expand All @@ -86,8 +95,15 @@ data class RuntimeConfig(

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

fun runtimeCrate(runtimeCrateName: String, optional: Boolean = false, scope: DependencyScope = DependencyScope.Compile): CargoDependency =
CargoDependency("$cratePrefix-$runtimeCrateName", runtimeCrateLocation.crateLocation(), optional = optional, scope = scope)
fun runtimeCrate(runtimeCrateName: String, optional: Boolean = false, scope: DependencyScope = DependencyScope.Compile): CargoDependency {
val crateName = "$cratePrefix-$runtimeCrateName"
return CargoDependency(
crateName,
runtimeCrateLocation.crateLocation(crateName),
optional = optional,
scope = scope,
)
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,177 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/

package software.amazon.smithy.rust.codegen.smithy

import io.kotest.matchers.shouldBe
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.rustlang.CratesIo
import software.amazon.smithy.rust.codegen.rustlang.DependencyLocation
import software.amazon.smithy.rust.codegen.rustlang.Local

class RuntimeTypesTest {
@ParameterizedTest
@MethodSource("runtimeConfigProvider")
fun `succeeded to parse runtime config`(
runtimeConfig: String,
expectedCrateLocation: RuntimeCrateLocation,
) {
val node = Node.parse(runtimeConfig)
val cfg = RuntimeConfig.fromNode(node.asObjectNode())
cfg.runtimeCrateLocation shouldBe expectedCrateLocation
}

@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
}

companion object {
@JvmStatic
private val defaultVersion = defaultRuntimeCrateVersion()

@JvmStatic
fun runtimeConfigProvider() = listOf(
Arguments.of(
"{}",
RuntimeCrateLocation(null, CrateVersionMap(mapOf())),
),
Arguments.of(
"""
{
"relativePath": "/path"
}
""",
RuntimeCrateLocation("/path", CrateVersionMap(mapOf())),
),
Arguments.of(
"""
{
"versions": {
"a": "1.0",
"b": "2.0"
}
}
""",
RuntimeCrateLocation(null, CrateVersionMap(mapOf("a" to "1.0", "b" to "2.0"))),
),
Arguments.of(
"""
{
"relativePath": "/path",
"versions": {
"a": "1.0",
"b": "2.0"
}
}
""",
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"),
),
)
}
}