Skip to content

Commit ee2b68f

Browse files
mcsprhasenradball
authored andcommitted
CI - restyle script summary and annotations in PRs (esp8266#9184)
similar to .ino builder, prepare a short 'diff' summary of all the required changes https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#adding-a-job-summary provide inline annotations, so it is apparent clang-format job is the cause of the PR actions check failure https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#setting-a-notice-message
1 parent be0d742 commit ee2b68f

File tree

5 files changed

+487
-50
lines changed

5 files changed

+487
-50
lines changed

.github/workflows/style-check.yml

+1
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ jobs:
2626
- name: Style check
2727
run: |
2828
sudo apt update
29+
python ./tests/test_restyle.py --quiet
2930
bash ./tests/ci/style_check.sh
3031
3132
# Validate orthography

tests/host/common/ArduinoCatch.hpp

+5-3
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,15 @@
2424

2525
std::ostream& operator<<(std::ostream&, const String&);
2626

27-
namespace Catch {
27+
namespace Catch
28+
{
2829

2930
std::string toString(const String&);
3031

3132
template<>
32-
struct StringMaker<String> {
33+
struct StringMaker<String>
34+
{
3335
static std::string convert(const String&);
3436
};
3537

36-
} // namespace Catch
38+
} // namespace Catch

tests/restyle.py

+290
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,290 @@
1+
#!/usr/bin/env python
2+
3+
import argparse
4+
import os
5+
import sys
6+
import pathlib
7+
import subprocess
8+
import contextlib
9+
10+
from dataclasses import dataclass
11+
12+
13+
GIT_ROOT = pathlib.Path(
14+
subprocess.check_output(
15+
["git", "rev-parse", "--show-toplevel"], universal_newlines=True
16+
).strip()
17+
)
18+
19+
20+
def clang_format(clang_format, config, files):
21+
if not files:
22+
raise ValueError("Files list cannot be empty")
23+
24+
cmd = [clang_format, "--verbose", f"--style=file:{config.as_posix()}", "-i"]
25+
cmd.extend(files)
26+
27+
subprocess.run(cmd, check=True)
28+
29+
30+
def ls_files(patterns):
31+
"""Git-only search, but rather poor at matching complex patterns (at least w/ <=py3.12)"""
32+
proc = subprocess.run(
33+
["git", "--no-pager", "ls-files"],
34+
capture_output=True,
35+
check=True,
36+
universal_newlines=True,
37+
)
38+
39+
out = []
40+
for line in proc.stdout.split("\n"):
41+
path = pathlib.Path(line.strip())
42+
if any(path.match(pattern) for pattern in patterns):
43+
out.append(path)
44+
45+
return out
46+
47+
48+
def diff_lines():
49+
proc = subprocess.run(
50+
["git", "--no-pager", "diff", "--ignore-submodules"],
51+
capture_output=True,
52+
check=True,
53+
universal_newlines=True,
54+
)
55+
56+
return proc.stdout.split("\n")
57+
58+
59+
def find_files(patterns):
60+
"""Filesystem search, matches both git and non-git files"""
61+
return [
62+
file
63+
for pattern in patterns
64+
for file in [found for found in GIT_ROOT.rglob(pattern)]
65+
]
66+
67+
68+
def find_core_files():
69+
"""Returns a subset of Core files that should be formatted"""
70+
return [
71+
file
72+
for file in find_files(
73+
(
74+
"cores/esp8266/Lwip*",
75+
"libraries/ESP8266mDNS/**/*",
76+
"libraries/Wire/**/*",
77+
"libraries/lwIP*/**/*",
78+
"cores/esp8266/debug*",
79+
"cores/esp8266/core_esp8266_si2c*",
80+
"cores/esp8266/StreamString*",
81+
"cores/esp8266/StreamSend*",
82+
"libraries/Netdump/**/*",
83+
"tests/**/*",
84+
)
85+
)
86+
if file.is_file()
87+
and file.suffix in (".c", ".cpp", ".h", ".hpp")
88+
and not GIT_ROOT / "tests/host/bin" in file.parents
89+
and not GIT_ROOT / "tests/host/common/catch.hpp" == file
90+
]
91+
92+
93+
def find_arduino_files():
94+
"""Returns every .ino file available in the repository, excluding submodule ones"""
95+
return [
96+
ino
97+
for library in find_files(("libraries/*",))
98+
if library.is_dir() and not (library / ".git").exists()
99+
for ino in library.rglob("**/*.ino")
100+
]
101+
102+
103+
FILES_PRESETS = {
104+
"core": find_core_files,
105+
"arduino": find_arduino_files,
106+
}
107+
108+
109+
@dataclass
110+
class Changed:
111+
file: str
112+
hunk: str
113+
lines: list[int]
114+
115+
116+
class Context:
117+
def __init__(self):
118+
self.append_hunk = False
119+
self.deleted = False
120+
self.file = ""
121+
self.hunk = []
122+
self.markers = []
123+
124+
def reset(self):
125+
self.__init__()
126+
127+
def reset_with_line(self, line):
128+
self.reset()
129+
self.hunk.append(line)
130+
131+
def pop(self, out, line):
132+
if self.file and self.hunk and self.markers:
133+
out.append(
134+
Changed(file=self.file, hunk="\n".join(self.hunk), lines=self.markers)
135+
)
136+
137+
self.reset_with_line(line)
138+
139+
140+
def changed_files_for_diff(lines: list[str] | str) -> list[Changed]:
141+
"""
142+
Naive git-diff output parser. Generates list of objects for every file changed after clang-format.
143+
"""
144+
match lines:
145+
case str():
146+
lines = lines.split("\n")
147+
case list():
148+
pass
149+
case _:
150+
raise ValueError("Unknown 'lines' type, can be either list[str] or str")
151+
152+
ctx = Context()
153+
out = []
154+
155+
# TODO: pygit2?
156+
# ref. https://github.com/cpp-linter/cpp-linter/blob/main/cpp_linter/git/__init__.py ::parse_diff
157+
# ref. https://github.com/libgit2/pygit2/blob/master/src/diff.c ::parse_diff
158+
for line in lines:
159+
# '--- a/path/to/changed/file' most likely
160+
# '--- /dev/null' aka created file. should be ignored, same as removed ones
161+
if line.startswith("---"):
162+
ctx.pop(out, line)
163+
164+
_, file = line.split(" ")
165+
ctx.deleted = "/dev/null" in file
166+
167+
# '+++ b/path/to/changed/file' most likely
168+
# '+++ /dev/null' aka removed file
169+
elif not ctx.deleted and line.startswith("+++"):
170+
ctx.hunk.append(line)
171+
172+
_, file = line.split(" ")
173+
ctx.deleted = "/dev/null" in file
174+
if not ctx.deleted:
175+
ctx.file = file[2:]
176+
177+
# @@ from-file-line-numbers to-file-line-numbers @@
178+
elif not ctx.deleted and line.startswith("@@"):
179+
ctx.hunk.append(line)
180+
181+
_, _, numbers, _ = line.split(" ", 3)
182+
if "," in numbers:
183+
numbers, _ = numbers.split(",") # drop count
184+
185+
numbers = numbers.replace("+", "")
186+
numbers = numbers.replace("-", "")
187+
188+
ctx.markers.append(int(numbers))
189+
ctx.append_hunk = True
190+
191+
# capture diff for the summary
192+
elif ctx.append_hunk and line.startswith(("+", "-", " ")):
193+
ctx.hunk.append(line)
194+
195+
ctx.pop(out, line)
196+
197+
return out
198+
199+
200+
def changed_files() -> list[Changed]:
201+
return changed_files_for_diff(diff_lines())
202+
203+
204+
def errors_changed(changed: Changed):
205+
all_lines = ", ".join(str(x) for x in changed.lines)
206+
for line in changed.lines:
207+
print(
208+
f"::error file={changed.file},title=Run tests/restyle.sh and re-commit {changed.file},line={line}::File {changed.file} failed clang-format style check. (lines {all_lines})"
209+
)
210+
211+
212+
SUMMARY_PATH = pathlib.Path(os.environ.get("GITHUB_STEP_SUMMARY", os.devnull))
213+
SUMMARY_OUTPUT = SUMMARY_PATH.open("a")
214+
215+
216+
def summary_diff(changed: Changed):
217+
with contextlib.redirect_stdout(SUMMARY_OUTPUT):
218+
print(f"# {changed.file} (suggested change)")
219+
print("```diff")
220+
print(changed.hunk)
221+
print("```")
222+
223+
224+
def stdout_diff():
225+
subprocess.run(["git", "--no-pager", "diff", "--ignore-submodules"])
226+
227+
228+
def assert_unchanged():
229+
subprocess.run(
230+
["git", "diff", "--ignore-submodules", "--exit-code"],
231+
check=True,
232+
stdout=subprocess.DEVNULL,
233+
)
234+
235+
236+
def run_format(args):
237+
targets = []
238+
239+
for include in args.include:
240+
targets.append(
241+
(GIT_ROOT / f"tests/clang-format-{include}.yaml", FILES_PRESETS[include]())
242+
)
243+
244+
if not targets:
245+
targets.append((args.config, args.files))
246+
247+
for target in targets:
248+
clang_format(args.clang_format, *target)
249+
250+
251+
def run_assert(args):
252+
for changed in changed_files():
253+
if args.with_errors:
254+
errors_changed(changed)
255+
if args.with_summary:
256+
summary_diff(changed)
257+
258+
if args.with_diff:
259+
stdout_diff()
260+
261+
assert_unchanged()
262+
263+
264+
if __name__ == "__main__":
265+
parser = argparse.ArgumentParser()
266+
267+
cmd = parser.add_subparsers(required=True)
268+
format_ = cmd.add_parser("format")
269+
format_.set_defaults(func=run_format)
270+
format_.add_argument("--clang-format", default="clang-format")
271+
272+
fmt = format_.add_subparsers(required=True)
273+
274+
preset = fmt.add_parser("preset")
275+
preset.add_argument(
276+
"--include", action="append", required=True, choices=tuple(FILES_PRESETS.keys())
277+
)
278+
279+
files = fmt.add_parser("files")
280+
files.add_argument("--config", type=pathlib.Path, required=True)
281+
files.add_argument("files", type=pathlib.Path, nargs="+")
282+
283+
assert_ = cmd.add_parser("assert")
284+
assert_.set_defaults(func=run_assert)
285+
assert_.add_argument("--with-diff", action="store_true")
286+
assert_.add_argument("--with-errors", action="store_true")
287+
assert_.add_argument("--with-summary", action="store_true")
288+
289+
args = parser.parse_args()
290+
args.func(args)

tests/restyle.sh

+9-47
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#!/bin/sh
2-
# requires clang-format, git, python3 with pyyaml
2+
# requires python3, git, and runnable clang-format (specified below)
33

44
set -e -x
55

@@ -11,51 +11,13 @@ test -d ${root}/libraries
1111
# default to v15, latest stable version from ubuntu-latest Github Actions image
1212
CLANG_FORMAT=${CLANG_FORMAT:-clang-format-15}
1313

14-
#########################################
15-
# 'all' variable should be "cores/esp8266 libraries"
16-
17-
all=${1:-"
18-
cores/esp8266/Lwip*
19-
libraries/ESP8266mDNS
20-
libraries/Wire
21-
libraries/lwIP*
22-
cores/esp8266/debug*
23-
cores/esp8266/core_esp8266_si2c.cpp
24-
cores/esp8266/StreamString.*
25-
cores/esp8266/StreamSend.*
26-
libraries/Netdump
27-
tests
28-
"}
29-
30-
#########################################
31-
# restyling core & libraries
32-
3314
cd $root
34-
35-
style=${root}/tests/clang-format-core.yaml
36-
for target in $all; do
37-
if [ -d "$target" ]; then
38-
find $target \
39-
'(' -name "*.cpp" -o -name "*.c" -o -name "*.h" ')' \
40-
-exec $CLANG_FORMAT --verbose --style="file:$style" -i {} \;
41-
else
42-
$CLANG_FORMAT --verbose --style="file:$style" -i $target
43-
fi
44-
done
45-
46-
#########################################
47-
# restyling arduino examples
48-
49-
# TODO should not be matched, these are formatted externally
50-
# exclude=$(git submodule --quiet foreach git rev-parse --show-toplevel | grep libraries)
51-
52-
if [ -z $1 ] ; then
53-
style=${root}/tests/clang-format-arduino.yaml
54-
find libraries \
55-
-path libraries/ESP8266SdFat -prune -o \
56-
-path libraries/Ethernet -prune -o \
57-
-path libraries/SoftwareSerial -prune -o \
58-
-name '*.ino' -exec $CLANG_FORMAT --verbose --style="file:$style" -i {} \;
15+
python $root/tests/restyle.py format --clang-format=$CLANG_FORMAT preset --include core --include arduino
16+
17+
if [ $CI = "true" ] ; then
18+
echo foo
19+
python $root/tests/restyle.py assert --with-summary --with-errors
20+
else
21+
echo bar
22+
python $root/tests/restyle.py assert --with-diff
5923
fi
60-
61-
#########################################

0 commit comments

Comments
 (0)