Skip to content

Commit 0dc8d90

Browse files
authored
Pull in llvm-project's clang-format check action (microsoft#5834)
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.
1 parent 342163b commit 0dc8d90

File tree

4 files changed

+337
-32
lines changed

4 files changed

+337
-32
lines changed
Lines changed: 48 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,38 +1,54 @@
1-
name: 'PR clang-format checker'
2-
on:
3-
pull_request_target:
4-
types:
5-
- opened
6-
- edited
7-
- synchronize
8-
- reopened
1+
name: "Check code formatting"
2+
on: pull_request_target
3+
permissions:
4+
pull-requests: write
5+
96
jobs:
10-
check-pr-formatting:
11-
permissions:
12-
contents: read
13-
pull-requests: write
14-
checks: write
7+
code_formatter:
158
runs-on: ubuntu-latest
169
steps:
17-
- uses: actions/checkout@v3
10+
- name: Fetch LLVM sources
11+
uses: actions/checkout@v4
1812
with:
19-
fetch-depth: 0
20-
- name: Comparing PR against target branch
13+
fetch-depth: 2
14+
15+
- name: Get changed files
16+
id: changed-files
17+
uses: tj-actions/changed-files@v39
18+
with:
19+
separator: ","
20+
fetch_depth: 100 # Fetches only the last 10 commits
21+
22+
- name: "Listed files"
23+
run: |
24+
echo "Formatting files:"
25+
echo "${{ steps.changed-files.outputs.all_changed_files }}"
26+
27+
- name: Install clang-format
28+
uses: aminya/setup-cpp@v1
29+
with:
30+
clangformat: 17.0.1
31+
32+
- name: Setup Python env
33+
uses: actions/setup-python@v4
34+
with:
35+
python-version: '3.11'
36+
cache: 'pip'
37+
cache-dependency-path: 'utils/git/requirements_formatting.txt'
38+
39+
- name: Install python dependencies
40+
run: pip install -r utils/git/requirements_formatting.txt
41+
42+
- name: Run code formatter
2143
env:
22-
PR_NUMBER: ${{ github.event.number }}
23-
GH_TOKEN: ${{ github.token }}
44+
GITHUB_PR_NUMBER: ${{ github.event.pull_request.number }}
45+
START_REV: ${{ github.event.pull_request.base.sha }}
46+
END_REV: ${{ github.event.pull_request.head.sha }}
47+
CHANGED_FILES: ${{ steps.changed-files.outputs.all_changed_files }}
2448
run: |
25-
echo Comparing $PR_NUMBER vs target branch
26-
git fetch origin refs/pull/$PR_NUMBER/head:pull/$PR_NUMBER
27-
git checkout pull/$PR_NUMBER
28-
git diff -U0 --no-color $GITHUB_SHA..pull/$PR_NUMBER | clang-format-diff-15 -p1 | tee format.diff
29-
if [ -s format.diff ]
30-
then
31-
echo PR contains clang-format violations. First 50 lines of the diff: >> message.txt
32-
echo \`\`\`diff >> message.txt
33-
cat format.diff | head -n 50 >> message.txt
34-
echo \`\`\` >> message.txt
35-
echo See [action log]\(https://github.com/microsoft/DirectXShaderCompiler/actions/runs/$GITHUB_RUN_ID/\) for the full diff. >> message.txt
36-
gh pr comment $PR_NUMBER --body-file message.txt
37-
exit 1
38-
fi
49+
python utils/git/code-format-helper.py \
50+
--token ${{ secrets.GITHUB_TOKEN }} \
51+
--issue-number $GITHUB_PR_NUMBER \
52+
--start-rev $START_REV \
53+
--end-rev $END_REV \
54+
--changed-files "$CHANGED_FILES"

utils/git/code-format-helper.py

Lines changed: 234 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,234 @@
1+
#!/usr/bin/env python3
2+
#
3+
# ====- code-format-helper, runs code formatters from the ci --*- python -*--==#
4+
#
5+
# Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
6+
# See https://llvm.org/LICENSE.txt for license information.
7+
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
8+
#
9+
# ==-------------------------------------------------------------------------==#
10+
11+
import argparse
12+
import os
13+
import subprocess
14+
import sys
15+
from functools import cached_property
16+
17+
import github
18+
from github import IssueComment, PullRequest
19+
20+
21+
class FormatHelper:
22+
COMMENT_TAG = "<!--LLVM CODE FORMAT COMMENT: {fmt}-->"
23+
name = "unknown"
24+
25+
@property
26+
def comment_tag(self) -> str:
27+
return self.COMMENT_TAG.replace("fmt", self.name)
28+
29+
def format_run(self, changed_files: [str], args: argparse.Namespace) -> str | None:
30+
pass
31+
32+
def pr_comment_text(self, diff: str) -> str:
33+
return f"""
34+
{self.comment_tag}
35+
36+
:warning: {self.friendly_name}, {self.name} found issues in your code. :warning:
37+
38+
<details>
39+
<summary>
40+
You can test this locally with the following command:
41+
</summary>
42+
43+
``````````bash
44+
{self.instructions}
45+
``````````
46+
47+
</details>
48+
49+
<details>
50+
<summary>
51+
View the diff from {self.name} here.
52+
</summary>
53+
54+
``````````diff
55+
{diff}
56+
``````````
57+
58+
</details>
59+
"""
60+
61+
def find_comment(
62+
self, pr: PullRequest.PullRequest
63+
) -> IssueComment.IssueComment | None:
64+
for comment in pr.as_issue().get_comments():
65+
if self.comment_tag in comment.body:
66+
return comment
67+
return None
68+
69+
def update_pr(self, diff: str, args: argparse.Namespace):
70+
repo = github.Github(args.token).get_repo(args.repo)
71+
pr = repo.get_issue(args.issue_number).as_pull_request()
72+
73+
existing_comment = self.find_comment(pr)
74+
pr_text = self.pr_comment_text(diff)
75+
76+
if existing_comment:
77+
existing_comment.edit(pr_text)
78+
else:
79+
pr.as_issue().create_comment(pr_text)
80+
81+
def update_pr_success(self, args: argparse.Namespace):
82+
repo = github.Github(args.token).get_repo(args.repo)
83+
pr = repo.get_issue(args.issue_number).as_pull_request()
84+
85+
existing_comment = self.find_comment(pr)
86+
if existing_comment:
87+
existing_comment.edit(
88+
f"""
89+
{self.comment_tag}
90+
:white_check_mark: With the latest revision this PR passed the {self.friendly_name}.
91+
"""
92+
)
93+
94+
def run(self, changed_files: [str], args: argparse.Namespace):
95+
diff = self.format_run(changed_files, args)
96+
if diff:
97+
self.update_pr(diff, args)
98+
return False
99+
else:
100+
self.update_pr_success(args)
101+
return True
102+
103+
104+
class ClangFormatHelper(FormatHelper):
105+
name = "clang-format"
106+
friendly_name = "C/C++ code formatter"
107+
108+
@property
109+
def instructions(self):
110+
return " ".join(self.cf_cmd)
111+
112+
@cached_property
113+
def libcxx_excluded_files(self):
114+
return [] # HLSL Change - libcxx is not in DXC's repo
115+
#with open("libcxx/utils/data/ignore_format.txt", "r") as ifd:
116+
# return [excl.strip() for excl in ifd.readlines()]
117+
118+
def should_be_excluded(self, path: str) -> bool:
119+
if path in self.libcxx_excluded_files:
120+
print(f"Excluding file {path}")
121+
return True
122+
return False
123+
124+
def filter_changed_files(self, changed_files: [str]) -> [str]:
125+
filtered_files = []
126+
for path in changed_files:
127+
_, ext = os.path.splitext(path)
128+
if ext in (".cpp", ".c", ".h", ".hpp", ".hxx", ".cxx"):
129+
if not self.should_be_excluded(path):
130+
filtered_files.append(path)
131+
return filtered_files
132+
133+
def format_run(self, changed_files: [str], args: argparse.Namespace) -> str | None:
134+
cpp_files = self.filter_changed_files(changed_files)
135+
if not cpp_files:
136+
return
137+
cf_cmd = [
138+
"git-clang-format",
139+
"--diff",
140+
args.start_rev,
141+
args.end_rev,
142+
"--",
143+
] + cpp_files
144+
print(f"Running: {' '.join(cf_cmd)}")
145+
self.cf_cmd = cf_cmd
146+
proc = subprocess.run(cf_cmd, capture_output=True)
147+
148+
# formatting needed
149+
if proc.returncode == 1:
150+
return proc.stdout.decode("utf-8")
151+
152+
return None
153+
154+
155+
class DarkerFormatHelper(FormatHelper):
156+
name = "darker"
157+
friendly_name = "Python code formatter"
158+
159+
@property
160+
def instructions(self):
161+
return " ".join(self.darker_cmd)
162+
163+
def filter_changed_files(self, changed_files: [str]) -> [str]:
164+
filtered_files = []
165+
for path in changed_files:
166+
name, ext = os.path.splitext(path)
167+
if ext == ".py":
168+
filtered_files.append(path)
169+
170+
return filtered_files
171+
172+
def format_run(self, changed_files: [str], args: argparse.Namespace) -> str | None:
173+
py_files = self.filter_changed_files(changed_files)
174+
if not py_files:
175+
return
176+
darker_cmd = [
177+
"darker",
178+
"--check",
179+
"--diff",
180+
"-r",
181+
f"{args.start_rev}..{args.end_rev}",
182+
] + py_files
183+
print(f"Running: {' '.join(darker_cmd)}")
184+
self.darker_cmd = darker_cmd
185+
proc = subprocess.run(darker_cmd, capture_output=True)
186+
187+
# formatting needed
188+
if proc.returncode == 1:
189+
return proc.stdout.decode("utf-8")
190+
191+
return None
192+
193+
194+
ALL_FORMATTERS = (DarkerFormatHelper(), ClangFormatHelper())
195+
196+
if __name__ == "__main__":
197+
parser = argparse.ArgumentParser()
198+
parser.add_argument(
199+
"--token", type=str, required=True, help="GitHub authentiation token"
200+
)
201+
parser.add_argument(
202+
"--repo",
203+
type=str,
204+
default=os.getenv("GITHUB_REPOSITORY", "llvm/llvm-project"),
205+
help="The GitHub repository that we are working with in the form of <owner>/<repo> (e.g. llvm/llvm-project)",
206+
)
207+
parser.add_argument("--issue-number", type=int, required=True)
208+
parser.add_argument(
209+
"--start-rev",
210+
type=str,
211+
required=True,
212+
help="Compute changes from this revision.",
213+
)
214+
parser.add_argument(
215+
"--end-rev", type=str, required=True, help="Compute changes to this revision"
216+
)
217+
parser.add_argument(
218+
"--changed-files",
219+
type=str,
220+
help="Comma separated list of files that has been changed",
221+
)
222+
223+
args = parser.parse_args()
224+
225+
changed_files = []
226+
if args.changed_files:
227+
changed_files = args.changed_files.split(",")
228+
229+
exit_code = 0
230+
for fmt in ALL_FORMATTERS:
231+
if not fmt.run(changed_files, args):
232+
exit_code = 1
233+
234+
sys.exit(exit_code)

utils/git/requirements_formatting.txt

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
#
2+
# This file is autogenerated by pip-compile with Python 3.11
3+
# by the following command:
4+
#
5+
# pip-compile --output-file=llvm/utils/git/requirements_formatting.txt llvm/utils/git/requirements_formatting.txt.in
6+
#
7+
black==23.9.1
8+
# via
9+
# -r llvm/utils/git/requirements_formatting.txt.in
10+
# darker
11+
certifi==2023.7.22
12+
# via requests
13+
cffi==1.15.1
14+
# via
15+
# cryptography
16+
# pynacl
17+
charset-normalizer==3.2.0
18+
# via requests
19+
click==8.1.7
20+
# via black
21+
cryptography==41.0.3
22+
# via pyjwt
23+
darker==1.7.2
24+
# via -r llvm/utils/git/requirements_formatting.txt.in
25+
deprecated==1.2.14
26+
# via pygithub
27+
idna==3.4
28+
# via requests
29+
mypy-extensions==1.0.0
30+
# via black
31+
packaging==23.1
32+
# via black
33+
pathspec==0.11.2
34+
# via black
35+
platformdirs==3.10.0
36+
# via black
37+
pycparser==2.21
38+
# via cffi
39+
pygithub==1.59.1
40+
# via -r llvm/utils/git/requirements_formatting.txt.in
41+
pyjwt[crypto]==2.8.0
42+
# via pygithub
43+
pynacl==1.5.0
44+
# via pygithub
45+
requests==2.31.0
46+
# via pygithub
47+
toml==0.10.2
48+
# via darker
49+
urllib3==2.0.4
50+
# via requests
51+
wrapt==1.15.0
52+
# via deprecated
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
black~=23.0
2+
darker==1.7.2
3+
PyGithub==1.59.1

0 commit comments

Comments
 (0)