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.

Sign up for GitHub

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

Jump to bottom

Add codegen version to generated package #1621

Merged
merged 8 commits into from
Aug 12, 2022
Merged

Add codegen version to generated package #1621

merged 8 commits into from
Aug 12, 2022

Conversation

unexge
Copy link
Contributor

@unexge unexge commented Aug 8, 2022 •
edited
Loading

Motivation and Context

Looking at the code that was generated by the "SmithyRs Code Generator" one cannot tell the version of the generator that was used.
Knowing the versions can help in post-deployment to track version specific bugs, and to potentially inform customers to regenerate in case of some vulnerability.

Closes #1612

Description

This PR adds smithy-rs's version to generated Cargo.tomls. This information later can be fetched via cargo metadata (or cargo_metadata crate).

Testing

It can be tested by generating any package, for example:

$ cd rust-runtime/aws-smithy-http-server/examples
$ make # generates both client and server for Pokemon service

$ rg -B1 -u codegen-version # version can be found in generated `Cargo.toml`s
pokemon-service-client/Cargo.toml
9-[package.metadata.smithy]
10:codegen-version = "0.47.0-0198d26096eb1af510ce24766c921ffc5e4c191e"

pokemon-service-server-sdk/Cargo.toml
9-[package.metadata.smithy]
10:codegen-version = "0.47.0-0198d26096eb1af510ce24766c921ffc5e4c191e"

# it can also be fetched via `cargo metadata`
$ cargo metadata --format-version 1 | jq -r '.packages[] | select(.name == "pokemon-service-server-sdk") | .metadata.smithy."codegen-version"'
0.47.0-0198d26096eb1af510ce24766c921ffc5e4c191e

Checklist

  • I have updated CHANGELOG.next.toml if I made changes to the smithy-rs codegen or runtime crates

@unexge unexge requested a review from a team as a code owner August 8, 2022 11:30
@github-actions
Copy link

github-actions bot commented Aug 8, 2022

A new generated diff is ready to view.

A new doc preview is ready to view.

82marbag
82marbag reviewed Aug 8, 2022
View reviewed changes
.../src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/CargoTomlGenerator.kt Outdated
@@ -74,3 +80,6 @@ class CargoTomlGenerator(
writer.writeWithNoFormatting(TomlWriter().write(cargoToml))
}
}

fun smithyCodegenVersion(): String =
try { Version.crateVersion() } catch (ex: Exception) { "unknown" }
Copy link
Contributor

Choose a reason for hiding this comment

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

Not needed because it throws in defaultRuntimeCrateVersion

@github-actions
Copy link

github-actions bot commented Aug 8, 2022

A new generated diff is ready to view.

A new doc preview is ready to view.

@unexge unexge force-pushed the unexge/add-codegen-version-to-generated-package branch from c4235df to 0198d26 Compare August 8, 2022 16:08
@github-actions
Copy link

github-actions bot commented Aug 8, 2022

A new generated diff is ready to view.

A new doc preview is ready to view.

@unexge unexge force-pushed the unexge/add-codegen-version-to-generated-package branch from fb907fc to 030cd19 Compare August 9, 2022 09:59
@github-actions
Copy link

github-actions bot commented Aug 9, 2022

A new generated diff is ready to view.

A new doc preview is ready to view.

@github-actions
Copy link

github-actions bot commented Aug 9, 2022

A new generated diff is ready to view.

A new doc preview is ready to view.

82marbag
82marbag reviewed Aug 9, 2022
View reviewed changes
codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/Version.kt Outdated
Comment on lines 14 to 18
val parts = content.split("-")
if (parts.size < 2) {
return ""
}
return parts.first()
Copy link
Contributor

Choose a reason for hiding this comment

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

If you want to go for "the file contains one line", this may not always work; some versions don't follow the format vx.y.z, like release-2022-08-04. In this case you should remove the last element, instead of picking the first one

@unexge unexge force-pushed the unexge/add-codegen-version-to-generated-package branch from 5cd9d69 to a6646ef Compare August 10, 2022 11:13
@github-actions
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

@github-actions
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

jdisanti
jdisanti approved these changes Aug 10, 2022
View reviewed changes
Copy link
Collaborator

@jdisanti jdisanti left a comment

Choose a reason for hiding this comment

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

Couple comments you may want to address, but nothing blocking from the client side.

codegen/src/test/kotlin/software/amazon/smithy/rust/codegen/smithy/VersionTest.kt Outdated
Comment on lines 53 to 62
Arguments.of(
"0.0.0",
"0.0.0",
"0.0.0",
),
Arguments.of(
"",
"",
"",
),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are these valid use-cases? It seems like it should fail in these cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah good point, I've updated test cases and the implementation.

.../src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/CargoTomlGenerator.kt Outdated
@@ -63,6 +64,11 @@ class CargoTomlGenerator(
"edition" to "2021",
"license" to settings.license,
"repository" to settings.moduleRepository,
"metadata" to listOfNotNull(
"smithy" to listOfNotNull(
"codegen-version" to smithyCodegenVersion(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: You can call Version.fullVersion() directly rather than wrapping it in a function.

@unexge unexge force-pushed the unexge/add-codegen-version-to-generated-package branch from 52cdb15 to fb56429 Compare August 11, 2022 10:38
@github-actions
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

@unexge unexge force-pushed the unexge/add-codegen-version-to-generated-package branch from fb56429 to 2671a36 Compare August 11, 2022 16:57
@github-actions
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

@unexge unexge force-pushed the unexge/add-codegen-version-to-generated-package branch from 2671a36 to 8b451ca Compare August 11, 2022 17:48
@github-actions
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

@unexge unexge force-pushed the unexge/add-codegen-version-to-generated-package branch from 8b451ca to a1a7f1f Compare August 12, 2022 09:44
@github-actions
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

@unexge unexge merged commit 610d963 into main Aug 12, 2022
@unexge unexge deleted the unexge/add-codegen-version-to-generated-package branch August 12, 2022 10:35
@unexge unexge mentioned this pull request Aug 16, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@82marbag 82marbag 82marbag approved these changes

@jdisanti jdisanti jdisanti approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Code generator version number should be emitted in generated crate
3 participants
@unexge @jdisanti @82marbag