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
6 changes: 6 additions & 0 deletions CHANGELOG.next.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,9 @@ 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"
references = ["smithy-rs#1416"]
weihanglo marked this conversation as resolved.
Show resolved Hide resolved
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