X Tutup
Skip to content

Unify lint CI job #7505

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
youknowone merged 19 commits into from
Mar 26, 2026
+174 −151
Merged

Unify lint CI job #7505

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
0c9f3e5
use `prek` for unified linting
ShaharNaveh Mar 24, 2026
72fd632
Fix actionlint error
ShaharNaveh Mar 25, 2026
149cc38
Generate metadata when specific files change
ShaharNaveh Mar 25, 2026
e09cf9e
`check_redundant_patches.py` to accept glob path
ShaharNaveh Mar 25, 2026
15b06c2
Test
ShaharNaveh Mar 25, 2026
59a7d2c
revert defective changes
ShaharNaveh Mar 25, 2026
b39ab51
use `rustfmt` over `cargo fmt` for individual files
ShaharNaveh Mar 26, 2026
2f12793
Merge remote-tracking branch 'upstream/main' into ci-lint-unify
ShaharNaveh Mar 26, 2026
c7be0d6
debug reviewdog
ShaharNaveh Mar 26, 2026
4c3b303
rustfmt
ShaharNaveh Mar 26, 2026
3adf498
Move comment to correct location
ShaharNaveh Mar 26, 2026
855eca1
defevtive fmt test
ShaharNaveh Mar 26, 2026
15bdf1f
Fail with reviewdog
ShaharNaveh Mar 26, 2026
cefe51f
fix reviewdog perms
ShaharNaveh Mar 26, 2026
ae898da
Try to use present token
ShaharNaveh Mar 26, 2026
88028e6
without checks oerms
ShaharNaveh Mar 26, 2026
0d52e5e
put normal perms
ShaharNaveh Mar 26, 2026
eefe093
fmt
ShaharNaveh Mar 26, 2026
b2ba0c3
Merge remote-tracking branch 'upstream/main' into ci-lint-unify
ShaharNaveh Mar 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .github/dependabot.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,3 +159,9 @@ updates:
semver-major-days: 30
semver-minor-days: 7
semver-patch-days: 3
- package-ecosystem: pre-commit
directory: /
schedule:
interval: weekly
cooldown:
default-days: 7
109 changes: 47 additions & 62 deletions .github/workflows/ci.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ jobs:
- name: check compilation without threading
run: cargo check ${{ env.CARGO_ARGS }}

- run: cargo doc --locked
Copy link
Contributor Author

@ShaharNaveh ShaharNaveh Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to here because cargo check and cargo doc uses the same profile

if: runner.os == 'Linux'

- name: check compilation without host_env (sandbox mode)
run: |
cargo check -p rustpython-vm --no-default-features --features compiler
Expand Down Expand Up @@ -326,8 +329,13 @@ jobs:
run: python -I scripts/whats_left.py ${{ env.CARGO_ARGS }} --features jit

lint:
name: Lint Rust & Python code
name: Lint
runs-on: ubuntu-latest
permissions:
contents: read
checks: write
pull-requests: write
security-events: write # for zizmor
Comment on lines +332 to +338
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Restore the skip:ci guard on lint if that label is still meant to suppress the whole workflow.

Every other job in this workflow still short-circuits on skip:ci, but the unified lint job no longer does. Applying that label will therefore still run this path.

♻️ Suggested fix 
   lint:
+    if: ${{ !contains(github.event.pull_request.labels.*.name, 'skip:ci') }}
     name: Lint
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/ci.yaml around lines 332 - 338, The unified lint job
("name: Lint") is missing the skip:ci short-circuit used elsewhere; update the
Lint job to include the same conditional guard used by other jobs so the
workflow is skipped when the skip:ci label is present on a PR (i.e., add the
same "if" expression/guard that checks for the skip:ci label to the Lint job),
leaving the existing permissions block intact.

Comment on lines +334 to +338
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's find and examine the workflow file
find . -name "ci.yaml" -o -name "ci.yml" | head -20

Repository: RustPython/RustPython

Length of output: 92

🌐 Web query:

reviewdog/action-suggester permissions requirements documentation

💡 Result:

The reviewdog/action-suggester GitHub Action requires the following permissions in your workflow YAML: permissions: contents: read checks: write issues: write pull-requests: write These are specified in the official README to enable posting code suggestions via GitHub's multi-line suggestions feature on pull requests. See GitHub docs on Assigning permissions to jobs for details. The action supports pull_request events primarily.

Citations:

🏁 Script executed:

# Read the workflow file around the mentioned lines
fd "ci.yaml" --type f --exec wc -l {} \; 2>/dev/null | head -5

Repository: RustPython/RustPython

Length of output: 47

🏁 Script executed:

# Read the workflow file to check permissions and action usage
cat -n .github/workflows/ci.yaml | sed -n '320,400p'

Repository: RustPython/RustPython

Length of output: 3245

🏁 Script executed:

# Also check the workflow triggers at the top
cat -n .github/workflows/ci.yaml | head -20

Repository: RustPython/RustPython

Length of output: 915

Add missing issues: write permission and condition the reviewdog/action-suggester to pull requests only.

The action's documentation specifies it requires issues: write in addition to checks: write and pull-requests: write. Since the workflow runs on push, merge_group, and workflow_dispatch events, the current if: failure() condition can invoke the action outside pull request context where it cannot function. Add the required permission and limit execution to pull request events.

Suggested fix 
     permissions:
       contents: read
       checks: write
+      issues: write
       pull-requests: write
       security-events: write # for zizmor
       - name: reviewdog
-        if: failure()
+        if: failure() && github.event_name == 'pull_request'
         uses: reviewdog/action-suggester@aa38384ceb608d00f84b4690cacc83a5aba307ff # 1.24.0
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/ci.yaml around lines 334 - 338, Add the missing permission
and guard the reviewdog action to PRs only: under the workflow permissions block
add "issues: write" alongside the existing "checks: write" and "pull-requests:
write", and update the reviewdog/action-suggester step's if condition so it only
runs in a pull request context (e.g. change the current if: failure() to an
expression that also checks the event is a pull_request, like github.event_name
== 'pull_request' && failure()).

steps:
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
with:
Expand All @@ -337,53 +345,52 @@ jobs:
with:
python-version: ${{ env.PYTHON_VERSION }}

- name: Check for redundant test patches
run: python scripts/check_redundant_patches.py

- uses: dtolnay/rust-toolchain@efa25f7f19611383d5b0ccf2d1c8914531636bf9
with:
components: clippy
toolchain: stable
components: rustfmt

- name: run clippy on wasm
run: cargo clippy --manifest-path=crates/wasm/Cargo.toml -- -Dwarnings

- name: Ensure docs generate no warnings
run: cargo doc --locked
- uses: cargo-bins/cargo-binstall@113a77a4ce971c41332f2129c3d995df993cf746 # v1.17.8
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, check the content around line 353 in the workflow file
cat -n .github/workflows/ci.yaml | sed -n '350,360p'

Repository: RustPython/RustPython

Length of output: 431

🌐 Web query:

cargo-binstall action version input default behavior

💡 Result:

The cargo-binstall GitHub Action is cargo-bins/cargo-binstall@main. It installs the cargo-binstall tool (a faster alternative to cargo install that downloads prebuilt binaries). The version input specifies the cargo-binstall version to install (e.g., version: "1.2.3"). If not provided, it defaults to the latest version available from the cargo-binstall GitHub releases. Default behavior: Downloads and installs the latest cargo-binstall binary for the current platform into ~/.cargo/bin, making cargo binstall available for subsequent steps. It supports unattended CI use and works across platforms (Linux, macOS, Windows). For specific versions, provide the version input; otherwise, latest is used automatically.

Citations:

🏁 Script executed:

# Check if the specific action SHA mentioned has version input support
# We'll search for action.yml metadata in the cargo-binstall repo history
curl -s "https://raw.githubusercontent.com/cargo-bins/cargo-binstall/113a77a4ce971c41332f2129c3d995df993cf746/action.yml" 2>/dev/null | head -50

Repository: RustPython/RustPython

Length of output: 852

Pin the cargo-binstall binary version explicitly in the action inputs.

The action SHA pins the action code but not the cargo-binstall binary version. The action defaults to installing the latest cargo-binstall release when the version input is not set, so this job will pick up whatever version is current when the workflow runs, affecting reproducibility.

♻️ Suggested fix 
-      - uses: cargo-bins/cargo-binstall@113a77a4ce971c41332f2129c3d995df993cf746 # v1.17.8
+      - uses: cargo-bins/cargo-binstall@113a77a4ce971c41332f2129c3d995df993cf746 # v1.17.8
+        with:
+          version: "1.17.8"
📝 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.

Suggested change
- uses: cargo-bins/cargo-binstall@113a77a4ce971c41332f2129c3d995df993cf746 # v1.17.8
- uses: cargo-bins/cargo-binstall@113a77a4ce971c41332f2129c3d995df993cf746 # v1.17.8
with:
version: "1.17.8"
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/ci.yaml at line 353, The workflow step that uses the
action "cargo-bins/cargo-binstall@113a77a4ce971c41332f2129c3d995df993cf746"
should explicitly pin the cargo-binstall binary by adding an inputs block
(with:) and setting the version field to a specific release (e.g., version:
"v1.17.8" or the exact binary tag you want), so update the step that contains
the uses: cargo-bins/cargo-binstall... line to include a with: version:
"<pinned-version>" entry to ensure reproducible installs.


- name: Ensure Lib/_opcode_metadata is updated
- name: cargo shear
run: |
python scripts/generate_opcode_metadata.py
if [ -n "$(git status --porcelain)" ]; then
exit 1
fi
cargo binstall --no-confirm cargo-shear
cargo shear
Comment on lines +355 to +358
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Checking current cargo-shear install command:"
rg -n 'cargo binstall --no-confirm .*cargo-shear' .github/workflows/ci.yaml

Repository: RustPython/RustPython

Length of output: 164

🏁 Script executed:

# Check the context around the cargo-shear installation and other tool installations
head -360 .github/workflows/ci.yaml | tail -30

Repository: RustPython/RustPython

Length of output: 919

🏁 Script executed:

# Look for other cargo binstall or tool installation commands to see if versions are pinned elsewhere
rg -n 'cargo binstall|curl.*install|wget' .github/workflows/ci.yaml -A 1 | head -50

Repository: RustPython/RustPython

Length of output: 426

🏁 Script executed:

# Check if there's a Cargo.lock or similar that might indicate version management strategy
git ls-files | grep -E '(Cargo|package.*lock|requirements)'

Repository: RustPython/RustPython

Length of output: 822

🏁 Script executed:

# Check if there are any examples of pinned versions with cargo binstall in the repo
rg -n 'cargo binstall.*@' .

Repository: RustPython/RustPython

Length of output: 47

🏁 Script executed:

# Look at cargo-binstall documentation - check if the repo has any docs or comments about it
rg -n 'cargo-binstall' .github/workflows/ci.yaml -B 2 -A 2

Repository: RustPython/RustPython

Length of output: 233

🏁 Script executed:

# Search for any references to cargo-shear version pinning or issues
rg -n 'cargo-shear' .

Repository: RustPython/RustPython

Length of output: 123

🌐 Web query:

cargo binstall version pinning syntax @

💡 Result:

The version pinning syntax in cargo-binstall uses @ followed by a Cargo version requirement string. For exact versions, use @=VERSION, such as cargo binstall radio-sx128x@=0.14.1-alpha.5. This resolves using Cargo's semver parser, where =0.14.1-alpha.5 pins to exactly that version, and underspecified forms like =1.2 are equivalent to >=1.2.0, <1.3.0. Examples: - Exact version: cargo binstall crate@=1.2.3 - Pre-release: cargo binstall crate@=1.0.0-alpha.1 - Range (compatible updates): cargo binstall crate@^1.2 Alternatively, use the --version VERSION flag: cargo binstall --version 1.2.3 crate. This syntax is shown in the official README and logs, where radio-sx128x@0.14.1-alpha.5 resolves as 'radio-sx128x@=0.14.1-alpha.5'. It leverages Cargo's version req parser (clap-cargo dependency) for flexibility matching Cargo.toml dependencies.

Citations:

Pin cargo-shear to a fixed version for reproducible CI.

Line 357 installs the latest cargo-shear on every run, which can cause sudden CI breakages from upstream releases. The cargo-binstall tool itself is pinned to v1.17.8, so cargo-shear should be pinned as well.

♻️ Proposed fix 
       - name: cargo shear
         run: |
-          cargo binstall --no-confirm cargo-shear
+          cargo binstall --no-confirm cargo-shear@=<pinned-version>
           cargo shear

Use the @=<version> syntax to pin to an exact version.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/ci.yaml around lines 355 - 358, The workflow currently
installs the latest cargo-shear which can break CI; update the step that runs
"cargo binstall --no-confirm cargo-shear" to pin cargo-shear to an exact version
using the cargo-binstall syntax (e.g., "cargo-shear@=x.y.z"); modify the step
named "cargo shear" so the binstall command references the pinned package name
and leave the subsequent "cargo shear" invocation unchanged.


- name: Install ruff
uses: astral-sh/ruff-action@4919ec5cf1f49eff0871dbcea0da843445b837e6 # v3.6.1
with:
version: "0.15.5"
args: "--version"
- name: actionlint
uses: reviewdog/action-actionlint@0d952c597ef8459f634d7145b0b044a9699e5e43 # v1.71.0

- run: ruff check --diff
- name: zizmor
uses: zizmorcore/zizmor-action@71321a20a9ded102f6e9ce5718a2fcec2c4f70d8 # v0.5.2

- run: ruff format --check
- name: restore prek cache
if: ${{ github.ref != 'refs/heads/main' }} # never restore on main
uses: actions/cache/restore@668228422ae6a00e4ad889ee87cd7109ec5666a7 # v5.0.4
with:
key: prek-${{ hashFiles('.pre-commit-config.yaml') }}
path: ~/.cache/prek

- name: install prettier
run: |
yarn global add prettier
yarn global bin >> "$GITHUB_PATH"
- name: prek
id: prek
uses: j178/prek-action@79f765515bd648eb4d6bb1b17277b7cb22cb6468 # v2.0.0
with:
cache: false
show-verbose-logs: false
continue-on-error: true

- name: check wasm code with prettier
# prettier doesn't handle ignore files very well: https://github.com/prettier/prettier/issues/8506
run: cd wasm && git ls-files -z | xargs -0 prettier --check -u
# Keep cspell check as the last step. This is optional test.
- name: install extra dictionaries
run: npm install @cspell/dict-en_us @cspell/dict-cpp @cspell/dict-python @cspell/dict-rust @cspell/dict-win32 @cspell/dict-shell
- name: spell checker
uses: streetsidesoftware/cspell-action@v8
- name: save prek cache
if: ${{ github.ref == 'refs/heads/main' }} # only save on main
uses: actions/cache/save@668228422ae6a00e4ad889ee87cd7109ec5666a7 # v5.0.4
with:
files: "**/*.rs"
incremental_files_only: true
key: prek-${{ hashFiles('.pre-commit-config.yaml') }}
path: ~/.cache/prek

- name: reviewdog
uses: reviewdog/action-suggester@aa38384ceb608d00f84b4690cacc83a5aba307ff # 1.24.0
with:
level: warning
fail_level: error
cleanup: false

miri:
if: ${{ !contains(github.event.pull_request.labels.*.name, 'skip:ci') }}
Expand Down Expand Up @@ -425,12 +432,16 @@ jobs:

- uses: dtolnay/rust-toolchain@efa25f7f19611383d5b0ccf2d1c8914531636bf9
with:
components: clippy
toolchain: stable

- uses: Swatinem/rust-cache@c19371144df3bb44fab255c43d04cbc2ab54d1c4 # v2.9.1
with:
save-if: ${{ github.ref == 'refs/heads/main' }}

- name: cargo clippy
run: cargo clippy --manifest-path=crates/wasm/Cargo.toml -- -Dwarnings

- name: install wasm-pack
run: curl https://rustwasm.github.io/wasm-pack/installer/init.sh -sSf | sh
- name: install geckodriver
Expand Down Expand Up @@ -520,29 +531,3 @@ jobs:
run: wasmer run --dir "$(pwd)" target/wasm32-wasip1/release/rustpython.wasm -- "$(pwd)/extra_tests/snippets/stdlib_random.py"
- name: run cpython unittest
run: wasmer run --dir "$(pwd)" target/wasm32-wasip1/release/rustpython.wasm -- "$(pwd)/Lib/test/test_int.py"

cargo-shear:
name: cargo shear
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
with:
persist-credentials: false

- uses: cargo-bins/cargo-binstall@113a77a4ce971c41332f2129c3d995df993cf746 # v1.17.8

- run: cargo binstall --no-confirm cargo-shear

- run: cargo shear

security-lint:
runs-on: ubuntu-latest
permissions:
security-events: write
steps:
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
with:
persist-credentials: false

- name: Run zizmor
uses: zizmorcore/zizmor-action@71321a20a9ded102f6e9ce5718a2fcec2c4f70d8 # v0.5.2
74 changes: 0 additions & 74 deletions .github/workflows/pr-format.yaml
View file
Open in desktop

This file was deleted.

71 changes: 71 additions & 0 deletions .pre-commit-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
# NOTE: Reason for not using `prek.toml` is dependabot supports `pre-commit` as an ecosystem
# See: https://github.blog/changelog/2026-03-10-dependabot-now-supports-pre-commit-hooks/

fail_fast: false
repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v6.0.0
hooks:
- id: check-merge-conflict
priority: 0

- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.15.7
hooks:
- id: ruff-format
priority: 0

- id: ruff-check
args: [--select, I, --fix, --exit-non-zero-on-fix]
types_or: [python]
require_serial: true
priority: 1

- repo: local
hooks:
- id: redundant-test-patches
name: check redundant test patches
entry: scripts/check_redundant_patches.py
files: '^Lib/test/.*\.py$'
language: script
types: [python]
priority: 0

- repo: local
hooks:
- id: rustfmt
name: rustfmt
entry: rustfmt
language: system
types: [rust]
priority: 0

- id: generate-opcode-metadata
name: generate opcode metadata
entry: python scripts/generate_opcode_metadata.py
files: '^(crates/compiler-core/src/bytecode/instruction\.rs|scripts/generate_opcode_metadata\.py)$'
pass_filenames: false
language: system
require_serial: true
priority: 1 # so rustfmt runs first

- repo: https://github.com/streetsidesoftware/cspell-cli
rev: v9.7.0
hooks:
- id: cspell
types: [rust]
additional_dependencies:
- '@cspell/dict-en_us'
- '@cspell/dict-cpp'
- '@cspell/dict-python'
- '@cspell/dict-rust'
- '@cspell/dict-win32'
- '@cspell/dict-shell'
priority: 0

- repo: https://github.com/rbubley/mirrors-prettier
rev: v3.8.1
hooks:
- id: prettier
files: '^wasm/.*$'
priority: 0
9 changes: 2 additions & 7 deletions example_projects/wasm32_without_js/rustpython-without-js/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use rustpython_vm::{Interpreter};
use rustpython_vm::Interpreter;

unsafe extern "C" {
fn kv_get(kp: i32, kl: i32, vp: i32, vl: i32) -> i32;
Expand Down Expand Up @@ -37,12 +37,7 @@ pub unsafe extern "C" fn eval(s: *const u8, l: usize) -> i32 {

let msg = format!("eval result: {result}");

unsafe {
print(
msg.as_str().as_ptr() as usize as i32,
msg.len() as i32,
)
};
unsafe { print(msg.as_str().as_ptr() as usize as i32, msg.len() as i32) };

0
}
Expand Down
Loading
X Tutup