ci: use self-hosted rust cache#1607
Conversation
📝 WalkthroughWalkthroughAdds a local composite GitHub Action at Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
✨ 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! |
Web Explorer#4605 Bundle Size — 368.23KiB (~+0.01%).bd453d9(current) vs 1952fc1 main#4586(baseline) Bundle metrics
Bundle size by type
Bundle analysis report Branch colinaaa:colin/0826/rust-cache Project dashboard Generated by RelativeCI Documentation Report issue |
React Example#4612 Bundle Size — 237.06KiB (0%).bd453d9(current) vs 1952fc1 main#4593(baseline) Bundle metrics
|
| Current #4612 |
Baseline #4593 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
4 |
4 |
|
158 |
158 |
|
64 |
64 |
|
45.83% |
45.83% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #4612 |
Baseline #4593 |
|
|---|---|---|
145.76KiB |
145.76KiB |
|
91.3KiB |
91.3KiB |
Bundle analysis report Branch colinaaa:colin/0826/rust-cache Project dashboard
Generated by RelativeCI Documentation Report issue
CodSpeed Performance ReportMerging #1607 will not alter performanceComparing Summary
Footnotes |
|
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.github/actions/rustup/action.yml (1)
25-29: Harden writes to GITHUB_PATH/OUTPUT and sanitize toolchain value (security).Avoid injection into env files; also ensure the toolchain string is trimmed and safely emitted.
Apply:
- run: | # zizmor: ignore[github-env] This action would only run on pull_request + run: | # zizmor: ignore[github-env] This action runs on PR; harden env writes. if ! command -v rustup &> /dev/null ; then curl --proto '=https' --tlsv1.2 --retry 10 --retry-connrefused -fsSL "https://sh.rustup.rs" | sh -s -- --default-toolchain none -y - echo "$HOME/.cargo/bin" >> $GITHUB_PATH + echo "$HOME/.cargo/bin" >> "$GITHUB_PATH" fi @@ - run: | - toolchain=$(cat rust-toolchain) - echo "toolchain=${toolchain}" >> $GITHUB_OUTPUT + run: | + if [[ -f rust-toolchain ]]; then + toolchain="$(head -n1 rust-toolchain | tr -d '\r')" + elif [[ -f rust-toolchain.toml ]]; then + toolchain="$(grep -E '^\s*channel\s*=' rust-toolchain.toml | head -n1 | sed -E 's/[^"]*"([^"]+)".*/\1/')" + else + toolchain="stable" + fi + { + echo "toolchain<<EOF" + echo "$toolchain" + echo "EOF" + } >> "$GITHUB_OUTPUT"Also applies to: 31-37
🧹 Nitpick comments (6)
.github/actions/rustup/action.yml (4)
38-45: Restore cache on all branches; use OS-specific toolchains path.Restoring only on main forfeits perf on PRs; also ~/.rustup path won’t resolve in cache inputs on Windows.
Apply:
- - name: Restore rustup cache - id: restore - if: ${{ inputs.save-if == 'true' }} + - name: Restore rustup cache (Unix) + id: restore + if: runner.os != 'Windows' uses: lynx-infra/cache/restore@5c6160a6a4c7fca80a2f3057bb9dfc9513fcb732 with: path: ~/.rustup/toolchains key: rustup-cache-v3-${{ runner.os }}-${{ steps.get-toolchain.outputs.toolchain }} + + - name: Restore rustup cache (Windows) + if: runner.os == 'Windows' + uses: lynx-infra/cache/restore@5c6160a6a4c7fca80a2f3057bb9dfc9513fcb732 + with: + path: ${{ env.USERPROFILE }}\.rustup\toolchains + key: rustup-cache-v3-${{ runner.os }}-${{ steps.get-toolchain.outputs.toolchain }} @@ - - name: Save rustup cache - uses: lynx-infra/cache/save@5c6160a6a4c7fca80a2f3057bb9dfc9513fcb732 + - name: Save rustup cache (Unix) + if: ${{ runner.os != 'Windows' && inputs.save-if == 'true' && steps.restore.outputs.cache-hit != 'true' }} + uses: lynx-infra/cache/save@5c6160a6a4c7fca80a2f3057bb9dfc9513fcb732 with: path: ~/.rustup/toolchains key: rustup-cache-v3-${{ runner.os }}-${{ steps.get-toolchain.outputs.toolchain }} + + - name: Save rustup cache (Windows) + if: ${{ runner.os == 'Windows' && inputs.save-if == 'true' && steps.restore.outputs.cache-hit != 'true' }} + uses: lynx-infra/cache/save@5c6160a6a4c7fca80a2f3057bb9dfc9513fcb732 + with: + path: ${{ env.USERPROFILE }}\.rustup\toolchains + key: rustup-cache-v3-${{ runner.os }}-${{ steps.get-toolchain.outputs.toolchain }}Also applies to: 60-66
46-59: Set minimal profile; gate wasm target via input to avoid unnecessary installs.Install is faster with minimal profile; wasm target should be optional.
Apply:
runs: using: composite steps: @@ - name: Install shell: bash env: toolchain: ${{ steps.get-toolchain.outputs.toolchain }} run: | - rustup toolchain install $toolchain \ + rustup set profile minimal + rustup toolchain install $toolchain \ -c rustc \ -c cargo \ -c rust-std \ -c clippy \ -c rustfmt - rustup target add wasm32-unknown-unknown + if [[ "${{ inputs.add-wasm-target || 'false' }}" == 'true' ]]; then + rustup target add wasm32-unknown-unknown + fiAnd add a new input:
inputs: key: @@ save-if: description: Should save toolchain cache or not default: "false" required: false + add-wasm-target: + description: Add wasm32-unknown-unknown target + default: "false" + required: false
67-73: Avoid cross-OS/toolchain cargo cache collisions.Include OS and toolchain in the cache key.
Apply:
- - name: Cargo cache + - name: Cargo cache uses: stormslowly/rust-cache@8269079380bc35105b3ed8b0f1f7c9557c6dec93 # v0.0.2 with: - prefix-key: "RCache-L-5" - shared-key: ${{ inputs.key }} + prefix-key: "RCache-L-6" + shared-key: ${{ inputs.key }}-${{ runner.os }}-${{ steps.get-toolchain.outputs.toolchain }} save-if: ${{ inputs.save-if }} fullmatch-only: "true"Please confirm stormslowly/rust-cache doesn’t itself add OS/toolchain; if it does, keeping them here is still harmless and explicit.
1-4: Fix header/comment accuracy and typos.
- “Swatinem/rust-cache” → “stormslowly/rust-cache”.
- “possbile” → “possible”; “some of components” → “some components”.
- The “only run on pull_request” note is misleading; this action also runs on push. Remove or reword.
Apply:
-# This action installs the minimal Rust profile and configures Swatinem/rust-cache. +# This action installs a minimal Rust toolchain and configures stormslowly/rust-cache. @@ -# It is needed to install as few Rust components as possbile because -# it takes a few minutes to install some of components on Windows and Mac, especially rust-doc. +# Install as few Rust components as possible because installing extra components (e.g., rust-doc) adds minutes on macOS/Windows.Also applies to: 25-25, 68-68
.github/workflows/rust.yml (1)
72-76: Unify toolchain setup across jobs via the local action.Use the same composite action for rustfmt and clippy to keep toolchain consistent and optionally benefit from cache.
Apply:
- - uses: actions-rust-lang/setup-rust-toolchain@fb51252c7ba57d633bc668f941da052e410add48 # v1 - with: - components: rustfmt - cache: false + - uses: ./.github/actions/rustup + with: + key: lint-fmt + save-if: ${{ github.ref_name == 'main' }} + add-wasm-target: "false" @@ - - uses: actions-rust-lang/setup-rust-toolchain@fb51252c7ba57d633bc668f941da052e410add48 # v1 - with: - components: clippy - cache: false + - uses: ./.github/actions/rustup + with: + key: lint-clippy + save-if: ${{ github.ref_name == 'main' }} + add-wasm-target: "false"Also applies to: 88-92
.github/workflows/workflow-build.yml (1)
86-99: Turbo cache keys look solid. Consider adding lockfiles to strengthen invalidation.Optional: include Cargo.lock, pnpm-lock.yaml to avoid stale caches when deps change without touching Rust sources.
Example:
- key: turbo-v4-${{ runner.os }}-${{ hashFiles('**/packages/**/src/**/*.rs') }}-${{ github.sha }} + key: turbo-v4-${{ runner.os }}-${{ hashFiles('**/packages/**/src/**/*.rs', '**/Cargo.lock', '**/pnpm-lock.yaml') }}-${{ github.sha }} @@ - turbo-v4-${{ runner.os }}-${{ hashFiles('**/packages/**/src/**/*.rs') }}-${{ needs.get-merge-base.outputs.merge-base }} + turbo-v4-${{ runner.os }}-${{ hashFiles('**/packages/**/src/**/*.rs', '**/Cargo.lock', '**/pnpm-lock.yaml') }}-${{ needs.get-merge-base.outputs.merge-base }} - turbo-v4-${{ runner.os }}-${{ hashFiles('**/packages/**/src/**/*.rs') }}-${{ github.event.pull_request.base.sha || github.event.merge_group.base_sha || github.event.push.before }} + turbo-v4-${{ runner.os }}-${{ hashFiles('**/packages/**/src/**/*.rs', '**/Cargo.lock', '**/pnpm-lock.yaml') }}-${{ github.event.pull_request.base.sha || github.event.merge_group.base_sha || github.event.push.before }} - turbo-v4-${{ runner.os }}-${{ hashFiles('**/packages/**/src/**/*.rs') }}- + turbo-v4-${{ runner.os }}-${{ hashFiles('**/packages/**/src/**/*.rs', '**/Cargo.lock', '**/pnpm-lock.yaml') }}-
📜 Review details
Configuration used: Path: .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 (3)
.github/actions/rustup/action.yml(1 hunks).github/workflows/rust.yml(1 hunks).github/workflows/workflow-build.yml(1 hunks)
🔇 Additional comments (2)
.github/workflows/rust.yml (2)
39-43: Good switch to local action; branch-guarded saving looks right.
39-43: Cache keys include OS and toolchain by defaultAccording to the action’s documentation, the cache key is automatically composed of the
rustcrelease / host / hash—where the “host” component encodes the target triple (e.g.x86_64-unknown-linux-gnu) and the “release” component encodes the Rust toolchain version—alongside other values (job_id, environment variables, lockfile hashes, etc.). This ensures caches cannot be shared or collide across different OS runners or Rust toolchains. (github.com)
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
.github/actions/rustup/action.yml (1)
22-37: Add a Windows-specific rustup bootstrap and gate the bash installer to non-Windows.The bash-based installer can fail or be slow on Windows. Add a PowerShell step for Windows and limit the bash step to non-Windows. This also addresses the earlier suggestion.
Apply:
- - name: Install rustup, if needed - shell: bash + - name: Install rustup, if needed (non-Windows) + if: runner.os != 'Windows' + shell: bash run: | # zizmor: ignore[github-env] This action would only run on pull_request - if ! command -v rustup &> /dev/null ; then - curl --proto '=https' --tlsv1.2 --retry 10 --retry-connrefused -fsSL "https://sh.rustup.rs" | sh -s -- --default-toolchain none -y - - # Resolve the correct CARGO_HOME path depending on OS - if [[ "$RUNNER_OS" == "Windows" ]]; then - echo "${CARGO_HOME:-$USERPROFILE/.cargo}/bin" | sed 's|/|\\|g' >> $GITHUB_PATH - else - echo "${CARGO_HOME:-$HOME/.cargo}/bin" >> $GITHUB_PATH - fi - fi - env: - RUNNER_OS: "${{ runner.os }}" + set -euo pipefail + if ! command -v rustup &> /dev/null ; then + curl --proto '=https' --tlsv1.2 --retry 10 --retry-connrefused -fsSL "https://sh.rustup.rs" | sh -s -- --default-toolchain none -y + # Ensure cargo bin dir is on PATH + printf '%s\n' "${CARGO_HOME:-$HOME/.cargo}/bin" >> "$GITHUB_PATH" + fi + + - name: Install rustup on Windows, if needed + if: runner.os == 'Windows' + shell: pwsh + run: | + if (-not (Get-Command rustup -ErrorAction SilentlyContinue)) { + Invoke-WebRequest -UseBasicParsing https://win.rustup.rs/x86_64 -OutFile rustup-init.exe + .\rustup-init.exe -y --default-toolchain none + '$env:USERPROFILE\.cargo\bin' | Out-File -FilePath $env:GITHUB_PATH -Encoding utf8 -Append + }
🧹 Nitpick comments (2)
.github/actions/rustup/action.yml (2)
1-5: Fix typo and align header comment with actual cache action used.
- possbile -> possible
- Header mentions Swatinem/rust-cache but the file uses stormslowly/rust-cache.
Apply:
-# This action installs the minimal Rust profile and configures Swatinem/rust-cache. +# This action installs a minimal Rust toolchain and configures a rust-cache action. @@ -# It is needed to install as few Rust components as possbile because +# It is needed to install as few Rust components as possible because
53-66: Consider setting override or default toolchain for workspace.If the repo lacks rust-toolchain[.toml], cargo may still use a different default. Optionally set an override.
Example:
rustup toolchain install "$toolchain" \ -c rustc \ -c cargo \ -c rust-std \ -c clippy \ -c rustfmt + # Optional: ensure this repo uses the intended toolchain + # rustup override set "$toolchain"
📜 Review details
Configuration used: Path: .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 (1)
.github/actions/rustup/action.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: build / Build (Windows)
- GitHub Check: test-rust / Test (Ubuntu)
- GitHub Check: CodeQL Analyze (actions)
- GitHub Check: CodeQL Analyze (javascript-typescript)
🔇 Additional comments (1)
.github/actions/rustup/action.yml (1)
74-81: Cargo cache usage looks good (pinned SHA, fullmatch-only).No issues spotted.
We always setup rust, now.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/workflow-build.yml (1)
86-91: Verify the Rustup composite action supports Windows and uses a Windows-safe cache path.This step runs on Ubuntu and Windows. Past feedback flagged missing Windows handling in .github/actions/rustup/action.yml and a Linux-only cache path (~/.rustup/toolchains). Confirm the composite installs rustup on Windows and caches via %USERPROFILE%.rustup\toolchains (or an OS-aware path). If not yet addressed, fix the composite; no workflow changes needed afterward.
Run this to confirm:
#!/bin/bash set -euo pipefail echo "Check Windows handling in rustup composite" sed -n '1,200p' .github/actions/rustup/action.yml | sed -n '1,200p' >/dev/null # warm cache echo "Search Windows branches:" rg -nP "runner\.os\s*==\s*'Windows'|shell:\s*powershell" .github/actions/rustup/action.yml || true echo echo "Search cache paths:" rg -nP "path:\s*~/.rustup/toolchains|USERPROFILE|\\\.rustup\\toolchains" .github/actions/rustup/action.yml || true echo echo "Search wasm target add:" rg -nP "wasm32-unknown-unknown|rustup target add" .github/actions/rustup/action.yml || true
🧹 Nitpick comments (1)
packages/react/transform/turbo.json (1)
10-10: Add a lightweight preflight or docs note for local dev.With dependsOn cleared, ensure scripts/build_wasm.sh fails fast with a helpful message if rustup/wasm target isn’t present, or document that the local Rustup action (or rustup target add wasm32-unknown-unknown) is required for local builds.
I can add a short rustup check to scripts/build_wasm.sh if you want.
📜 Review details
Configuration used: Path: .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 (5)
.github/workflows/workflow-build.yml(1 hunks)package.json(0 hunks)packages/react/transform/turbo.json(1 hunks)packages/web-platform/web-style-transformer/turbo.json(0 hunks)turbo.json(0 hunks)
💤 Files with no reviewable changes (3)
- packages/web-platform/web-style-transformer/turbo.json
- turbo.json
- package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build / Build (Windows)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: test-rust / Test (Ubuntu)
- GitHub Check: CodeQL Analyze (javascript-typescript)
🔇 Additional comments (1)
packages/react/transform/turbo.json (1)
10-10: Removing cargo-version gating is consistent with the repo-wide shift to the local Rustup action.No functional concerns from Turbo’s perspective; build:wasm remains correctly wired via build → build:wasm.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Qingyu Wang <40660121+colinaaa@users.noreply.github.com>
Summary by CodeRabbit
Edit from Rspack ❤️
Checklist