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.verisions` is an object that keys are runtime crate names (e.g. aws-smithy-http),
and values are their versions user specifying.
weihanglo marked this conversation as resolved.
Show resolved Hide resolved

If you previous set `version = "DEFAULT"`, then you're lucky.
The new behaviour automatically detects and injects the version for you by checking the version of the code generator.
You don't need to add any new configuration.
weihanglo marked this conversation as resolved.
Show resolved Hide resolved

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 crate are set with version 0.47.0 by default unless specified versions by crate name. For example,
weihanglo marked this conversation as resolved.
Show resolved Hide resolved

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

It implies that we're using aws-smithy-http 0.47.1 specifically. For other crates default to 0.47.0.
weihanglo marked this conversation as resolved.
Show resolved Hide resolved
"""
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,33 @@ 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?) {
data class RuntimeCrateLocation(val path: String?, val versions: CrateVersionMap) {
init {
check(path != null || version != null) {
"path ($path) or version ($version) must not be null"
check(path != null || versions.map.isNotEmpty()) {
"path ($path) or versions ($versions) must not be null or empty"
weihanglo marked this conversation as resolved.
Show resolved Hide resolved
}
}

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 crate we do not
// provide a detected version unless user explicitly sets via `DEFAULT` key.
weihanglo marked this conversation as resolved.
Show resolved Hide resolved
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 +61,14 @@ fun defaultRuntimeCrateVersion(): String {
}
}

/**
* A mapping for crate name to user specified version.
weihanglo marked this conversation as resolved.
Show resolved Hide resolved
*/
@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 +83,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 +101,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