chore: avoid multiple rustup installation#1571
Conversation
|
📝 WalkthroughWalkthroughIntroduces an internal Cargo version task to the Turborepo config and wires it as a dependency for wasm-related build tasks in package subprojects. Also adds an npm script to print Cargo’s version and updates the top-level Turbo schema reference. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
React Example#4416 Bundle Size — 237.07KiB (0%).2885958(current) vs cf46821 main#4413(baseline) Bundle metrics
|
| Current #4416 |
Baseline #4413 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
4 |
4 |
|
158 |
158 |
|
64 |
64 |
|
45.86% |
45.86% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #4416 |
Baseline #4413 |
|
|---|---|---|
145.76KiB |
145.76KiB |
|
91.31KiB |
91.31KiB |
Bundle analysis report Branch colinaaa:colin/0820/cargo-versio... Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#4409 Bundle Size — 366.68KiB (0%).2885958(current) vs cf46821 main#4406(baseline) Bundle metrics
Bundle size by type
|
| Current #4409 |
Baseline #4406 |
|
|---|---|---|
234.68KiB |
234.68KiB |
|
100.16KiB |
100.16KiB |
|
31.84KiB |
31.84KiB |
Bundle analysis report Branch colinaaa:colin/0820/cargo-versio... Project dashboard
Generated by RelativeCI Documentation Report issue
CodSpeed Performance ReportMerging #1571 will not alter performanceComparing Summary
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
package.json (1)
10-10: Make the cargo gate tolerant when Cargo isn’t installed (first pass on Linux/macOS).On runners where Rust isn’t preinstalled (your “first turbo build” before toolchain setup),
cargo --versionwill exit with ENOENT and fail the dependency, potentially breaking the initial pass even when a remote cache would otherwise skip Rust builds. Consider either making the script a no-op when Cargo is absent, or ensuring Rust is installed before this task runs on non-Windows matrices.Two options:
- CI-side: install/setup Rust before the first Turborepo pass on non-Windows runners.
- Script-side: make this resilient to missing Cargo.
Example (resilient script) — optional approach using Node to avoid shell portability issues on Windows:
- "_internal:cargo:version": "cargo --version", + "_internal:cargo:version": "node -e \"const {spawn}=require('node:child_process'); const bin=process.platform==='win32'?'cargo.exe':'cargo'; const p=spawn(bin,['--version'],{stdio:'inherit'}); p.on('error',()=>process.exit(0)); p.on('exit',code=>process.exit(code));\"",This still triggers a Rust toolchain install where present (due to rust-toolchain override), but won’t fail on hosts without Cargo.
Can you confirm whether your initial “turbo build” runs on Linux/macOS before Rust is installed? If yes, one of the above mitigations is recommended.
turbo.json (1)
2-2: Schema URL inconsistency across the repo.Root uses
https://turborepo.com/schema.json, while package turbo files still point tohttps://turbo.build/schema.json. Functionally they’re equivalent today, but it’s better to standardize to one URL across the repo to avoid confusion and IDE lint diffs.packages/react/transform/turbo.json (1)
10-12: LGTM: wasm build depends on Cargo version gate; consider adding it to other Rust-heavy tasks too.
build:wasmis correctly gated. Givenbuild:debugalso references Rust inputs, consider making it depend on//#_internal:cargo:versionto avoid any chance of parallel rustup when that task runs in CI.Apply within this file if you want to gate
build:debugas well:"build:debug": { - "inputs": ["src/**/*.rs", "build.rs", "Cargo.toml"], + "dependsOn": ["//#_internal:cargo:version"], + "inputs": ["src/**/*.rs", "build.rs", "Cargo.toml"], "outputs": ["index.d.ts", "index.cjs", "*.node"] },Also confirm that all packages which run Cargo have been updated to depend on the internal gate.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
package.json(1 hunks)packages/react/transform/turbo.json(1 hunks)packages/web-platform/web-style-transformer/turbo.json(1 hunks)turbo.json(2 hunks)
🔇 Additional comments (2)
turbo.json (1)
28-36: Good approach to serialize Rust toolchain initialization; verify cache semantics with GITHUB_SHA.
- Defining
//#_internal:cargo:versionwith inputs onrust-toolchain*and wiring dependents to it should ensure a single, serialized toolchain install viacargo --version.- Including
env: ["GITHUB_SHA"]forces this task to be a cache miss each commit, which is useful to trigger toolchain presence even when downstream tasks are fully cached. That said, it also means this task always executes locally on every commit.Action items:
- Confirm the always-execute behavior is intended. If it’s too chatty locally, you could scope this behavior to CI (e.g., set an env like
CARGO_VERSION_GATE_NO_CACHE=1in CI and list that env instead, or dropGITHUB_SHAand rely solely onrust-toolchain*inputs).- Ensure the corresponding root script exists (it does in package.json) and is resilient where Cargo isn’t present (see package.json comment).
No change required if the current behavior is desired; just verify the CI implications.
packages/web-platform/web-style-transformer/turbo.json (1)
7-9: No additional Rust-invoking tasks in web-style-transformer
I ran the audit across packages/web-platform/web-style-transformer/turbo.json and found onlybuild:wasm-bindgenreads Rust inputs. The new dependency on//#_internal:cargo:versionalready covers all Rust usages here—no further changes needed.
Summary by CodeRabbit
No user-facing behavior changes; these updates improve build reliability and consistency across environments.
This is used to avoid downloading Rust toolchain in parallel, which could cause error.
See: https://github.com/lynx-family/lynx-stack/actions/runs/17091970609/job/48467545202?pr=1569
Note
We would run
turbo buildfirst on CI. And if no cache hit forcargo buildtasks, we would setup Rust toolchain and re-run build again.But this is a little different with the Windows runner. Since there are
cargoandrustupon the Windows runner, we would start downloading Rust toolchain in parallel in the firstturbo build.Checklist