Skip to content

Commit

Permalink
Pull in llvm-project's clang-format check action
Browse files Browse the repository at this point in the history
This action and scripting was written by @tru for LLVM and addresses
some of the false positives we've been seeing in our action. There is
more room for improvement, but it would be better to unify than to have
divergent approaches since we're solving the same problem.
  • Loading branch information
llvm-beanz committed Oct 6, 2023
1 parent 342163b commit 51ff0ac
Show file tree
Hide file tree
Showing 4 changed files with 336 additions and 32 deletions.
80 changes: 48 additions & 32 deletions .github/workflows/clang-format-checker.yml
Original file line number Diff line number Diff line change
@@ -1,38 +1,54 @@
name: 'PR clang-format checker'
on:
pull_request_target:
types:
- opened
- edited
- synchronize
- reopened
name: "Check code formatting"
on: pull_request_target
permissions:
pull-requests: write

jobs:
check-pr-formatting:
permissions:
contents: read
pull-requests: write
checks: write
code_formatter:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- name: Fetch LLVM sources
uses: actions/checkout@v4
with:
fetch-depth: 0
- name: Comparing PR against target branch
fetch-depth: 2

- name: Get changed files
id: changed-files
uses: tj-actions/changed-files@v39
with:
separator: ","
fetch_depth: 100 # Fetches only the last 10 commits

- name: "Listed files"
run: |
echo "Formatting files:"
echo "${{ steps.changed-files.outputs.all_changed_files }}"
- name: Install clang-format
uses: aminya/setup-cpp@v1
with:
clangformat: 17.0.1

- name: Setup Python env
uses: actions/setup-python@v4
with:
python-version: '3.11'
cache: 'pip'
cache-dependency-path: 'utils/git/requirements_formatting.txt'

- name: Install python dependencies
run: pip install -r utils/git/requirements_formatting.txt

- name: Run code formatter
env:
PR_NUMBER: ${{ github.event.number }}
GH_TOKEN: ${{ github.token }}
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 }}
run: |
echo Comparing $PR_NUMBER vs target branch
git fetch origin refs/pull/$PR_NUMBER/head:pull/$PR_NUMBER
git checkout pull/$PR_NUMBER
git diff -U0 --no-color $GITHUB_SHA..pull/$PR_NUMBER | clang-format-diff-15 -p1 | tee format.diff
if [ -s format.diff ]
then
echo PR contains clang-format violations. First 50 lines of the diff: >> message.txt
echo \`\`\`diff >> message.txt
cat format.diff | head -n 50 >> message.txt
echo \`\`\` >> message.txt
echo See [action log]\(https://github.com/microsoft/DirectXShaderCompiler/actions/runs/$GITHUB_RUN_ID/\) for the full diff. >> message.txt
gh pr comment $PR_NUMBER --body-file message.txt
exit 1
fi
python 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"
233 changes: 233 additions & 0 deletions utils/git/code-format-helper.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,233 @@
#!/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 = "<!--LLVM CODE FORMAT COMMENT: {fmt}-->"
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:
<details>
<summary>
You can test this locally with the following command:
</summary>
``````````bash
{self.instructions}
``````````
</details>
<details>
<summary>
View the diff from {self.name} here.
</summary>
``````````diff
{diff}
``````````
</details>
"""

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):
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


class DarkerFormatHelper(FormatHelper):
name = "darker"
friendly_name = "Python code formatter"

@property
def instructions(self):
return " ".join(self.darker_cmd)

def filter_changed_files(self, changed_files: [str]) -> [str]:
filtered_files = []
for path in changed_files:
name, ext = os.path.splitext(path)
if ext == ".py":
filtered_files.append(path)

return filtered_files

def format_run(self, changed_files: [str], args: argparse.Namespace) -> str | None:
py_files = self.filter_changed_files(changed_files)
if not py_files:
return
darker_cmd = [
"darker",
"--check",
"--diff",
"-r",
f"{args.start_rev}..{args.end_rev}",
] + py_files
print(f"Running: {' '.join(darker_cmd)}")
self.darker_cmd = darker_cmd
proc = subprocess.run(darker_cmd, capture_output=True)

# formatting needed
if proc.returncode == 1:
return proc.stdout.decode("utf-8")

return None


ALL_FORMATTERS = (DarkerFormatHelper(), 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 <owner>/<repo> (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)
52 changes: 52 additions & 0 deletions utils/git/requirements_formatting.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
#
# This file is autogenerated by pip-compile with Python 3.11
# by the following command:
#
# pip-compile --output-file=llvm/utils/git/requirements_formatting.txt llvm/utils/git/requirements_formatting.txt.in
#
black==23.9.1
# via
# -r llvm/utils/git/requirements_formatting.txt.in
# darker
certifi==2023.7.22
# via requests
cffi==1.15.1
# via
# cryptography
# pynacl
charset-normalizer==3.2.0
# via requests
click==8.1.7
# via black
cryptography==41.0.3
# via pyjwt
darker==1.7.2
# via -r llvm/utils/git/requirements_formatting.txt.in
deprecated==1.2.14
# via pygithub
idna==3.4
# via requests
mypy-extensions==1.0.0
# via black
packaging==23.1
# via black
pathspec==0.11.2
# via black
platformdirs==3.10.0
# via black
pycparser==2.21
# via cffi
pygithub==1.59.1
# via -r llvm/utils/git/requirements_formatting.txt.in
pyjwt[crypto]==2.8.0
# via pygithub
pynacl==1.5.0
# via pygithub
requests==2.31.0
# via pygithub
toml==0.10.2
# via darker
urllib3==2.0.4
# via requests
wrapt==1.15.0
# via deprecated
3 changes: 3 additions & 0 deletions utils/git/requirements_formatting.txt.in
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
black~=23.0
darker==1.7.2
PyGithub==1.59.1

0 comments on commit 51ff0ac

Please sign in to comment.