Skip to content

Split cargo check matrix to individual targets. Avoid cache poisoning#7540

Merged
youknowone merged 4 commits intoRustPython:mainfrom
ShaharNaveh:cargo-check-split-and-cache
Mar 30, 2026
Merged

Split cargo check matrix to individual targets. Avoid cache poisoning#7540
youknowone merged 4 commits intoRustPython:mainfrom
ShaharNaveh:cargo-check-split-and-cache

Conversation

@ShaharNaveh
Copy link
Copy Markdown
Contributor

@ShaharNaveh ShaharNaveh commented Mar 30, 2026 •
edited by coderabbitai bot
Loading

The main motivation of this PR is to move away from Swatinem/rust-cache action, as it doesn't allow a fine grained control over the cache like actions/cache does.

This PR will resolve the following lints warnings:

  • runtime artifacts potentially vulnerable to a cache poisoning attack (by using other actions/cache)
  • action functionality is already included by the runner (by using rustup instead of dtolnay/rust-toolchain)

Summary by CodeRabbit

Release Notes

No user-facing changes

This release contains internal improvements to the build and continuous integration infrastructure only.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 30, 2026 •
edited
Loading

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 4b694ed5-7013-4d21-b1e2-20d1556c7d9e

📥 Commits

Reviewing files that changed from the base of the PR and between 57a6b73 and 7e6a7cb.

📒 Files selected for processing (1)
  • .github/workflows/ci.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/ci.yaml
📝 Walkthrough

Walkthrough

Restructures the CI cargo_check job to run per-target matrix entries, adds explicit env vars, replaces Swatinem/rust-cache with actions/cache restore/save, switches from dtolnay/rust-toolchain to manual rustup installs, and adjusts Android NDK and caching conditions.

Changes

Cohort / File(s) Summary
CI Workflow
.github/workflows/ci.yaml		 Reworks cargo_check job: set CARGO_INCREMENTAL=0 and CARGO_TERM_COLOR=always; change matrix from multi-target loop to per-target rows; replace Swatinem/rust-cache with actions/cache/restore and actions/cache/save using keys scoped by runner.os and matrix.target (save keyed also on Cargo.toml, Cargo.lock, github.sha); switch toolchain setup from dtolnay/rust-toolchain to rustup toolchain install stable --target "${{ matrix.target }}"; adjust Android NDK setup condition to matrix.target == 'aarch64-linux-android'; run a single cargo check --target "${{ matrix.target }}" per job.

Sequence Diagram(s)

(omitted)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • youknowone

Poem

🐰 I hopped through YAML, tidy and bright,

Caches snug, targets set right.
Per-target steps now bound and neat,
Rust checks hop forward, light on their feet.
🥕🦀

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main changes in the pull request: splitting the cargo check matrix to individual targets and implementing cache poisoning prevention measures.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@ShaharNaveh ShaharNaveh force-pushed the cargo-check-split-and-cache branch from 57a6b73 to 7e6a7cb Compare March 30, 2026 10:37
@youknowone youknowone merged commit 2703f94 into RustPython:main Mar 30, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@youknowone youknowone youknowone approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ShaharNaveh @youknowone