fix: fix circular dependency for releases#40
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request fixes a circular dependency issue between aptos-sdk and aptos-sdk-macros crates that prevented them from being published to crates.io. The solution converts the aptos-sdk dev-dependency in aptos-sdk-macros to a path-only dependency (stripped from published crates), and updates the release workflow to handle this change appropriately.
Changes:
- Converted
aptos-sdk-macros's dev-dependency onaptos-sdkfrom workspace dependency to path-only dependency to break circular publish dependency - Added
--no-verifyflag to macros crate publishing since the path-only dev-dependency is unavailable during verification - Replaced simple 120-second sleep with intelligent polling that checks crates.io API to ensure macros crate is indexed before publishing SDK
- Added verification step to ensure macros dependency is available on crates.io when publishing SDK independently
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| crates/aptos-sdk-macros/Cargo.toml | Changed aptos-sdk dev-dependency from workspace to path-only to break circular dependency |
| .github/workflows/release.yml | Updated publish workflow to use --no-verify for macros, added intelligent crates.io polling, and dependency verification |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| aptos-sdk = { workspace = true } | ||
| # Path-only dep: stripped from published crate (breaks circular publish dependency). | ||
| # The macros crate only needs aptos-sdk for trybuild compile tests during local dev. | ||
| aptos-sdk = { path = "../aptos-sdk" } |
There was a problem hiding this comment.
aptos-sdk is now a path-only dev-dependency while this crate is publish = true. That path will be present in the published Cargo.toml, so consumers running cargo test on the crates.io tarball (and cargo publish verification builds) won’t be able to resolve ../aptos-sdk. Prefer specifying a version alongside the path (or using the workspace dependency form) so Cargo can rewrite the dependency for published artifacts while still using the local path in-workspace.
| aptos-sdk = { path = "../aptos-sdk" } | |
| aptos-sdk = { version = "0.2.0", path = "../aptos-sdk" } |
| # Path-only dep: stripped from published crate (breaks circular publish dependency). | ||
| # The macros crate only needs aptos-sdk for trybuild compile tests during local dev. |
There was a problem hiding this comment.
The comment claims this path-only dev-dependency is "stripped from published crate", but Cargo will still include dev-dependencies in the published manifest. Consider rewording to avoid implying it’s removed from the published Cargo.toml, and explain that --no-verify is required because the packaged crate can’t resolve workspace-relative paths during publish verification.
| # Path-only dep: stripped from published crate (breaks circular publish dependency). | |
| # The macros crate only needs aptos-sdk for trybuild compile tests during local dev. | |
| # Path-only dev-dependency used for trybuild compile tests during local development. | |
| # Cargo still includes this in the published Cargo.toml, but the packaged crate cannot | |
| # resolve this workspace-relative path during `cargo publish --verify`, so publishing | |
| # this crate requires using `cargo publish --no-verify`. |
| - name: Publish aptos-sdk-macros (dry run) | ||
| if: github.event_name == 'workflow_dispatch' && inputs.dry_run == true | ||
| run: cargo publish -p aptos-sdk-macros --locked --dry-run | ||
| run: cargo publish -p aptos-sdk-macros --no-verify --dry-run | ||
|
|
||
| - name: Publish aptos-sdk-macros | ||
| if: github.event_name == 'push' || (github.event_name == 'workflow_dispatch' && inputs.dry_run != true) | ||
| run: cargo publish -p aptos-sdk-macros --locked | ||
| run: cargo publish -p aptos-sdk-macros --no-verify |
There was a problem hiding this comment.
The publish commands for aptos-sdk-macros dropped --locked. For release automation, keeping --locked helps ensure the publish/dry-run uses the committed lockfile and avoids accidental dependency resolution drift compared to the verified build.
The previous grep pattern matched the workspace members array entry before the dependency definition line, causing the sed substitution to output the wrong value.
gregnazario
force-pushed
the
fix-rust-release
branch
from
6a4877b to
c5fe682
Compare
No description provided.