-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Update CI auto-formate #6237
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.
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
Update CI auto-formate #6237
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -307,15 +307,13 @@ jobs: | |
| run: python -I whats_left.py | ||
|
|
||
| lint: | ||
| name: Check Rust code with rustfmt and clippy | ||
| name: Check Rust code with clippy | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v5 | ||
| - uses: dtolnay/rust-toolchain@stable | ||
| with: | ||
| components: rustfmt, clippy | ||
| - name: run rustfmt | ||
| run: cargo fmt --check | ||
| components: clippy | ||
| - name: run clippy on wasm | ||
| run: cargo clippy --manifest-path=wasm/lib/Cargo.toml -- -Dwarnings | ||
| - uses: actions/setup-python@v6 | ||
|
|
@@ -450,3 +448,55 @@ 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 | ||
|
|
||
| auto_format_commit: | ||
| needs: [rust_tests, exotic_targets, snippets_cpython, lint, miri, wasm, wasm-wasi] | ||
| permissions: | ||
| contents: write | ||
| pull-requests: write | ||
| name: Auto-format code | ||
| runs-on: ubuntu-latest | ||
| if: ${{ !contains(github.event.head_commit.message, '[skip ci]') }} | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainCritical: Condition check fails for pull_request events.
This is why the author reports CI passing on their fork but failing here—depending on how GitHub Actions handles For pull_request events, you need to check the base branch's latest commit or use an alternative approach. Replace the condition with: - if: ${{ !contains(github.event.head_commit.message, '[skip ci]') }}
+ if: ${{ github.event_name != 'pull_request' || !contains(github.event.pull_request.head.sha, 'skip-not-applicable') }}Or, if you want to reliably check the commit message for PRs, you'll need to fetch it explicitly in a step. For now, a safer fallback is: + if: ${{ github.event_name == 'push' || github.event_name == 'pull_request' }}(This allows the job to run on both; the git check inside the step will prevent unnecessary commits.) Alternatively, consider moving the I'll verify the review comment by examining the workflow file and checking GitHub Actions documentation. [/run_scripts] [/web_search] 🏁 Script executed: #!/bin/bash
# Inspect the workflow file around line 459
head -n 470 .github/workflows/ci.yaml | tail -n 30Length of output: 1166 🏁 Script executed: #!/bin/bash
# Show the workflow on/trigger section to see which events activate it
head -n 20 .github/workflows/ci.yamlLength of output: 666 🏁 Script executed: #!/bin/bash
# Find all jobs and their conditionals to understand context
rg -nP "^\s+if:|^\s+jobs:|^\s+[a-z_]+:" .github/workflows/ci.yaml | head -n 50Length of output: 2131 🌐 Web query: 💡 Result: Short answer
Examples
References Critical: The workflow triggers on both Use the same pattern as other jobs in this workflow (lines 111, 176): add a conditional that checks the event type and uses the appropriate context field. For PRs, use - if: ${{ !contains(github.event.head_commit.message, '[skip ci]') }}
+ if: ${{ github.event_name == 'push' || github.event_name != 'push' }}Or safely move the commit message check into the bash step where you always have access to the local git history.
🤖 Prompt for AI Agents |
||
| concurrency: | ||
| group: fmt-${{ github.ref }} | ||
| cancel-in-progress: true | ||
|
|
||
| steps: | ||
| - name: Checkout code | ||
| uses: actions/checkout@v5 | ||
| with: | ||
| fetch-depth: 0 | ||
| ref: ${{ github.head_ref || github.ref_name }} | ||
|
|
||
| - name: Setup Rust | ||
| uses: dtolnay/rust-toolchain@stable | ||
| with: | ||
| components: rustfmt | ||
|
|
||
| - name: Run cargo fmt | ||
| run: | | ||
| echo "Running cargo fmt --all" | ||
| cargo fmt --all | ||
|
|
||
| - name: Commit and push if changes | ||
| id: commit | ||
| run: | | ||
| git config user.name "github-actions[bot]" | ||
| git config user.email "github-actions[bot]@users.noreply.github.com" | ||
| if [ -n "$(git status --porcelain)" ]; then | ||
| git add -u | ||
| git commit -m "Auto-format code [skip ci]" | ||
| git push | ||
| echo "formatted=true" >> $GITHUB_OUTPUT | ||
| else | ||
| echo "formatted=false" >> $GITHUB_OUTPUT | ||
| fi | ||
|
|
||
| - name: Comment on PR if formatting was applied | ||
| if: steps.commit.outputs.formatted == 'true' && github.event_name == 'pull_request' | ||
| uses: marocchino/sticky-pull-request-comment@v2 | ||
| with: | ||
| message: | | ||
| Code has been automatically formatted. | ||
| No action needed. | ||
| the changes were committed with `[skip ci]`. | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,99 @@ | ||||||||||||||||||||||||||||||||||||||||||||||
| name: PR Auto-format | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| # This workflow triggers when a PR is opened/updated | ||||||||||||||||||||||||||||||||||||||||||||||
| on: | ||||||||||||||||||||||||||||||||||||||||||||||
| pull_request_target: | ||||||||||||||||||||||||||||||||||||||||||||||
| types: [opened, synchronize, reopened] | ||||||||||||||||||||||||||||||||||||||||||||||
| branches: | ||||||||||||||||||||||||||||||||||||||||||||||
| - main | ||||||||||||||||||||||||||||||||||||||||||||||
| - release | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| concurrency: | ||||||||||||||||||||||||||||||||||||||||||||||
| group: pr-fmt-${{ github.event.pull_request.number }} | ||||||||||||||||||||||||||||||||||||||||||||||
| cancel-in-progress: true | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| jobs: | ||||||||||||||||||||||||||||||||||||||||||||||
| auto_format: | ||||||||||||||||||||||||||||||||||||||||||||||
| if: | | ||||||||||||||||||||||||||||||||||||||||||||||
| !contains(github.event.pull_request.labels.*.name, 'skip:ci') && | ||||||||||||||||||||||||||||||||||||||||||||||
| !contains(github.event.pull_request.head.sha, '[skip ci]') | ||||||||||||||||||||||||||||||||||||||||||||||
| permissions: | ||||||||||||||||||||||||||||||||||||||||||||||
| contents: write | ||||||||||||||||||||||||||||||||||||||||||||||
| pull-requests: write | ||||||||||||||||||||||||||||||||||||||||||||||
| checks: read | ||||||||||||||||||||||||||||||||||||||||||||||
| runs-on: ubuntu-latest | ||||||||||||||||||||||||||||||||||||||||||||||
| timeout-minutes: 60 | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+16
to
+26
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainAdd guard to skip auto-format for forked PRs. Per the PR comments and static analysis, this workflow fails when triggered by PRs from forked repositories. GitHub Actions running in the base repository cannot push to a fork's branch, even with Add a check to the condition to verify the PR originates from the base repository: auto_format:
if: |
!contains(github.event.pull_request.labels.*.name, 'skip:ci') &&
!contains(github.event.pull_request.head.sha, '[skip ci]') &&
+ github.event.pull_request.head.repo.full_name == github.repositoryThis ensures the job only runs for PRs against the main repository, not forked PRs. 🏁 Script executed: #!/bin/bash
# Check if file exists and examine lines 16-26 plus surrounding context
if [ -f ".github/workflows/pr-auto-commit.yaml" ]; then
echo "=== File exists, showing lines 1-35 ==="
head -35 .github/workflows/pr-auto-commit.yaml
else
echo "=== File does not exist ==="
find . -name "*pr-auto-commit*" -type f 2>/dev/null || echo "No matching files found"
fiLength of output: 924 🏁 Script executed: #!/bin/bash
# Search for any existing references to github.repository or fork-related conditions
if [ -f ".github/workflows/pr-auto-commit.yaml" ]; then
echo "=== Searching for fork/repository guards ==="
grep -n "github.repository\|\.repo\.full_name\|forked" .github/workflows/pr-auto-commit.yaml || echo "No fork guards found"
echo ""
echo "=== Full condition section ==="
sed -n '16,26p' .github/workflows/pr-auto-commit.yaml
fiLength of output: 452 🏁 Script executed: #!/bin/bash
# Check if pull_request_target is used (which would require fork handling)
if [ -f ".github/workflows/pr-auto-commit.yaml" ]; then
echo "=== Checking trigger event ==="
grep -n "on:\|pull_request" .github/workflows/pr-auto-commit.yaml | head -10
fiLength of output: 597 Add guard to skip auto-format for forked PRs. The workflow uses Add a check to the condition to verify the PR originates from the base repository: auto_format:
if: |
!contains(github.event.pull_request.labels.*.name, 'skip:ci') &&
!contains(github.event.pull_request.head.sha, '[skip ci]') &&
+ github.event.pull_request.head.repo.full_name == github.repositoryThis ensures the job only runs for PRs against the main repository, not forked PRs. 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||
| steps: | ||||||||||||||||||||||||||||||||||||||||||||||
| - name: Checkout PR branch | ||||||||||||||||||||||||||||||||||||||||||||||
| uses: actions/checkout@v5 | ||||||||||||||||||||||||||||||||||||||||||||||
| with: | ||||||||||||||||||||||||||||||||||||||||||||||
| ref: ${{ github.event.pull_request.head.ref }} | ||||||||||||||||||||||||||||||||||||||||||||||
| repository: ${{ github.event.pull_request.head.repo.full_name }} | ||||||||||||||||||||||||||||||||||||||||||||||
| token: ${{ secrets.GITHUB_TOKEN }} | ||||||||||||||||||||||||||||||||||||||||||||||
| fetch-depth: 0 | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| # Wait for all PR check runs to complete | ||||||||||||||||||||||||||||||||||||||||||||||
| - name: Wait for all checks to complete | ||||||||||||||||||||||||||||||||||||||||||||||
| uses: poseidon/wait-for-status-checks@v0.6.0 | ||||||||||||||||||||||||||||||||||||||||||||||
| with: | ||||||||||||||||||||||||||||||||||||||||||||||
| token: ${{ secrets.GITHUB_TOKEN }} | ||||||||||||||||||||||||||||||||||||||||||||||
| delay: 60 | ||||||||||||||||||||||||||||||||||||||||||||||
| interval: 30 | ||||||||||||||||||||||||||||||||||||||||||||||
| timeout: 7200 | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| - name: CI completed successfully | ||||||||||||||||||||||||||||||||||||||||||||||
| run: echo "CI workflow completed successfully - proceeding with auto-format" | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| - name: Setup Rust | ||||||||||||||||||||||||||||||||||||||||||||||
| uses: dtolnay/rust-toolchain@stable | ||||||||||||||||||||||||||||||||||||||||||||||
| with: | ||||||||||||||||||||||||||||||||||||||||||||||
| components: rustfmt | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| - name: Run cargo fmt | ||||||||||||||||||||||||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||||||||||||||||||||||||
| echo "Running cargo fmt --all on PR #${{ github.event.pull_request.number }}" | ||||||||||||||||||||||||||||||||||||||||||||||
| cargo fmt --all | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| - name: Check for formatting changes | ||||||||||||||||||||||||||||||||||||||||||||||
| id: check_changes | ||||||||||||||||||||||||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||||||||||||||||||||||||
| if [ -n "$(git status --porcelain)" ]; then | ||||||||||||||||||||||||||||||||||||||||||||||
| echo "has_changes=true" >> $GITHUB_OUTPUT | ||||||||||||||||||||||||||||||||||||||||||||||
| else | ||||||||||||||||||||||||||||||||||||||||||||||
| echo "has_changes=false" >> $GITHUB_OUTPUT | ||||||||||||||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| - name: Commit and push formatting changes | ||||||||||||||||||||||||||||||||||||||||||||||
| if: steps.check_changes.outputs.has_changes == 'true' | ||||||||||||||||||||||||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||||||||||||||||||||||||
| git config user.name "github-actions[bot]" | ||||||||||||||||||||||||||||||||||||||||||||||
| git config user.email "github-actions[bot]@users.noreply.github.com" | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| git add -u | ||||||||||||||||||||||||||||||||||||||||||||||
| git commit -m "Auto-format code [skip ci]" | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| git push origin HEAD:${{ github.event.pull_request.head.ref }} | ||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+67
to
+76
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mitigate script injection risk: pass Line 69 is flagged by actionlint for potential script injection. The Pass the ref through an environment variable instead: - name: Commit and push formatting changes
if: steps.check_changes.outputs.has_changes == 'true'
+ env:
+ HEAD_REF: ${{ github.event.pull_request.head.ref }}
run: |
git config user.name "github-actions[bot]"
git config user.email "github-actions[bot]@users.noreply.github.com"
git add -u
git commit -m "Auto-format code [skip ci]"
- git push origin HEAD:${{ github.event.pull_request.head.ref }}
+ git push origin HEAD:"$HEAD_REF"See GitHub's security guidance. 📝 Committable suggestion
Suggested change
🧰 Tools🪛 actionlint (1.7.8)69-69: "github.event.pull_request.head.ref" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/reference/security/secure-use#good-practices-for-mitigating-script-injection-attacks for more details (expression) 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| - name: Comment on PR | ||||||||||||||||||||||||||||||||||||||||||||||
| if: steps.check_changes.outputs.has_changes == 'true' | ||||||||||||||||||||||||||||||||||||||||||||||
| uses: marocchino/sticky-pull-request-comment@v2 | ||||||||||||||||||||||||||||||||||||||||||||||
| with: | ||||||||||||||||||||||||||||||||||||||||||||||
| number: ${{ github.event.pull_request.number }} | ||||||||||||||||||||||||||||||||||||||||||||||
| message: | | ||||||||||||||||||||||||||||||||||||||||||||||
| **Code has been automatically formatted** | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| The code in this PR has been formatted using `cargo fmt`. | ||||||||||||||||||||||||||||||||||||||||||||||
| The changes have been committed with `[skip ci]` to avoid triggering another CI run. | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| **Triggered by commit:** `${{ github.event.pull_request.head.sha }}` | ||||||||||||||||||||||||||||||||||||||||||||||
| **Last formatted:** ${{ github.event.pull_request.updated_at }} | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| You may need to pull the latest changes before pushing again: | ||||||||||||||||||||||||||||||||||||||||||||||
| ```bash | ||||||||||||||||||||||||||||||||||||||||||||||
| git pull origin ${{ github.event.pull_request.head.ref }} | ||||||||||||||||||||||||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| - name: No formatting needed | ||||||||||||||||||||||||||||||||||||||||||||||
| if: steps.check_changes.outputs.has_changes == 'false' | ||||||||||||||||||||||||||||||||||||||||||||||
| run: echo "Code is already properly formatted" | ||||||||||||||||||||||||||||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.