[zizmor] ignore superfluous-actions#7548
[zizmor] ignore superfluous-actions#7548ShaharNaveh wants to merge 12 commits intoRustPython:mainfrom
superfluous-actions#7548Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughReplaced the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/ci.yaml (2)
414-427:⚠️ Potential issue | 🔴 CriticalUpdate the miri command to use the correct toolchain variable.
Line 414 defines the job environment variable
RUSTUP_TOOLCHAIN: nightly, but Line 427 references the non-existentenv.NIGHTLY_CHANNEL. SinceRUSTUP_TOOLCHAINis already set at the job level, the+${{ env.NIGHTLY_CHANNEL }}prefix should be removed entirely—cargo will automatically use the nightly toolchain without it.🛠️ Proposed fix
- name: Run tests under miri - run: cargo +${{ env.NIGHTLY_CHANNEL }} miri test -p rustpython-vm -- miri_test + run: cargo miri test -p rustpython-vm -- miri_test🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yaml around lines 414 - 427, The workflow uses RUSTUP_TOOLCHAIN: nightly but the "Run tests under miri" step calls cargo with a non-existent toolchain variable (+${{ env.NIGHTLY_CHANNEL }}); remove the +${{ env.NIGHTLY_CHANNEL }} prefix from the "cargo +... miri test -p rustpython-vm -- miri_test" command so it simply runs "cargo miri test -p rustpython-vm -- miri_test" and relies on the job-level RUSTUP_TOOLCHAIN setting.
360-364:⚠️ Potential issue | 🔴 CriticalSplit the
rustfmtinstall into its own step.This step has both
usesandrun, which violates GitHub Actions workflow syntax. A single step cannot define both keys. Moverustup component add rustfmt --toolchain=stableinto a separate step.🛠️ Proposed fix
- uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0 with: python-version: ${{ env.PYTHON_VERSION }} - run: rustup component add rustfmt --toolchain=stable + - name: Install rustfmt + run: rustup component add rustfmt --toolchain=stable🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yaml around lines 360 - 364, The workflow step currently mixes an action usage and a run command (the step with uses: actions/setup-python@a309ff8b426b58ec0e45f0f869d46889d02405 and the run: rustup component add rustfmt --toolchain=stable), which is invalid; fix it by removing the run from that step and creating a new standalone step that only runs the command "rustup component add rustfmt --toolchain=stable" (ensure the new step appears after the setup-python step and uses only the run key), referencing the existing actions/setup-python step to keep ordering.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In @.github/workflows/ci.yaml:
- Around line 414-427: The workflow uses RUSTUP_TOOLCHAIN: nightly but the "Run
tests under miri" step calls cargo with a non-existent toolchain variable (+${{
env.NIGHTLY_CHANNEL }}); remove the +${{ env.NIGHTLY_CHANNEL }} prefix from the
"cargo +... miri test -p rustpython-vm -- miri_test" command so it simply runs
"cargo miri test -p rustpython-vm -- miri_test" and relies on the job-level
RUSTUP_TOOLCHAIN setting.
- Around line 360-364: The workflow step currently mixes an action usage and a
run command (the step with uses:
actions/setup-python@a309ff8b426b58ec0e45f0f869d46889d02405 and the run: rustup
component add rustfmt --toolchain=stable), which is invalid; fix it by removing
the run from that step and creating a new standalone step that only runs the
command "rustup component add rustfmt --toolchain=stable" (ensure the new step
appears after the setup-python step and uses only the run key), referencing the
existing actions/setup-python step to keep ordering.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: b9692e1e-c0cf-422a-9d2f-06acfe2af859
📒 Files selected for processing (1)
.github/workflows/ci.yaml
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/ci.yaml (1)
414-427:⚠️ Potential issue | 🔴 CriticalUpdate the Miri command to reference the correct environment variable.
Line 427 references
env.NIGHTLY_CHANNEL, which is undefined. The job definesRUSTUP_TOOLCHAIN: nightlyat line 414. The cargo command will fail when expanded tocargo +(with empty toolchain). Update to use the defined variable:Suggested fix
- run: cargo +${{ env.NIGHTLY_CHANNEL }} miri test -p rustpython-vm -- miri_test + run: cargo +${{ env.RUSTUP_TOOLCHAIN }} miri test -p rustpython-vm -- miri_test🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yaml around lines 414 - 427, The Miri test step uses an undefined env var NIGHTLY_CHANNEL; update the "Run tests under miri" step to use the defined RUSTUP_TOOLCHAIN variable (or the literal "nightly") so the cargo +<toolchain> invocation expands correctly — change the cargo command that references env.NIGHTLY_CHANNEL to reference env.RUSTUP_TOOLCHAIN (or hardcode "nightly") in that step.
🤖 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 50-53: The Miri job is using an undefined env var `${{
env.NIGHTLY_CHANNEL }}` and several "stable" jobs install stable but don't force
it for subsequent cargo commands; update the Miri step that runs `cargo +${{
env.NIGHTLY_CHANNEL }} miri test` to use a concrete toolchain (either `cargo
+nightly miri test` or `cargo +${{ env.RUSTUP_TOOLCHAIN }} miri test`) and
ensure the MUST-FIX env is `RUSTUP_TOOLCHAIN: nightly` where that job sets it;
for each job that installs `stable` (where `rustup toolchain install stable` is
run) either add `RUSTUP_TOOLCHAIN: stable` at the job-level env or change
subsequent invocations like `cargo clippy`, `cargo shear`, and other `cargo ...`
steps to explicitly run `cargo +stable ...` so they deterministically use the
stable toolchain.
---
Outside diff comments:
In @.github/workflows/ci.yaml:
- Around line 414-427: The Miri test step uses an undefined env var
NIGHTLY_CHANNEL; update the "Run tests under miri" step to use the defined
RUSTUP_TOOLCHAIN variable (or the literal "nightly") so the cargo +<toolchain>
invocation expands correctly — change the cargo command that references
env.NIGHTLY_CHANNEL to reference env.RUSTUP_TOOLCHAIN (or hardcode "nightly") in
that step.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 5bda2a95-4ca3-4b4f-9a59-1eb46e269f5e
📒 Files selected for processing (1)
.github/workflows/ci.yaml
.github/workflows/ci.yaml
Outdated
| - run: rustup toolchain install stable | ||
|
|
||
| - name: Install clippy | ||
| run: rustup component add clippy --toolchain=stable |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Stable provisioning:"
rg -n 'rustup toolchain install stable|rustup component add .*--toolchain=stable' .github/workflows/ci.yaml
echo
echo "Explicit toolchain selection:"
rg -n 'RUSTUP_TOOLCHAIN:|cargo \+stable|rustup (default|override set) stable' .github/workflows/ci.yamlRepository: RustPython/RustPython
Length of output: 594
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Lines with stable provisioning and surrounding context (10 lines after) ==="
for line in 50 170 251 364 444 519; do
echo "--- Line $line ---"
sed -n "${line},$((line+10))p" .github/workflows/ci.yaml
echo
done
echo "=== Checking for NIGHTLY_CHANNEL definition ==="
rg -n 'NIGHTLY_CHANNEL' .github/workflows/ci.yaml
echo "=== Line 414 context and following 15 lines ==="
sed -n '414,429p' .github/workflows/ci.yamlRepository: RustPython/RustPython
Length of output: 3382
Fix undefined environment variable and explicitly select stable toolchain.
Two related issues:
-
Critical bug at line 427: The Miri job references
${{ env.NIGHTLY_CHANNEL }}, which is never defined. The job setsRUSTUP_TOOLCHAIN: nightly(line 414) but uses the wrong environment variable name. Replacecargo +${{ env.NIGHTLY_CHANNEL }} miri testwith eithercargo +nightly miri testorcargo +${{ env.RUSTUP_TOOLCHAIN }} miri test. -
Stable jobs don't deterministically select
stable: Lines 50, 170, 251, 364, 444, and 519 provisionstable, but the subsequentcargoinvocations in those jobs (e.g.,cargo clippy,cargo shear) don't explicitly specify the toolchain. These steps will use whatever toolchain is default on the runner image, not the newly installedstable. AddRUSTUP_TOOLCHAIN: stableat the job level, or usecargo +stable ...in subsequent build commands.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ci.yaml around lines 50 - 53, The Miri job is using an
undefined env var `${{ env.NIGHTLY_CHANNEL }}` and several "stable" jobs install
stable but don't force it for subsequent cargo commands; update the Miri step
that runs `cargo +${{ env.NIGHTLY_CHANNEL }} miri test` to use a concrete
toolchain (either `cargo +nightly miri test` or `cargo +${{ env.RUSTUP_TOOLCHAIN
}} miri test`) and ensure the MUST-FIX env is `RUSTUP_TOOLCHAIN: nightly` where
that job sets it; for each job that installs `stable` (where `rustup toolchain
install stable` is run) either add `RUSTUP_TOOLCHAIN: stable` at the job-level
env or change subsequent invocations like `cargo clippy`, `cargo shear`, and
other `cargo ...` steps to explicitly run `cargo +stable ...` so they
deterministically use the stable toolchain.
|
any reason to change? |
The initial motivation of this PR was to resolve "note" level warnings from zizmor, as the functionality of |
|
After reading zizmorcore/zizmor#1817 I've changed my mind. I'll make zizmor to ignore this, and shift this PR to ignore this rule instead |
ShaharNaveh
changed the title
rustup in ci.yamlsuperfluous-actions
2 participants
Summary by CodeRabbit