Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 58be1c2

Browse filesBrowse files
nohlsonblurb-it[bot]
nohlson
and
blurb-it[bot]
authored
gh-112301: Add macOS warning tracking tooling (#122211)
Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com>
1 parent 4b66b6b commit 58be1c2
Copy full SHA for 58be1c2

File tree

Expand file treeCollapse file tree

5 files changed

+117
-55
lines changed
Filter options
Expand file treeCollapse file tree

5 files changed

+117
-55
lines changed

‎.github/workflows/reusable-macos.yml

Copy file name to clipboardExpand all lines: .github/workflows/reusable-macos.yml
+3-1Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,10 @@ jobs:
4848
--prefix=/opt/python-dev \
4949
--with-openssl="$(brew --prefix openssl@3.0)"
5050
- name: Build CPython
51-
run: make -j8
51+
run: set -o pipefail; make -j8 2>&1 | tee compiler_output.txt
5252
- name: Display build info
5353
run: make pythoninfo
54+
- name: Check compiler warnings
55+
run: python3 Tools/build/check_warnings.py --compiler-output-file-path=compiler_output.txt --warning-ignore-file-path=Tools/build/.warningignore_macos --compiler-output-type=clang
5456
- name: Tests
5557
run: make test

‎.github/workflows/reusable-ubuntu.yml

Copy file name to clipboardExpand all lines: .github/workflows/reusable-ubuntu.yml
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ jobs:
8080
working-directory: ${{ env.CPYTHON_BUILDDIR }}
8181
run: make pythoninfo
8282
- name: Check compiler warnings
83-
run: python Tools/build/check_warnings.py --compiler-output-file-path=${{ env.CPYTHON_BUILDDIR }}/compiler_output.txt --warning-ignore-file-path ${GITHUB_WORKSPACE}/Tools/build/.warningignore_ubuntu
83+
run: python Tools/build/check_warnings.py --compiler-output-file-path=${{ env.CPYTHON_BUILDDIR }}/compiler_output.txt --warning-ignore-file-path ${GITHUB_WORKSPACE}/Tools/build/.warningignore_ubuntu --compiler-output-type=json
8484
- name: Remount sources writable for tests
8585
# some tests write to srcdir, lack of pyc files slows down testing
8686
run: sudo mount $CPYTHON_RO_SRCDIR -oremount,rw

‎Misc/NEWS.d/next/Security/2024-07-24-05-18-25.gh-issue-112301.lfINgZ.rst

Copy file name to clipboard
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Add macOS warning tracking to warning check tooling.
2+
Patch by Nate Ohlson.

‎Tools/build/.warningignore_macos

Copy file name to clipboard
+3Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
# Files listed will be ignored by the compiler warning checker
2+
# for the macOS/build and test job.
3+
# Keep lines sorted lexicographically to help avoid merge conflicts.

‎Tools/build/check_warnings.py

Copy file name to clipboardExpand all lines: Tools/build/check_warnings.py
+108-53Lines changed: 108 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -2,39 +2,87 @@
22
Parses compiler output with -fdiagnostics-format=json and checks that warnings
33
exist only in files that are expected to have warnings.
44
"""
5+
56
import argparse
7+
from collections import defaultdict
68
import json
79
import re
810
import sys
911
from pathlib import Path
1012

1113

12-
def extract_warnings_from_compiler_output(compiler_output: str) -> list[dict]:
14+
def extract_warnings_from_compiler_output_clang(
15+
compiler_output: str,
16+
) -> list[dict]:
1317
"""
14-
Extracts warnings from the compiler output when using
15-
-fdiagnostics-format=json
18+
Extracts warnings from the compiler output when using clang
19+
"""
20+
# Regex to find warnings in the compiler output
21+
clang_warning_regex = re.compile(
22+
r"(?P<file>.*):(?P<line>\d+):(?P<column>\d+): warning: (?P<message>.*)"
23+
)
24+
compiler_warnings = []
25+
for line in compiler_output.splitlines():
26+
if match := clang_warning_regex.match(line):
27+
compiler_warnings.append(
28+
{
29+
"file": match.group("file"),
30+
"line": match.group("line"),
31+
"column": match.group("column"),
32+
"message": match.group("message"),
33+
}
34+
)
1635

17-
Compiler output as a whole is not a valid json document, but includes many
18-
json objects and may include other output that is not json.
36+
return compiler_warnings
37+
38+
39+
def extract_warnings_from_compiler_output_json(
40+
compiler_output: str,
41+
) -> list[dict]:
1942
"""
43+
Extracts warnings from the compiler output when using
44+
-fdiagnostics-format=json.
2045
46+
Compiler output as a whole is not a valid json document,
47+
but includes many json objects and may include other output
48+
that is not json.
49+
"""
2150
# Regex to find json arrays at the top level of the file
2251
# in the compiler output
2352
json_arrays = re.findall(
24-
r"\[(?:[^\[\]]|\[(?:[^\[\]]|\[[^\[\]]*\])*\])*\]", compiler_output
53+
r"\[(?:[^[\]]|\[[^\]]*\])*\]", compiler_output
2554
)
2655
compiler_warnings = []
2756
for array in json_arrays:
2857
try:
2958
json_data = json.loads(array)
3059
json_objects_in_array = [entry for entry in json_data]
31-
compiler_warnings.extend(
32-
[
33-
entry
34-
for entry in json_objects_in_array
35-
if entry.get("kind") == "warning"
36-
]
37-
)
60+
warning_list = [
61+
entry
62+
for entry in json_objects_in_array
63+
if entry.get("kind") == "warning"
64+
]
65+
for warning in warning_list:
66+
locations = warning["locations"]
67+
for location in locations:
68+
for key in ["caret", "start", "end"]:
69+
if key in location:
70+
compiler_warnings.append(
71+
{
72+
# Remove leading current directory if present
73+
"file": location[key]["file"].lstrip("./"),
74+
"line": location[key]["line"],
75+
"column": location[key]["column"],
76+
"message": warning["message"],
77+
}
78+
)
79+
# Found a caret, start, or end in location so
80+
# break out completely to address next warning
81+
break
82+
else:
83+
continue
84+
break
85+
3886
except json.JSONDecodeError:
3987
continue # Skip malformed JSON
4088

@@ -46,27 +94,16 @@ def get_warnings_by_file(warnings: list[dict]) -> dict[str, list[dict]]:
4694
Returns a dictionary where the key is the file and the data is the warnings
4795
in that file
4896
"""
49-
warnings_by_file = {}
97+
warnings_by_file = defaultdict(list)
5098
for warning in warnings:
51-
locations = warning["locations"]
52-
for location in locations:
53-
for key in ["caret", "start", "end"]:
54-
if key in location:
55-
file = location[key]["file"]
56-
file = file.lstrip(
57-
"./"
58-
) # Remove leading current directory if present
59-
if file not in warnings_by_file:
60-
warnings_by_file[file] = []
61-
warnings_by_file[file].append(warning)
99+
warnings_by_file[warning["file"]].append(warning)
62100

63101
return warnings_by_file
64102

65103

66104
def get_unexpected_warnings(
67-
warnings: list[dict],
68105
files_with_expected_warnings: set[str],
69-
files_with_warnings: set[str],
106+
files_with_warnings: dict[str, list[dict]],
70107
) -> int:
71108
"""
72109
Returns failure status if warnings discovered in list of warnings
@@ -88,13 +125,12 @@ def get_unexpected_warnings(
88125

89126

90127
def get_unexpected_improvements(
91-
warnings: list[dict],
92128
files_with_expected_warnings: set[str],
93-
files_with_warnings: set[str],
129+
files_with_warnings: dict[str, list[dict]],
94130
) -> int:
95131
"""
96-
Returns failure status if there are no warnings in the list of warnings for
97-
a file that is in the list of files with expected warnings
132+
Returns failure status if there are no warnings in the list of warnings
133+
for a file that is in the list of files with expected warnings
98134
"""
99135
unexpected_improvements = []
100136
for file in files_with_expected_warnings:
@@ -123,7 +159,6 @@ def main(argv: list[str] | None = None) -> int:
123159
"-i",
124160
"--warning-ignore-file-path",
125161
type=str,
126-
required=True,
127162
help="Path to the warning ignore file",
128163
)
129164
parser.add_argument(
@@ -141,6 +176,14 @@ def main(argv: list[str] | None = None) -> int:
141176
help="Flag to fail if files that were expected "
142177
"to have warnings have no warnings",
143178
)
179+
parser.add_argument(
180+
"-t",
181+
"--compiler-output-type",
182+
type=str,
183+
required=True,
184+
choices=["json", "clang"],
185+
help="Type of compiler output file (json or clang)",
186+
)
144187

145188
args = parser.parse_args(argv)
146189

@@ -149,44 +192,56 @@ def main(argv: list[str] | None = None) -> int:
149192
# Check that the compiler output file is a valid path
150193
if not Path(args.compiler_output_file_path).is_file():
151194
print(
152-
"Compiler output file does not exist: "
153-
f"{args.compiler_output_file_path}"
195+
f"Compiler output file does not exist:"
196+
f" {args.compiler_output_file_path}"
154197
)
155198
return 1
156199

157-
# Check that the warning ignore file is a valid path
158-
if not Path(args.warning_ignore_file_path).is_file():
200+
# Check that a warning ignore file was specified and if so is a valid path
201+
if not args.warning_ignore_file_path:
159202
print(
160-
"Warning ignore file does not exist: "
161-
f"{args.warning_ignore_file_path}"
203+
"Warning ignore file not specified."
204+
" Continuing without it (no warnings ignored)."
162205
)
163-
return 1
206+
files_with_expected_warnings = set()
207+
else:
208+
if not Path(args.warning_ignore_file_path).is_file():
209+
print(
210+
f"Warning ignore file does not exist:"
211+
f" {args.warning_ignore_file_path}"
212+
)
213+
return 1
214+
with Path(args.warning_ignore_file_path).open(
215+
encoding="UTF-8"
216+
) as clean_files:
217+
files_with_expected_warnings = {
218+
file.strip()
219+
for file in clean_files
220+
if file.strip() and not file.startswith("#")
221+
}
164222

165223
with Path(args.compiler_output_file_path).open(encoding="UTF-8") as f:
166224
compiler_output_file_contents = f.read()
167225

168-
with Path(args.warning_ignore_file_path).open(
169-
encoding="UTF-8"
170-
) as clean_files:
171-
files_with_expected_warnings = {
172-
file.strip()
173-
for file in clean_files
174-
if file.strip() and not file.startswith("#")
175-
}
176-
177-
warnings = extract_warnings_from_compiler_output(
178-
compiler_output_file_contents
179-
)
226+
if args.compiler_output_type == "json":
227+
warnings = extract_warnings_from_compiler_output_json(
228+
compiler_output_file_contents
229+
)
230+
elif args.compiler_output_type == "clang":
231+
warnings = extract_warnings_from_compiler_output_clang(
232+
compiler_output_file_contents
233+
)
234+
180235
files_with_warnings = get_warnings_by_file(warnings)
181236

182237
status = get_unexpected_warnings(
183-
warnings, files_with_expected_warnings, files_with_warnings
238+
files_with_expected_warnings, files_with_warnings
184239
)
185240
if args.fail_on_regression:
186241
exit_code |= status
187242

188243
status = get_unexpected_improvements(
189-
warnings, files_with_expected_warnings, files_with_warnings
244+
files_with_expected_warnings, files_with_warnings
190245
)
191246
if args.fail_on_improvement:
192247
exit_code |= status

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.