From 4b3a3222ef1a76f809fce4ef604cca7b4c8734ed Mon Sep 17 00:00:00 2001 From: Damyan Pepper Date: Tue, 1 Sep 2026 11:47:40 -0700 Subject: [PATCH 1/8] Simplify clang format check Use a read-only pull request workflow. Remove pull request comments and automatic formatting changes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6543b38e-a489-4210-9e73-9a51687c3646 --- .github/workflows/clang-format-checker.yml | 169 ++++-------------- utils/git/code-format-helper.py | 196 --------------------- utils/git/code-format-save-diff.py | 102 ----------- utils/git/requirements_formatting.txt | 34 ---- utils/git/requirements_formatting.txt.in | 1 - 5 files changed, 36 insertions(+), 466 deletions(-) delete mode 100644 utils/git/code-format-helper.py delete mode 100644 utils/git/code-format-save-diff.py delete mode 100644 utils/git/requirements_formatting.txt delete mode 100644 utils/git/requirements_formatting.txt.in diff --git a/.github/workflows/clang-format-checker.yml b/.github/workflows/clang-format-checker.yml index 3e574ca3c8..edd15778ac 100644 --- a/.github/workflows/clang-format-checker.yml +++ b/.github/workflows/clang-format-checker.yml @@ -1,21 +1,25 @@ name: "Check code formatting" -on: - pull_request_target: - types: [opened,synchronize] - issue_comment: - types: edited + +on: + pull_request: + types: [opened, synchronize, reopened] + +permissions: + contents: read + +concurrency: + group: clang-format-${{ github.event.pull_request.number }} + cancel-in-progress: true jobs: code_formatter: - if: github.event_name == 'pull_request_target' runs-on: ubuntu-latest - permissions: - pull-requests: write + timeout-minutes: 30 steps: - name: Fetch DirectXShaderCompiler sources - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 with: - ref: ${{ github.event.pull_request.head.sha }} + persist-credentials: false - name: Checkout through merge base uses: rmacklin/fetch-through-merge-base@bfe4d03a86f9afa52bc1a70e9814fc92a07f7b75 # v0.3.0 @@ -24,139 +28,38 @@ jobs: head_ref: ${{ github.event.pull_request.head.sha }} deepen_length: 500 - - name: Get changed files - id: changed-files - uses: step-security/changed-files@3dbe17c78367e7d60f00d78ae6781a35be47b4a1 # v45.0.1 - with: - separator: "," - skip_initial_fetch: true - - # We need to pull the script from the main branch, so that we ensure - # we get the latest version of this script. - - name: Fetch code formatting utils - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 - with: - repository: microsoft/DirectXShaderCompiler - ref: ${{ github.event.repository.default_branch }} - sparse-checkout: | - utils/git/requirements_formatting.txt - utils/git/code-format-helper.py - utils/git/code-format-save-diff.py - sparse-checkout-cone-mode: false - path: code-format-tools - - - name: "Listed files" - env: - LISTED_FILES: ${{ steps.changed-files.outputs.all_changed_files }} - run: | - echo "Formatting files:" - echo "$LISTED_FILES" - - name: Install clang-format uses: aminya/setup-cpp@8170d66c458f4a045220b7b0966c10940bb2a15d # v1.8.1 with: clangformat: 17.0.1 - - name: Setup Python env - uses: actions/setup-python@7f4fc3e22c37d6ff65e88745f38bd3157c663f7c # v4.9.1 - with: - python-version: '3.11' - cache: 'pip' - cache-dependency-path: 'code-format-tools/utils/git/requirements_formatting.txt' - - - name: Install python dependencies - run: pip install -r code-format-tools/utils/git/requirements_formatting.txt - - - name: Run code formatter - id: formatter + - name: Check code formatting env: - GITHUB_PR_NUMBER: ${{ github.event.pull_request.number }} START_REV: ${{ github.event.pull_request.base.sha }} END_REV: ${{ github.event.pull_request.head.sha }} - CHANGED_FILES: ${{ steps.changed-files.outputs.all_changed_files }} + shell: bash run: | - python code-format-tools/utils/git/code-format-helper.py \ - --token ${{ secrets.GITHUB_TOKEN }} \ - --issue-number $GITHUB_PR_NUMBER \ - --start-rev $START_REV \ - --end-rev $END_REV \ - --changed-files "$CHANGED_FILES" - apply_diff: - if: ${{ github.event_name == 'issue_comment' && github.event.comment.user.login == 'github-actions[bot]' && endsWith(github.event.comment.body, '- [x] Check this box to apply formatting changes to this branch.') }} - runs-on: ubuntu-latest - env: - TMP_DIFF_FILE: /tmp/diff.patch - permissions: - pull-requests: write - contents: write - steps: - - uses: actions/github-script@ffc2c79a5b2490bd33e0a41c1de74b877714d736 # v3.2.0 - id: get-pr - with: - script: | - const request = { - owner: context.repo.owner, - repo: context.repo.repo, - pull_number: context.issue.number - } - core.info(`Getting PR #${request.pull_number} from ${request.owner}/${request.repo}`) - try { - const result = await github.pulls.get(request) - return result.data - } catch (err) { - core.setFailed(`Request failed with error ${err}`) - } - - # We need to pull the script from the main branch, so that we ensure - # we get the latest version of this script. - - name: Fetch code formatting utils - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 - with: - repository: microsoft/DirectXShaderCompiler - ref: ${{ github.event.repository.default_branch }} - sparse-checkout: | - utils/git/requirements_formatting.txt - utils/git/code-format-helper.py - utils/git/code-format-save-diff.py - sparse-checkout-cone-mode: false - path: code-format-tools + set -euo pipefail - - name: Setup Python env - uses: actions/setup-python@7f4fc3e22c37d6ff65e88745f38bd3157c663f7c # v4.9.1 - with: - python-version: '3.11' - cache: 'pip' - cache-dependency-path: 'code-format-tools/utils/git/requirements_formatting.txt' + output_file="$(mktemp)" + trap 'rm -f "$output_file"' EXIT - - name: Install python dependencies - run: pip install -r code-format-tools/utils/git/requirements_formatting.txt + set +e + git-clang-format \ + --diff \ + --extensions=cpp,c,h,hpp,hxx,cxx \ + "$START_REV" \ + "$END_REV" >"$output_file" 2>&1 + formatter_status=$? + set -e - - name: Apply code diff - env: - GITHUB_PR_NUMBER: ${{ github.event.issue.number }} - COMMENT_ID: ${{ github.event.comment.id }} - run: | - python code-format-tools/utils/git/code-format-save-diff.py \ - --token ${{ secrets.GITHUB_TOKEN }} \ - --issue-number $GITHUB_PR_NUMBER \ - --tmp-diff-file $TMP_DIFF_FILE \ - --comment-id $COMMENT_ID + sed 's/^/[clang-format] /' "$output_file" + if [[ "$formatter_status" -gt 1 ]]; then + echo "::error::git-clang-format failed." + exit "$formatter_status" + fi - - name: Fetch LLVM sources for head - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 - with: - fetch-depth: 2 - ref: ${{ fromJSON(steps.get-pr.outputs.result).head.ref }} - repository: ${{ fromJSON(steps.get-pr.outputs.result).head.repo.full_name }} - - - name: apply diff - run: | - git apply $TMP_DIFF_FILE - git add . - - - name: Commit & Push changes - uses: actions-js/push@968f4695ca558093eadb24ad83cc5891f47e0cdc # v1.6 - with: - branch: ${{ fromJSON(steps.get-pr.outputs.result).head.ref }} - repository: ${{ fromJSON(steps.get-pr.outputs.result).head.repo.full_name }} - github_token: ${{ secrets.GITHUB_TOKEN }} + if [[ "$formatter_status" -eq 1 ]] || grep -q '^diff --git ' "$output_file"; then + echo "::error::C/C++ formatting changes are required. Run 'git clang-format $START_REV' locally and update the pull request." + exit 1 + fi diff --git a/utils/git/code-format-helper.py b/utils/git/code-format-helper.py deleted file mode 100644 index f60f7aa3de..0000000000 --- a/utils/git/code-format-helper.py +++ /dev/null @@ -1,196 +0,0 @@ -#!/usr/bin/env python3 -# -# ====- code-format-helper, runs code formatters from the ci --*- python -*--==# -# -# Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -# See https://llvm.org/LICENSE.txt for license information. -# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -# -# ==-------------------------------------------------------------------------==# - -import argparse -import os -import subprocess -import sys -from functools import cached_property - -import github -from github import IssueComment, PullRequest - - -class FormatHelper: - COMMENT_TAG = "" - name = "unknown" - - @property - def comment_tag(self) -> str: - return self.COMMENT_TAG.replace("fmt", self.name) - - def format_run(self, changed_files: [str], args: argparse.Namespace) -> str | None: - pass - - def pr_comment_text(self, diff: str) -> str: - return f""" -{self.comment_tag} - -:warning: {self.friendly_name}, {self.name} found issues in your code. :warning: - -
- -You can test this locally with the following command: - - -``````````bash -{self.instructions} -`````````` - -
- -
- -View the diff from {self.name} here. - - -``````````diff -{diff} -`````````` - -
- -- [ ] Check this box to apply formatting changes to this branch.""" - - def find_comment( - self, pr: PullRequest.PullRequest - ) -> IssueComment.IssueComment | None: - for comment in pr.as_issue().get_comments(): - if self.comment_tag in comment.body: - return comment - return None - - def update_pr(self, diff: str, args: argparse.Namespace): - repo = github.Github(args.token).get_repo(args.repo) - pr = repo.get_issue(args.issue_number).as_pull_request() - - existing_comment = self.find_comment(pr) - pr_text = self.pr_comment_text(diff) - - if existing_comment: - existing_comment.edit(pr_text) - else: - pr.as_issue().create_comment(pr_text) - - def update_pr_success(self, args: argparse.Namespace): - repo = github.Github(args.token).get_repo(args.repo) - pr = repo.get_issue(args.issue_number).as_pull_request() - - existing_comment = self.find_comment(pr) - if existing_comment: - existing_comment.edit( - f""" -{self.comment_tag} -:white_check_mark: With the latest revision this PR passed the {self.friendly_name}. -""" - ) - - def run(self, changed_files: [str], args: argparse.Namespace): - diff = self.format_run(changed_files, args) - if diff: - self.update_pr(diff, args) - return False - else: - self.update_pr_success(args) - return True - - -class ClangFormatHelper(FormatHelper): - name = "clang-format" - friendly_name = "C/C++ code formatter" - - @property - def instructions(self): - return " ".join(self.cf_cmd) - - @cached_property - def libcxx_excluded_files(self): - return [] # HLSL Change - libcxx is not in DXC's repo - #with open("libcxx/utils/data/ignore_format.txt", "r") as ifd: - # return [excl.strip() for excl in ifd.readlines()] - - def should_be_excluded(self, path: str) -> bool: - if path in self.libcxx_excluded_files: - print(f"Excluding file {path}") - return True - return False - - def filter_changed_files(self, changed_files: [str]) -> [str]: - filtered_files = [] - for path in changed_files: - _, ext = os.path.splitext(path) - if ext in (".cpp", ".c", ".h", ".hpp", ".hxx", ".cxx"): - if not self.should_be_excluded(path): - filtered_files.append(path) - return filtered_files - - def format_run(self, changed_files: [str], args: argparse.Namespace) -> str | None: - cpp_files = self.filter_changed_files(changed_files) - if not cpp_files: - return - cf_cmd = [ - "git-clang-format", - "--diff", - args.start_rev, - args.end_rev, - "--", - ] + cpp_files - print(f"Running: {' '.join(cf_cmd)}") - self.cf_cmd = cf_cmd - proc = subprocess.run(cf_cmd, capture_output=True) - - # formatting needed - if proc.returncode == 1: - return proc.stdout.decode("utf-8") - - return None - - -ALL_FORMATTERS = (ClangFormatHelper(),) - -if __name__ == "__main__": - parser = argparse.ArgumentParser() - parser.add_argument( - "--token", type=str, required=True, help="GitHub authentiation token" - ) - parser.add_argument( - "--repo", - type=str, - default=os.getenv("GITHUB_REPOSITORY", "llvm/llvm-project"), - help="The GitHub repository that we are working with in the form of / (e.g. llvm/llvm-project)", - ) - parser.add_argument("--issue-number", type=int, required=True) - parser.add_argument( - "--start-rev", - type=str, - required=True, - help="Compute changes from this revision.", - ) - parser.add_argument( - "--end-rev", type=str, required=True, help="Compute changes to this revision" - ) - parser.add_argument( - "--changed-files", - type=str, - help="Comma separated list of files that has been changed", - ) - - args = parser.parse_args() - - changed_files = [] - if args.changed_files: - changed_files = args.changed_files.split(",") - - exit_code = 0 - for fmt in ALL_FORMATTERS: - if not fmt.run(changed_files, args): - exit_code = 1 - - sys.exit(exit_code) diff --git a/utils/git/code-format-save-diff.py b/utils/git/code-format-save-diff.py deleted file mode 100644 index 8b99780e2f..0000000000 --- a/utils/git/code-format-save-diff.py +++ /dev/null @@ -1,102 +0,0 @@ -#!/usr/bin/env python3 -# -# ====- code-format-save-diff, save diff from comment --*- python -*---------==# -# -# Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -# See https://llvm.org/LICENSE.txt for license information. -# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -# -# ==-------------------------------------------------------------------------==# - -import argparse -import os -import re -import subprocess -import sys -import tempfile -from functools import cached_property - -import github -from github import IssueComment, PullRequest - -LF = '\n' -CRLF = '\r\n' -CR = '\r' - - -COMMENT_TAG = "