RFC: Independent Crate Versioning #1194
Conversation
|
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
|
| - `patch`: Backwards compatible bug fixes | ||
| - `pre`: Pre-release version tag (omitted for normal releases) | ||
|
|
||
| In the new versioning strategy, the `minor` version number will no longer be coordinated across |
There was a problem hiding this comment.
is this the case for all three phases?
| - Bug fixes that do not break backwards compatibility | ||
| - Model updates that _only_ have documentation changes | ||
|
|
||
| During phase 3, bumps to the `major` version must be coordinated across all SDK and runtime crates. |
There was a problem hiding this comment.
is this a requirement of this proposal? I could definitely imagine wanting to major version bump an individual service that made a breaking change, eg.
There was a problem hiding this comment.
I suppose this isn't a hard requirement, but we would be walking away from the ability to make a large refactor and coordinate it under a major version release like the other SDKs do with their V2/V3 releases.
|
|
||
| ### Release Identification | ||
|
|
||
| Since there will no longer be one SDK "version", release tags will be dates in `YYYY-MM-DD` format |
There was a problem hiding this comment.
this refers to tags on GitHub, not cargo versions, right?
| # ... | ||
| ``` | ||
|
|
||
| The auto-sync tool is responsible for maintaining this file. When it generates a new SDK, it will take |
There was a problem hiding this comment.
will there be some way to override / hint?
| affect only documentation, but then one model update that affects an API, then as a whole they will be | ||
| considered as affecting an API and require a `minor` version bump. | ||
|
|
||
| The previously released version number will be retrieved from crates.io using its API. The [smithy-rs] version |
There was a problem hiding this comment.
conceivably we could track the previously deployed version in versions.toml right?
There was a problem hiding this comment.
We could definitely track it in the versions.toml, but I think this has a greater chance for error than retrieving it from the crates.io index.
| [aws-smithy-types] | ||
| version = "0.50.1" |
There was a problem hiding this comment.
the runtime crate versions here are just for consistency right? they aren't actually required for the system to function?
We could consider tracking published = ... and next = ... in this file which removes the crates.io API dependency (or would just provide a spot to verify)
There was a problem hiding this comment.
The idea was to have all the version numbers for a given release tag centralized in one place, so that's why the runtime crate versions are in here. Nothing else is requiring them to be in this file.
|
|
||
|  | ||
|
|
||
| - __A: Crate has changed?__ The crate's source files and manifest will be hashed for the previous version |
There was a problem hiding this comment.
whats manifest here? Cargo.toml?
There was a problem hiding this comment.
Yeah. Manifest is the name for a Cargo.toml file according to https://doc.rust-lang.org/cargo/reference/manifest.html:
The Cargo.toml file for each package is called its manifest.
| - [ ] Update CI to verify no older runtime crates are used. For example, if `aws-smithy-client` is bumped to | ||
| `0.50.0`, then verify no crates (generated or runtime) depend on `0.49.0` or lower. | ||
|
|
||
| **Estimate:** 2-4 dev weeks |
| The publisher tool will be updated to read the `versions.toml` to yank all versions published in a release. | ||
| This process will look as follows: | ||
| 1. Take a path to a local clone of the [aws-sdk-rust] repository | ||
| 2. Confirm the branch is `main` and that HEAD is tagged as a release | ||
| 3. Read `versions.toml` and print out summary of crates to yank | ||
| 4. Confirm with user before proceeding | ||
| 5. Yank crates |
There was a problem hiding this comment.
I'm a little confused about this—I think I'm missing how versions.toml tells you which crates were released and which were not given the current schema—do you have to diff the files?
There was a problem hiding this comment.
Let's say we have a bad release at tag bad that we want to yank. The versions.toml file at that tag will have all the crate version numbers associated with that release, but if the publish failed half way through, then some of those might be published to crates.io while others might not be. But that's OK since the yank tool can just check that the version is actually published using the crates.io API before yanking it.
That said, I'm now confused by (2) above. I think I'll revise this to make sure you're on a release tag rather than on main.
|
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
|
jdisanti
changed the title
|
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
|
Rust Wrk benchmark report:Duration: 90 sec, Connections: 32, Threads: 2
|
Velfi
left a comment
Velfi
left a comment
There was a problem hiding this comment.
i had a few comments/questions/suggestions
| @@ -0,0 +1,207 @@ | |||
| RFC: Unsynchronized Crate Versioning | |||
There was a problem hiding this comment.
Consider "Independent Crate Versioning"
|
|
||
| ### Release Identification | ||
|
|
||
| Since there will no longer be one SDK "version", release tags will be dates in `YYYY-MM-DD` format |
There was a problem hiding this comment.
What if we release 2+ times in one day?
There was a problem hiding this comment.
SDK release automation is limited to one release per day, but there is the question of what this would look like in the event of a manual release taking place in addition to an automatic release. I don't really have a good answer for this currently.
| Code generated crates will have their `minor` version bumped when the version of [smithy-rs] used to generate | ||
| them changes, or when model updates with API changes are made. Three pieces of information are required to | ||
| handle this process: the previously released version number, the [smithy-rs] version used to generate the code, | ||
| and the level of model updates being applied. For this last one, if there are multiple model updates that | ||
| affect only documentation, but then one model update that affects an API, then as a whole they will be | ||
| considered as affecting an API and require a `minor` version bump. |
There was a problem hiding this comment.
paraphrasing this to make sure I understand
"if we update 2+ models in a release, and at least 1 of those models has API changes, all updated models will receive a
minorversion bump"
Is that right?
There was a problem hiding this comment.
So imagine we have the following updates to the S3 model, and the S3 version was previously 1.2.3:
- Documentation
- Documentation
- API change
- API change
The new version in this case, will be exactly one minor version bump: 1.3.0
| The previously released version number will be retrieved from crates.io using its API. The [smithy-rs] version | ||
| used during code generation will become a build artifact that is saved to `versions.toml` in [aws-sdk-rust]. | ||
| During phase 1, the tooling required to know if a model is a documentation-only change will not be available, | ||
| so all model changes will result in a `minor` version bump during this phase. |
There was a problem hiding this comment.
Are we ever going to run into a situation where we start releasing a version and then it chokes on the crates.io upload because of some problem with our code? If yes, would the versioning get messed up?
There was a problem hiding this comment.
I think in that case, we would make a code fix to the publisher tool in smithy-rs, and then manually run the publish to fix the partial release. Or, otherwise, fix the publisher tool, yank what got published, and let the next automated release fix things.
| a build step to replace those with a consistent `major.minor` will be removed. These runtime crates will begin | ||
| having their actual next version number in the Cargo.toml file in smithy-rs. |
| - __A: Crate has changed?__ The crate's source files and manifest will be hashed for the previous version | ||
| and the next version. If these hashes match, then the crate is considered unchanged. |
There was a problem hiding this comment.
Cargo has something like this for incremental compilation right? It'd be kinda neat if we could use that functionality to do this.
There was a problem hiding this comment.
It would be neat, but I think it would require building a tool that utilizes the compiler internals, and introduce the maintenance burden that comes with that kind of tooling.
| number was bumped. | ||
| - __E: Semverver passes?__ Runs [rust-semverver] against the old and new versions of the crate. | ||
| - If semverver fails to run (for example, if it needs to be updated to the latest nightly to succeed), | ||
| then fail CI saying that either semverver needs to be updated, or that a minor version bump is required. |
There was a problem hiding this comment.
| then fail CI saying that either semverver needs to be updated, or that a minor version bump is required. | |
| then fail CI saying that semverver needs maintenance. |
There was a problem hiding this comment.
The point about a minor version bump being an option should stay, but I like the suggestion of "needs maintenance" over "to be updated".
| When stabilizing to 1.x, the version process will stay the same, but with the introduction of the greater than zero | ||
| major version number, the minor version bumps caused by version bumping runtime crates, updating models, or changing |
There was a problem hiding this comment.
| When stabilizing to 1.x, the version process will stay the same, but with the introduction of the greater than zero | |
| major version number, the minor version bumps caused by version bumping runtime crates, updating models, or changing | |
| When stabilizing to 1.x, the version process will stay the same, but with the minor version bumps caused by version bumping runtime crates, updating models, or changing |
The current wording feels a bit redundant but I don't feel super strongly about this
jdisanti
changed the title
|
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
|
Rust Wrk benchmark report:Duration: 90 sec, Connections: 32, Threads: 2
|
|
A new generated diff is ready to view.
A new doc preview is ready to view. Rust Wrk benchmark report:Duration: 90 sec, Connections: 32, Threads: 2
|
3 participants
Motivation and Context
This adds an RFC describing how we should approach versioning and publishing to crates.io long-term. It incorporates feedback from a previous RFC by breaking the requirement that
major.minorversions should be the same across all crates in the SDK.Rendered
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.