-
Notifications
You must be signed in to change notification settings - Fork 811
[CI] Enable clang-tidy in precommit #21234
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
base: sycl
Are you sure you want to change the base?
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 |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| name: clang-tidy | ||
|
|
||
| on: | ||
| workflow_call: | ||
|
|
||
| permissions: read-all | ||
|
|
||
| jobs: | ||
| run-clang-tidy: | ||
| runs-on: [Linux, build] | ||
| container: | ||
| image: ghcr.io/intel/llvm/sycl_ubuntu2404_nightly:latest | ||
| options: -u 1001:1001 | ||
| steps: | ||
| - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 | ||
| with: | ||
| sparse-checkout: | | ||
| devops/actions | ||
| devops/scripts | ||
|
Comment on lines
+15
to
+19
Check warningCode scanning / zizmor credential persistence through GitHub Actions artifacts Warning
credential persistence through GitHub Actions artifacts
|
||
| - name: Register cleanup after job is finished | ||
| uses: ./devops/actions/cleanup | ||
|
|
||
| - name: Check diff | ||
| id: should_run_tidy | ||
| run: | | ||
| echo "Downloading the diff" | ||
| curl -L "${{ github.server_url }}/${{ github.repository }}/pull/${{ github.event.pull_request.number }}.diff" -o pr.diff | ||
|
|
||
| if python3 devops/scripts/should_run_clang-tidy.py pr.diff; then | ||
| echo "run=true" >> "$GITHUB_OUTPUT" | ||
| else | ||
| echo "run=false" >> "$GITHUB_OUTPUT" | ||
| fi | ||
|
|
||
| - if: steps.should_run_tidy.outputs.run == 'true' | ||
| uses: ./devops/actions/cached_checkout | ||
| with: | ||
| path: src | ||
| ref: ${{ github.sha }} | ||
| cache_path: "/__w/repo_cache/" | ||
| - name: Configure | ||
| if: steps.should_run_tidy.outputs.run == 'true' | ||
| env: | ||
| CC: gcc | ||
| CXX: g++ | ||
| CUDA_LIB_PATH: "/usr/local/cuda/lib64/stubs" | ||
| run: | | ||
| mkdir -p $GITHUB_WORKSPACE/build | ||
| cd $GITHUB_WORKSPACE/build | ||
| python3 $GITHUB_WORKSPACE/src/buildbot/configure.py -w $GITHUB_WORKSPACE \ | ||
| -s $GITHUB_WORKSPACE/src -o $GITHUB_WORKSPACE/build -t Release \ | ||
| --ci-defaults -DCMAKE_EXPORT_COMPILE_COMMANDS=ON | ||
|
|
||
| - name: Preprocess compile_commands.json | ||
| if: steps.should_run_tidy.outputs.run == 'true' | ||
| run: | | ||
| # Remove commands containing "-D__INTEL_PREVIEW_BREAKING_CHANGES" to avoid running on the same file twice. | ||
| jq '[ .[] | select(.command | contains("-D__INTEL_PREVIEW_BREAKING_CHANGES") | not) ]' $GITHUB_WORKSPACE/build/compile_commands.json > $GITHUB_WORKSPACE/build/compile_commands.temp.json | ||
|
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. Could we just pass |
||
| mv $GITHUB_WORKSPACE/build/compile_commands.temp.json $GITHUB_WORKSPACE/build/compile_commands.json | ||
|
|
||
| # Remove gcc-specific flags | ||
| perl -0777 -pe ' | ||
| s/(?:^|[ \t])\-Wno-class-memaccess(?=$|[ \t])//g; | ||
|
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. Instead of doing this could we just use clang, even just some system package manager installed one, to run |
||
| s/(?:^|[ \t])\-Wno-dangling-reference(?=$|[ \t])//g; | ||
| s/(?:^|[ \t])\-Wno-stringop-overread(?=$|[ \t])//g; | ||
| s/[ \t]{2,}/ /g; | ||
| ' -i $GITHUB_WORKSPACE/build/compile_commands.json | ||
|
|
||
| - name: Run clang-tidy on modified files | ||
| if: steps.should_run_tidy.outputs.run == 'true' | ||
| # Exeprimental workflow, it won't affect the pre-commit status in case of failure. | ||
| continue-on-error: true | ||
| run: | | ||
| cd "$GITHUB_WORKSPACE/src" | ||
| python3 "$GITHUB_WORKSPACE/src/clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py" \ | ||
| -clang-tidy-binary "/opt/sycl/bin/clang-tidy" \ | ||
| -p 1 \ | ||
| -path "$GITHUB_WORKSPACE/build" \ | ||
| -checks "clang-analyzer-*,bugprone-*,performance-*,-bugprone-std-namespace-modification" \ | ||
| < "$GITHUB_WORKSPACE/pr.diff" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| #!/usr/bin/env python3 | ||
|
|
||
| # This script checks if a diff contains changes that should be inspected by | ||
| # clang-tidy. | ||
|
|
||
| from __future__ import annotations | ||
| import re | ||
| import sys | ||
| from typing import Iterator | ||
|
|
||
| EXCLUDE_PATH_RE = re.compile(r"(?:^|/)(test|test-e2e|unittests)(?:/|$)") | ||
|
|
||
|
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. nit: two lines inbetween instead of one in a few places |
||
|
|
||
| def is_relevant_path(path: str) -> bool: | ||
| return EXCLUDE_PATH_RE.search(path) is None | ||
|
|
||
|
|
||
| # This iterator splits a diff file into separate sections like: | ||
| # diff --git a/path-to/file b/path-to/file | ||
| # ... code changes ... | ||
| def iter_diff_sections(diff_content: str) -> Iterator[str]: | ||
| section_lines: list[str] = [] | ||
| started = False | ||
|
|
||
| for line in diff_content.splitlines(True): # keep '\n' | ||
| if line.startswith("diff --git "): | ||
| if started and section_lines: | ||
| yield "".join(section_lines) | ||
| section_lines = [] | ||
| started = True | ||
|
|
||
| if started: | ||
| section_lines.append(line) | ||
|
|
||
| if started and section_lines: | ||
| yield "".join(section_lines) | ||
|
|
||
|
|
||
| def main() -> int: | ||
| diff_path = sys.argv[1] | ||
| with open(diff_path, "r", encoding="utf-8", errors="replace") as f: | ||
| diff_content = f.read() | ||
|
|
||
| should_run = False | ||
| for section in iter_diff_sections(diff_content): | ||
| lines = section.splitlines() | ||
| # Skip removed files. | ||
| if lines[4] == "+++ /dev/null": | ||
| continue | ||
| result_file = lines[3] | ||
| # Skip non-c++ files. | ||
| if not result_file.endswith((".cpp", ".hpp", ".h")): | ||
| continue | ||
| # Skip tests etc. | ||
| if not is_relevant_path(result_file): | ||
| continue | ||
| # Check if any non-comment string was added. | ||
| for line in lines[4:]: | ||
| if line.startswith("+") and not line[1:].lstrip().startswith("//"): | ||
| should_run = True | ||
| break | ||
| if should_run == True: | ||
| break | ||
|
|
||
| sys.exit(0 if should_run else 1) | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| sys.exit(main()) | ||
Check failure
Code scanning / zizmor
unpinned image references Error