Unpin Rust nightly in CI #6022
Unpin Rust nightly in CI #6022
Conversation
WalkthroughThe CI workflow for the "miri" job was updated to use a generic "nightly" Rust toolchain channel instead of a fixed nightly version. The environment variable for the toolchain was renamed, and the test execution step was modified to set Miri-specific flags using an explicit environment map. Changes
Sequence Diagram(s)sequenceDiagram
participant CI_Workflow
participant GitHub_Action
participant Cargo
participant Miri
CI_Workflow->>GitHub_Action: Set NIGHTLY_CHANNEL="nightly"
GitHub_Action->>Cargo: Use toolchain from NIGHTLY_CHANNEL
CI_Workflow->>Cargo: Run tests with MIRIFLAGS='-Zmiri-ignore-leaks'
Cargo->>Miri: Execute tests under Miri with specified flags
Estimated code review effort1 (~2 minutes) Possibly related PRs
Poem
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/ci.yaml (1)
356-365: Redundant “+${{ env.NIGHTLY_CHANNEL }}” — the toolchain override is already in effect.
dtolnay/rust-toolchaininstalls and overrides the directory with the requested toolchain (override: trueby default).
Adding+nightlyto thecargoinvocation is therefore superfluous and slightly obscures intent.A minimal adjustment:
- run: cargo +${{ env.NIGHTLY_CHANNEL }} miri test -p rustpython-vm -- miri_test + run: cargo miri test -p rustpython-vm -- miri_testThis keeps the command shorter while still running under nightly.
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci.yaml(1 hunks)
🧠 Learnings (2)
📓 Common learnings
Learnt from: arihant2math
PR: RustPython/RustPython#5790
File: build.rs:2-2
Timestamp: 2025-06-28T16:31:03.991Z
Learning: In Cargo build scripts (build.rs), the environment variable CARGO_CFG_TARGET_OS is guaranteed to exist and is automatically set by Cargo during the build process, making unwrap() safe to use when accessing this variable.
.github/workflows/ci.yaml (3)
Learnt from: ShaharNaveh
PR: #5932
File: .github/workflows/comment-commands.yml:18-24
Timestamp: 2025-07-10T10:08:43.330Z
Learning: In GitHub Actions workflows for the RustPython project, the maintainer ShaharNaveh prefers to keep workflows simple and doesn't mind if steps fail when the desired state is already achieved (e.g., user already assigned to an issue). Avoid suggesting complex error handling for edge cases they don't consider problematic.
Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-19T03:16:56.511Z
Learning: Applies to Lib/test/**/*.py : When tests fail due to unsupported syntax, keep the test as @unittest.expectedFailure, document that it requires PEP 695 support, and focus on tests that can be fixed through Rust code changes only
Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-19T03:16:56.511Z
Learning: Applies to **/*.rs : Always run clippy to lint code (cargo clippy) before completing tasks. Fix any warnings or lints that are introduced by your changes
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: arihant2math
PR: RustPython/RustPython#5790
File: build.rs:2-2
Timestamp: 2025-06-28T16:31:03.991Z
Learning: In Cargo build scripts (build.rs), the environment variable CARGO_CFG_TARGET_OS is guaranteed to exist and is automatically set by Cargo during the build process, making unwrap() safe to use when accessing this variable.
.github/workflows/ci.yaml (3)
Learnt from: ShaharNaveh
PR: #5932
File: .github/workflows/comment-commands.yml:18-24
Timestamp: 2025-07-10T10:08:43.330Z
Learning: In GitHub Actions workflows for the RustPython project, the maintainer ShaharNaveh prefers to keep workflows simple and doesn't mind if steps fail when the desired state is already achieved (e.g., user already assigned to an issue). Avoid suggesting complex error handling for edge cases they don't consider problematic.
Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-19T03:16:56.511Z
Learning: Applies to Lib/test/**/*.py : When tests fail due to unsupported syntax, keep the test as @unittest.expectedFailure, document that it requires PEP 695 support, and focus on tests that can be fixed through Rust code changes only
Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-19T03:16:56.511Z
Learning: Applies to **/*.rs : Always run clippy to lint code (cargo clippy) before completing tasks. Fix any warnings or lints that are introduced by your changes
| env: | ||
| RUSTUP_TOOLCHAIN: nightly-2025-07-18 # TODO: Set to "nightly" once https://github.com/rust-lang/rust/issues/144168 is fixed | ||
| NIGHTLY_CHANNEL: nightly |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Prefer the built-in RUSTUP_TOOLCHAIN variable over a bespoke NIGHTLY_CHANNEL.
RUSTUP_TOOLCHAIN is understood by both cargo and rustup; setting it once eliminates the need to thread a custom variable through every command. Unless you have a strong reason to diverge, consider reverting to the canonical env-var.
🏁 Script executed:
#!/bin/bash
# Check for remaining references to the old variable that would now be dead code.
rg -n 'RUSTUP_TOOLCHAIN' .github/workflowsLength of output: 42
🏁 Script executed:
#!/bin/bash
# Verify all usages of the custom NIGHTLY_CHANNEL variable in CI workflows
rg -n 'NIGHTLY_CHANNEL' .github/workflows -A2Length of output: 680
Replace custom NIGHTLY_CHANNEL with the built-in RUSTUP_TOOLCHAIN
To standardize on the canonical toolchain variable and avoid threading a bespoke env var through every command, update your CI workflow as follows:
• In .github/workflows/ci.yaml around line 352, rename the environment variable:
- env:
- NIGHTLY_CHANNEL: nightly
+ env:
+ RUSTUP_TOOLCHAIN: nightly• In the Rust setup step (around line 358), point toolchain at the new var:
- with:
- toolchain: ${{ env.NIGHTLY_CHANNEL }}
+ with:
+ toolchain: ${{ env.RUSTUP_TOOLCHAIN }}• In the Miri test step (around line 364), you can drop the +${…} prefix now that the default toolchain is set:
- run: cargo +${{ env.NIGHTLY_CHANNEL }} miri test -p rustpython-vm -- miri_test
+ run: cargo miri test -p rustpython-vm -- miri_testThis ensures both rustup and cargo honor your chosen nightly channel without a custom variable.
🤖 Prompt for AI Agents
In .github/workflows/ci.yaml around lines 351 to 364, replace the custom
environment variable NIGHTLY_CHANNEL with the built-in RUSTUP_TOOLCHAIN to
standardize the toolchain usage. Rename the env variable NIGHTLY_CHANNEL to
RUSTUP_TOOLCHAIN at line 352, update the Rust setup step near line 358 to use
the new RUSTUP_TOOLCHAIN variable for the toolchain setting, and modify the Miri
test step around line 364 to remove the '+${…}' prefix since the default
toolchain is now set via RUSTUP_TOOLCHAIN. This change ensures rustup and cargo
consistently use the specified nightly channel without a bespoke env var.
Summary by CodeRabbit