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

Fix CI step for cargo semver checks #4033

Merged
merged 5 commits into from
Feb 25, 2025
Merged

Conversation

ysaito1001
Copy link
Contributor

@ysaito1001 ysaito1001 commented Feb 22, 2025

Motivation and Context

Fixes CI step for running cargo semver checks

Description

The said CI step has been failing for the past three months. The most recent failure observed in CI looks like the following, since we upgraded the version of cargo-semver-checks to 0.36

error: package `aws-runtime` is ambiguous: it is defined by in multiple manifests within the root path /home/build/workspace/smithy-rs/target/semver-checks/git-base/be980cd7049d9acdb38f68dd11c465b3a242733a

defined in:
  /home/build/workspace/smithy-rs/target/semver-checks/git-base/be980cd7049d9acdb38f68dd11c465b3a242733a/aws/rust-runtime/aws-runtime/Cargo.toml
  /home/build/workspace/smithy-rs/target/semver-checks/git-base/be980cd7049d9acdb38f68dd11c465b3a242733a/tmp-codegen-diff/aws-sdk/sdk/aws-runtime/Cargo.toml

Referring to the diagram in a previous relevant PR, we see that the tmp-codegen-diff directory is used as the output directory by diff_lib.py. However, the current version of cargo semver checks no longer supports duplicated crates within the same root directory (PR).

This requires a solution where we maintain a single source of crate layout to validate against cargo-semver-checks under smithy-rs. To address this, this PR implements the following approach:

  • (Bonus point) Reverts the changes made in the previous relevant PR.
  • Updates semver-checks.py to stop using checkout_commit_and_generate in diff_lib.py, which automatically places both the runtime crates and the generated SDK crates into tmp-codegen-diff, leading to conflicts with the crates in rust-runtime and in aws/rust-runtime.
  • When semver-checks.py creates the base and current branches, we remove runtime crates from both rust-runtime and aws/rust-runtime to uniquify the runtime crates in the aws/sdk/build/aws-sdk/sdk directory. Note that the removal of crates only occurs in the base and current branches and does not impact the main branch.
  • Moves the aws/sdk/build/aws-sdk directory to the root of smithy-rs for a reason explained in dd021d1.

Testing

  • CI passed cargo semver checks
  • Removed this enum variant intentionally to cause an API breakage, the step correctly detected it
--- failure enum_variant_missing: pub enum variant removed or renamed ---

Description:
A publicly-visible enum has at least one variant that is no longer available under its prior name. It may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.36.0/src/lints/enum_variant_missing.ron

Failed in:
  variant BusinessMetric::GzipRequestCompression, previously in file /Users/awsaito/src/smithy-rs/target/semver-checks/git-base/d87d9ee34a13da0ad4c0cdcbfa2c74b7f98d278c/aws-sdk/sdk/aws-runtime/src/user_agent/metrics.rs:108

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Copy link

A new generated diff is ready to view.

  • No codegen difference in the AWS SDK
  • No codegen difference in the Client Test
  • No codegen difference in the Server Test
  • No codegen difference in the Server Test Python
  • No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

@ysaito1001 ysaito1001 marked this pull request as ready for review February 24, 2025 23:47
@ysaito1001 ysaito1001 requested review from a team as code owners February 24, 2025 23:47
Copy link
Contributor

@landonxjames landonxjames left a comment

Choose a reason for hiding this comment

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

Nice! Excited to get back to all green CI ✅

@ysaito1001 ysaito1001 enabled auto-merge February 25, 2025 21:24
Copy link

A new generated diff is ready to view.

  • No codegen difference in the AWS SDK
  • No codegen difference in the Client Test
  • No codegen difference in the Server Test
  • No codegen difference in the Server Test Python
  • No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

@ysaito1001 ysaito1001 added this pull request to the merge queue Feb 25, 2025
Merged via the queue into main with commit 357415f Feb 25, 2025
45 checks passed
@ysaito1001 ysaito1001 deleted the ysaito/fix-cargo-semver-checks branch February 25, 2025 22:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants