feat(toolkit-macros): compile-time gear dependency linking via re-exports#4146
feat(toolkit-macros): compile-time gear dependency linking via re-exports#4146maurolacy wants to merge 6 commits into
Conversation
Signed-off-by: Mauro Lacy <mauro.lacy@acronis.com>
Signed-off-by: Mauro Lacy <mauro.lacy@acronis.com>
Signed-off-by: Mauro Lacy <mauro.lacy@acronis.com>
Signed-off-by: Mauro Lacy <mauro.lacy@acronis.com>
Signed-off-by: Mauro Lacy <mauro.lacy@acronis.com>
|
Warning Review limit reached
More reviews will be available in 35 minutes. Learn how PR review limits work. To continue reviewing without waiting, enable usage-based billing in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe workspace adds eight new system crate entries, and multiple gears, plugins, and examples add workspace or direct dependencies on the new resolver, registry, and gateway crates. The ChangesGear dependency wiring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
libs/toolkit-macros/src/lib.rs (1)
771-787: 🩺 Stability & Availability | 🔵 TrivialAnchor the re-export path with a leading
::to avoid module-scope shadowing.The generated
pub use#crate_identas#alias_ident;expands at the call site of the#[gear]macro. If the macro is invoked within a submodule that defines a local item (e.g.,mod crate_nameorconst crate_name), the unqualifieduseresolution may resolve to that local item instead of the intended extern crate. Using::#crate_ident`` forces resolution against the extern prelude, ensuring the re-export remains stable regardless of local scope.♻️ Proposed change
quote! { #[cfg(not(test))] #[doc(hidden)] - pub use `#crate_ident` as `#alias_ident`; + pub use ::`#crate_ident` as `#alias_ident`; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libs/toolkit-macros/src/lib.rs` around lines 771 - 787, The generated re-export in the macro expansion is currently using an unqualified path that can be shadowed by local module items. Update the re-export inside the dep reexports generation in lib.rs so the pub use in the quote! block anchors the crate path with a leading ::, using the existing crate_ident and alias_ident symbols, to ensure the extern crate is always resolved from the extern prelude even when #[gear] is invoked inside a submodule with a conflicting local name.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@libs/toolkit-macros/src/lib.rs`:
- Around line 771-787: The generated re-export in the macro expansion is
currently using an unqualified path that can be shadowed by local module items.
Update the re-export inside the dep reexports generation in lib.rs so the pub
use in the quote! block anchors the crate path with a leading ::, using the
existing crate_ident and alias_ident symbols, to ensure the extern crate is
always resolved from the extern prelude even when #[gear] is invoked inside a
submodule with a conflicting local name.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ea1176a7-fe6b-4137-a78c-d7bf844fa564
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (25)
Cargo.tomlexamples/oop-gears/calculator-gateway/calculator-gateway/Cargo.tomlexamples/toolkit/users-info/users-info/Cargo.tomlgears/credstore/credstore/Cargo.tomlgears/credstore/plugins/static-credstore-plugin/Cargo.tomlgears/mini-chat/mini-chat/Cargo.tomlgears/simple-user-settings/simple-user-settings/Cargo.tomlgears/system/account-management/account-management/Cargo.tomlgears/system/account-management/plugins/static-idp-plugin/Cargo.tomlgears/system/api-gateway/Cargo.tomlgears/system/authn-resolver/authn-resolver/Cargo.tomlgears/system/authn-resolver/plugins/oidc-authn-plugin/Cargo.tomlgears/system/authn-resolver/plugins/static-authn-plugin/Cargo.tomlgears/system/authz-resolver/authz-resolver/Cargo.tomlgears/system/authz-resolver/plugins/static-authz-plugin/Cargo.tomlgears/system/authz-resolver/plugins/tr-authz-plugin/Cargo.tomlgears/system/oagw/oagw/Cargo.tomlgears/system/resource-group/resource-group/Cargo.tomlgears/system/tenant-resolver/plugins/rg-tr-plugin/Cargo.tomlgears/system/tenant-resolver/plugins/single-tenant-tr-plugin/Cargo.tomlgears/system/tenant-resolver/plugins/static-tr-plugin/Cargo.tomlgears/system/tenant-resolver/tenant-resolver/Cargo.tomlgears/system/usage-collector/plugins/noop-usage-collector-plugin/Cargo.tomlgears/system/usage-collector/usage-collector/Cargo.tomllibs/toolkit-macros/src/lib.rs
Signed-off-by: Mauro Lacy <mauro.lacy@acronis.com>
Problem
The
depsfield in#[toolkit::gear(deps = [...])]declares runtime lifecycledependencies between gears. At startup,
build_dependency_graph()validates thatevery declared dep exists in the
inventory-based registry. However,inventoryrelies on linker-section tricks: only crates explicitly referenced with
use X as _;in the generated
main.rssurvive the linker. Transitive Cargo dependencies gettheir
inventory::submit!registrations silently stripped.This forces users to flatten the entire gear dependency tree in
Gears.toml. Forexample, listing
api-gatewayis not enough. You must also manually listgrpc-hub,authn-resolver,types-registry, and every other transitive dep.Forgetting one causes a runtime
UnknownDependencyerror at startup, with nocompile-time signal that anything is wrong.
Solution
The
#[toolkit::gear]macro now generatespub usere-exports for each declareddependency:
#[toolkit::gear(name = "api-gateway", deps = ["grpc-hub", "authn-resolver"])]expands to (among other things):
This forces the linker to keep the dependency gear crates (and their
inventory::submit!registrations) alive when a gear is pulled in transitively.The re-exports chain: if
api-gatewayre-exportsauthn-resolver, andauthn-resolverre-exportstypes-registry, then pulling inapi-gatewayaloneis sufficient to register the entire transitive dependency tree.
The re-exports are gated behind
#[cfg(not(test))], so that test builds (wheregears are often defined in the same compilation unit) don't require phantom
external crate dependencies.
Trade-off: Cargo.toml dependencies
For the
pub useto compile, each gear'sCargo.tomlmust list its dep gearcrates as Cargo dependencies (not just SDKs). This is a new requirement, but
forgetting a dependency is now a compile-time error (
no external crate),not a silent runtime failure. The SDK dependencies remain necessary for the
actual API types used in gear implementations.
Naming convention
This change relies on a strict, bidirectional naming convention:
api-gatewaycf-gears-api-gatewayapi_gatewaytypes-registrycf-gears-types-registrytypes_registryThe macro derives the lib name from the dep's gear name by replacing
-with_.This convention is already followed by all gear crates in the repo (24/27 gears;
the 3 exceptions are internal plugins in
mini-chatwhich are same-crate andunaffected).
Changes
libs/toolkit-macros/src/lib.rs: Generatepub usere-exports for eachentry in
deps = [...], gated behind#[cfg(not(test))].Cargo.toml(root): Added 8 workspace-level dependency entries for gearcrates that were previously only available as SDKs.
Cargo.tomlfiles: Added the full gear crate as a dependency(via
{ workspace = true }) for each declared dep.Verification
cargo build: Entire workspace compiles cleanly.cargo test --workspace: All test suites pass.What this enables
With this change,
cargo gearsand future tooling no longer needs to resolveand flatten the transitive gear dependency tree. Users can list only the gears
they directly use in
Gears.toml, and the macro-generated re-exports ensure thefull dependency chain is linked.
Summary by CodeRabbit
New Features
Bug Fixes