Use matrix for cargo check#7388
Conversation
📝 WalkthroughWalkthroughReplaced per-target CI steps with a matrix-driven Changes
Sequence Diagram(s)sequenceDiagram
participant Runner as GitHub Runner
participant Action as install-linux-deps Action
participant Apt as apt-get
participant Toolchain as rustup / toolchain
participant Cache as action/cache
participant NDK as Android NDK (conditional)
participant Cargo as cargo check
Runner->>Action: invoke with inputs (e.g., gcc-aarch64-linux-gnu)
Action->>Apt: apt-get install --no-install-recommends (add arm64 cross pkgs if input=true)
Runner->>Toolchain: setup Rust toolchain and add targets
Runner->>Cache: restore/save cargo cache
alt Android target present
Runner->>NDK: setup Android NDK
end
Runner->>Cargo: loop over matrix targets -> cargo check (per-target env)
Cargo-->>Runner: return check results
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yaml:
- Around line 197-229: The Install dependencies step uses a scalar expression
for with: (with: ${{ matrix.dependencies || fromJSON('{}') }}) which violates
GitHub Actions schema; change the with: to an explicit mapping of the action
inputs (the boolean flags) instead of a single scalar. Update the step that uses
./.github/actions/install-linux-deps so its with: lists each dependency key as a
mapping entry (e.g., gcc-multilib, musl-tools, gcc-aarch64-linux-gnu) and set
each value to a safe expression that reads the matrix entry with a default
(e.g., ${{ matrix.dependencies.gcc-multilib || false }}), ensuring every
possible dependency key in the matrix is represented as a boolean input.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 81db98f2-101c-44e3-9ba0-013dbad11b83
📒 Files selected for processing (2)
.github/actions/install-linux-deps/action.yml.github/workflows/ci.yaml
| include: | ||
| - os: ubuntu-latest | ||
| targets: | ||
| - aarch64-linux-android | ||
| - i686-unknown-linux-gnu | ||
| - i686-unknown-linux-musl | ||
| - wasm32-wasip2 | ||
| - x86_64-unknown-freebsd | ||
| dependencies: | ||
| gcc-multilib: true | ||
| musl-tools: true | ||
| - os: ubuntu-latest | ||
| targets: | ||
| - aarch64-unknown-linux-gnu | ||
| dependencies: | ||
| gcc-aarch64-linux-gnu: true # conflict with `gcc-multilib` | ||
| - os: macos-latest | ||
| targets: | ||
| - aarch64-apple-ios | ||
| - x86_64-apple-darwin | ||
| fail-fast: false | ||
| steps: | ||
| - uses: actions/[email protected] | ||
| - uses: dtolnay/rust-toolchain@stable | ||
| - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 | ||
| with: | ||
| target: i686-unknown-linux-gnu | ||
| persist-credentials: false | ||
|
|
||
| - name: Install gcc-multilib and musl-tools | ||
| uses: ./.github/actions/install-linux-deps | ||
| - uses: Swatinem/rust-cache@v2 | ||
| with: | ||
| gcc-multilib: true | ||
| musl-tools: true | ||
| prefix-key: v0-rust-${{ join(matrix.targets, '-') }} | ||
|
|
||
| - name: Check compilation for x86 32bit | ||
| run: cargo check --target i686-unknown-linux-gnu ${{ env.CARGO_ARGS_NO_SSL }} | ||
| - name: Install dependencies | ||
| uses: ./.github/actions/install-linux-deps | ||
| with: ${{ matrix.dependencies || fromJSON('{}') }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and examine the install-linux-deps action
fd -t f "install-linux-deps" --type dRepository: RustPython/RustPython
Length of output: 47
🏁 Script executed:
# List the contents of the install-linux-deps action directory
git ls-files ".github/actions/install-linux-deps/"Repository: RustPython/RustPython
Length of output: 110
🏁 Script executed:
# Read the install-linux-deps action.yml to see its inputs
cat ".github/actions/install-linux-deps/action.yml"Repository: RustPython/RustPython
Length of output: 1730
🏁 Script executed:
# Also check the install-macos-deps for comparison (mentioned in the learning)
cat ".github/actions/install-macos-deps/action.yml"Repository: RustPython/RustPython
Length of output: 1579
🏁 Script executed:
# Read the current ci.yaml workflow to see the exact line 229 context
sed -n '190,235p' ".github/workflows/ci.yaml"Repository: RustPython/RustPython
Length of output: 1453
Fix the schema violation: with: must be a mapping, not a scalar expression.
Line 229 passes a scalar expression where GitHub Actions requires an explicit mapping. Flatten the matrix flags into individual booleans and bind each action input explicitly.
Proposed fix
matrix:
include:
- os: ubuntu-latest
targets:
- aarch64-linux-android
- i686-unknown-linux-gnu
- i686-unknown-linux-musl
- wasm32-wasip2
- x86_64-unknown-freebsd
- dependencies:
- gcc-multilib: true
- musl-tools: true
+ gcc_multilib: true
+ musl_tools: true
- os: ubuntu-latest
targets:
- aarch64-unknown-linux-gnu
- dependencies:
- gcc-aarch64-linux-gnu: true # conflict with `gcc-multilib`
+ gcc_aarch64_linux_gnu: true # conflict with `gcc-multilib`
- os: macos-latest
targets:
- aarch64-apple-ios
- x86_64-apple-darwin
fail-fast: false
...
- name: Install dependencies
+ if: runner.os == 'Linux'
uses: ./.github/actions/install-linux-deps
- with: ${{ matrix.dependencies || fromJSON('{}') }}
+ with:
+ gcc-multilib: ${{ matrix.gcc_multilib || false }}
+ musl-tools: ${{ matrix.musl_tools || false }}
+ clang: ${{ matrix.clang || false }}
+ gcc-aarch64-linux-gnu: ${{ matrix.gcc_aarch64_linux_gnu || false }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| include: | |
| - os: ubuntu-latest | |
| targets: | |
| - aarch64-linux-android | |
| - i686-unknown-linux-gnu | |
| - i686-unknown-linux-musl | |
| - wasm32-wasip2 | |
| - x86_64-unknown-freebsd | |
| dependencies: | |
| gcc-multilib: true | |
| musl-tools: true | |
| - os: ubuntu-latest | |
| targets: | |
| - aarch64-unknown-linux-gnu | |
| dependencies: | |
| gcc-aarch64-linux-gnu: true # conflict with `gcc-multilib` | |
| - os: macos-latest | |
| targets: | |
| - aarch64-apple-ios | |
| - x86_64-apple-darwin | |
| fail-fast: false | |
| steps: | |
| - uses: actions/[email protected] | |
| - uses: dtolnay/rust-toolchain@stable | |
| - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 | |
| with: | |
| target: i686-unknown-linux-gnu | |
| persist-credentials: false | |
| - name: Install gcc-multilib and musl-tools | |
| uses: ./.github/actions/install-linux-deps | |
| - uses: Swatinem/rust-cache@v2 | |
| with: | |
| gcc-multilib: true | |
| musl-tools: true | |
| prefix-key: v0-rust-${{ join(matrix.targets, '-') }} | |
| - name: Check compilation for x86 32bit | |
| run: cargo check --target i686-unknown-linux-gnu ${{ env.CARGO_ARGS_NO_SSL }} | |
| - name: Install dependencies | |
| uses: ./.github/actions/install-linux-deps | |
| with: ${{ matrix.dependencies || fromJSON('{}') }} | |
| include: | |
| - os: ubuntu-latest | |
| targets: | |
| - aarch64-linux-android | |
| - i686-unknown-linux-gnu | |
| - i686-unknown-linux-musl | |
| - wasm32-wasip2 | |
| - x86_64-unknown-freebsd | |
| gcc_multilib: true | |
| musl_tools: true | |
| - os: ubuntu-latest | |
| targets: | |
| - aarch64-unknown-linux-gnu | |
| gcc_aarch64_linux_gnu: true # conflict with `gcc-multilib` | |
| - os: macos-latest | |
| targets: | |
| - aarch64-apple-ios | |
| - x86_64-apple-darwin | |
| fail-fast: false | |
| steps: | |
| - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 | |
| with: | |
| persist-credentials: false | |
| - uses: Swatinem/rust-cache@v2 | |
| with: | |
| prefix-key: v0-rust-${{ join(matrix.targets, '-') }} | |
| - name: Install dependencies | |
| if: runner.os == 'Linux' | |
| uses: ./.github/actions/install-linux-deps | |
| with: | |
| gcc-multilib: ${{ matrix.gcc_multilib || false }} | |
| musl-tools: ${{ matrix.musl_tools || false }} | |
| clang: ${{ matrix.clang || false }} | |
| gcc-aarch64-linux-gnu: ${{ matrix.gcc_aarch64_linux_gnu || false }} |
🧰 Tools
🪛 actionlint (1.7.11)
[error] 229-229: "with" section is scalar node but mapping node is expected
(syntax-check)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ci.yaml around lines 197 - 229, The Install dependencies
step uses a scalar expression for with: (with: ${{ matrix.dependencies ||
fromJSON('{}') }}) which violates GitHub Actions schema; change the with: to an
explicit mapping of the action inputs (the boolean flags) instead of a single
scalar. Update the step that uses ./.github/actions/install-linux-deps so its
with: lists each dependency key as a mapping entry (e.g., gcc-multilib,
musl-tools, gcc-aarch64-linux-gnu) and set each value to a safe expression that
reads the matrix entry with a default (e.g., ${{
matrix.dependencies.gcc-multilib || false }}), ensuring every possible
dependency key in the matrix is represented as a boolean input.
| - name: Check compilation for wasip2 | ||
| run: cargo check --target wasm32-wasip2 ${{ env.CARGO_ARGS_NO_SSL }} | ||
|
|
||
| # - name: Prepare repository for redox compilation |
There was a problem hiding this comment.
where should redox to be put?
There was a problem hiding this comment.
In one of the target groups. I can try to do in a separate PR. this PR isn't meant to change existing behavior.
I assume we will have a special condition for it like we do in the setup-ndk step
There was a problem hiding this comment.
redox is(was?) actually using rustpython for their python. This build was guaranteeing it is working.
I forgot when this is commented out and not fixed back, (probably redoxer action is broken) but we at least need to keep the comment to revive it later.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.github/workflows/ci.yaml (1)
227-229:⚠️ Potential issue | 🔴 CriticalFix the
Install dependenciesstep shape and scope.Line 229 is invalid GitHub Actions syntax:
with:must be a mapping, so this job will not pass workflow validation. Even after that is fixed, this step still runs for themacos-latestrow even though it invokes the Linux-onlyinstall-linux-depsaction.One working shape
matrix: include: - os: ubuntu-latest targets: - aarch64-linux-android - i686-unknown-linux-gnu - i686-unknown-linux-musl - wasm32-wasip2 - x86_64-unknown-freebsd - dependencies: - gcc-multilib: true - musl-tools: true + gcc_multilib: true + musl_tools: true - os: ubuntu-latest targets: - aarch64-unknown-linux-gnu - dependencies: - gcc-aarch64-linux-gnu: true # conflict with `gcc-multilib` + gcc_aarch64_linux_gnu: true # conflict with `gcc-multilib` - os: macos-latest targets: - aarch64-apple-ios - x86_64-apple-darwin ... - name: Install dependencies + if: runner.os == 'Linux' uses: ./.github/actions/install-linux-deps - with: ${{ matrix.dependencies || fromJSON('{}') }} + with: + gcc-multilib: ${{ matrix.gcc_multilib || false }} + musl-tools: ${{ matrix.musl_tools || false }} + clang: ${{ matrix.clang || false }} + gcc-aarch64-linux-gnu: ${{ matrix.gcc_aarch64_linux_gnu || false }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yaml around lines 227 - 229, The "Install dependencies" step uses invalid YAML for the with: field and also unconditionally invokes the Linux-only action ./.github/actions/install-linux-deps for non-Linux matrix rows; change the step to provide a proper mapping for the inputs (e.g., a key like dependencies mapped to the expression using matrix.dependencies) and add a conditional that restricts the step to Linux matrix entries (use matrix.os in the if: expression) so the step only runs for Linux jobs and the with: block is valid.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.github/workflows/ci.yaml:
- Around line 227-229: The "Install dependencies" step uses invalid YAML for the
with: field and also unconditionally invokes the Linux-only action
./.github/actions/install-linux-deps for non-Linux matrix rows; change the step
to provide a proper mapping for the inputs (e.g., a key like dependencies mapped
to the expression using matrix.dependencies) and add a conditional that
restricts the step to Linux matrix entries (use matrix.os in the if: expression)
so the step only runs for Linux jobs and the with: block is valid.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 68bac21c-80f5-4e72-a059-13705ac74f77
📒 Files selected for processing (1)
.github/workflows/ci.yaml
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.github/workflows/ci.yaml (1)
227-229:⚠️ Potential issue | 🔴 CriticalSchema violation:
with:requires a mapping, not a scalar expression.The syntax
with: ${{ matrix.dependencies || fromJSON('{}') }}is invalid. GitHub Actions expectswith:to contain an explicit mapping of input names to values. Additionally, this step will run on macOS whereinstall-linux-depsis not applicable.The fix proposed in the previous review remains valid—flatten the dependency flags to top-level matrix properties and bind them explicitly with an OS condition.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yaml around lines 227 - 229, The CI step "Install dependencies" uses an invalid scalar for with: and is not guarded by OS; change the step to pass a mapping (e.g. with: dependencies: ${{ matrix.dependencies }} ) and add an OS guard (if: runner.os == 'Linux') so the local action ./.github/actions/install-linux-deps only runs on Linux; also ensure your matrix defines the top-level property matrix.dependencies (use fromJSON default when building the matrix if needed) so the referenced matrix key exists.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.github/workflows/ci.yaml:
- Around line 227-229: The CI step "Install dependencies" uses an invalid scalar
for with: and is not guarded by OS; change the step to pass a mapping (e.g.
with: dependencies: ${{ matrix.dependencies }} ) and add an OS guard (if:
runner.os == 'Linux') so the local action ./.github/actions/install-linux-deps
only runs on Linux; also ensure your matrix defines the top-level property
matrix.dependencies (use fromJSON default when building the matrix if needed) so
the referenced matrix key exists.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 2aaff4f0-652d-4034-a957-2a97a27a6ca8
📒 Files selected for processing (1)
.github/workflows/ci.yaml
2 participants
Summary by CodeRabbit