Skip to content

Commit a349080

Browse files
committed
Check Sphinx warnings and fail if improved
1 parent 70e2a42 commit a349080

File tree

5 files changed

+159
-110
lines changed

5 files changed

+159
-110
lines changed

.github/workflows/reusable-docs.yml

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -28,35 +28,28 @@ jobs:
2828
cache-dependency-path: 'Doc/requirements.txt'
2929
- name: 'Install build dependencies'
3030
run: make -C Doc/ venv
31-
- name: 'Build HTML documentation'
32-
run: make -C Doc/ SPHINXOPTS="-q" SPHINXERRORHANDLING="-W --keep-going" html
3331

34-
# Add pull request annotations for Sphinx nitpicks (missing references)
32+
# To annotate PRs with Sphinx nitpicks (missing references)
3533
- name: 'Get list of changed files'
3634
if: github.event_name == 'pull_request'
3735
id: changed_files
3836
uses: Ana06/[email protected]
3937
with:
4038
filter: "Doc/**"
4139
format: csv # works for paths with spaces
42-
- name: 'Build changed files in nit-picky mode'
43-
if: github.event_name == 'pull_request'
40+
- name: 'Build HTML documentation'
4441
continue-on-error: true
4542
run: |
4643
set -Eeuo pipefail
47-
# Mark files the pull request modified
48-
python Doc/tools/touch-clean-files.py --clean '${{ steps.changed_files.outputs.added_modified }}'
49-
# Build docs with the '-n' (nit-picky) option; convert warnings to annotations
50-
make -C Doc/ PYTHON=../python SPHINXOPTS="-q -n --keep-going" html 2>&1 |
51-
python Doc/tools/warnings-to-gh-actions.py
52-
53-
# Ensure some files always pass Sphinx nit-picky mode (no missing references)
54-
- name: 'Build known-good files in nit-picky mode'
44+
# Build docs with the '-n' (nit-picky) option; write warnings to file
45+
make -C Doc/ PYTHON=../python SPHINXOPTS="-q -n -W --keep-going -w sphinx-warnings.txt" html
46+
- name: 'Check warnings'
47+
if: github.event_name == 'pull_request'
5548
run: |
56-
# Mark files that must pass nit-picky
57-
python Doc/tools/touch-clean-files.py
58-
# Build docs with the '-n' (nit-picky) option, convert warnings to errors (-W)
59-
make -C Doc/ PYTHON=../python SPHINXOPTS="-q -n -W --keep-going" html 2>&1
49+
python Doc/tools/check-warnings.py \
50+
--check-and-annotate '${{ steps.changed_files.outputs.added_modified }}' \
51+
--fail-if-regression \
52+
--fail-if-improved
6053
6154
# This build doesn't use problem matchers or check annotations
6255
build_doc_oldest_supported_sphinx:

Doc/tools/.nitignore

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
# All RST files under Doc/ -- except these -- must pass Sphinx nit-picky mode,
2-
# as tested on the CI via touch-clean-files.py in doc.yml.
3-
# Add blank lines between files and keep them sorted lexicographically
4-
# to help avoid merge conflicts.
2+
# as tested on the CI via check-warnings.py in reusable-docs.yml.
3+
# Keep lines sorted lexicographically to help avoid merge conflicts.
54

65
Doc/c-api/allocation.rst
76
Doc/c-api/apiabiversion.rst

Doc/tools/check-warnings.py

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
#!/usr/bin/env python3
2+
"""
3+
Check the output of running Sphinx in nit-picky mode (missing references).
4+
"""
5+
import argparse
6+
import csv
7+
import os
8+
import re
9+
import sys
10+
from pathlib import Path
11+
12+
# Exclude these whether they're dirty or clean,
13+
# because they trigger a rebuild of dirty files.
14+
EXCLUDE_FILES = {
15+
"Doc/whatsnew/changelog.rst",
16+
}
17+
18+
# Subdirectories of Doc/ to exclude.
19+
EXCLUDE_SUBDIRS = {
20+
".env",
21+
".venv",
22+
"env",
23+
"includes",
24+
"venv",
25+
}
26+
27+
28+
def check_and_annotate(warnings: list[str], files_to_check: str) -> None:
29+
"""
30+
Convert Sphinx warning messages to GitHub Actions.
31+
32+
Converts lines like:
33+
.../Doc/library/cgi.rst:98: WARNING: reference target not found
34+
to:
35+
::warning file=.../Doc/library/cgi.rst,line=98::reference target not found
36+
37+
Non-matching lines are echoed unchanged.
38+
39+
see:
40+
https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#setting-a-warning-message
41+
"""
42+
pattern = re.compile(r"(?P<file>[^:]+):(?P<line>\d+): WARNING: (?P<msg>.+)")
43+
files_to_check = next(csv.reader([files_to_check]))
44+
for warning in warnings:
45+
if any(filename in warning for filename in files_to_check):
46+
if match := pattern.fullmatch(warning):
47+
print("::warning file={file},line={line}::{msg}".format_map(match))
48+
49+
50+
def fail_if_regression(
51+
warnings: list[str], files_with_expected_nits: set[str], files_with_nits: set[str]
52+
) -> int:
53+
"""
54+
Ensure some files always pass Sphinx nit-picky mode (no missing references).
55+
These are files which are *not* in .nitignore.
56+
"""
57+
all_rst = {
58+
str(rst)
59+
for rst in Path("Doc/").rglob("*.rst")
60+
if rst.parts[1] not in EXCLUDE_SUBDIRS
61+
}
62+
should_be_clean = all_rst - files_with_expected_nits - EXCLUDE_FILES
63+
problem_files = sorted(should_be_clean & files_with_nits)
64+
if problem_files:
65+
print("\nError: must not contain warnings:\n")
66+
for filename in problem_files:
67+
print(filename)
68+
for warning in warnings:
69+
if filename in warning:
70+
print(warning)
71+
return -1
72+
return 0
73+
74+
75+
def fail_if_improved(
76+
files_with_expected_nits: set[str], files_with_nits: set[str]
77+
) -> int:
78+
"""
79+
We may have fixed warnings in some files so that the files are now completely clean.
80+
Good news! Let's add them to .nitignore to prevent regression.
81+
"""
82+
files_with_no_nits = files_with_expected_nits - files_with_nits
83+
if files_with_no_nits:
84+
print("\nCongratulations! You improved:\n")
85+
for filename in sorted(files_with_no_nits):
86+
print(filename)
87+
print("\nPlease remove from Doc/tools/.nitignore\n")
88+
return -1
89+
return 0
90+
91+
92+
def main() -> int:
93+
parser = argparse.ArgumentParser()
94+
parser.add_argument(
95+
"--check-and-annotate",
96+
help="Comma-separated list of files to check, "
97+
"and annotate those with warnings on GitHub Actions",
98+
)
99+
parser.add_argument(
100+
"--fail-if-regression",
101+
action="store_true",
102+
help="Fail if known-good files have warnings",
103+
)
104+
parser.add_argument(
105+
"--fail-if-improved",
106+
action="store_true",
107+
help="Fail if new files with no nits are found",
108+
)
109+
args = parser.parse_args()
110+
exit_code = 0
111+
112+
wrong_directory_msg = "Must run this script from the repo root"
113+
assert Path("Doc").exists() and Path("Doc").is_dir(), wrong_directory_msg
114+
115+
with Path("Doc/sphinx-warnings.txt").open() as f:
116+
warnings = f.read().splitlines()
117+
118+
cwd = str(Path.cwd()) + os.path.sep
119+
files_with_nits = {
120+
warning.removeprefix(cwd).split(":")[0]
121+
for warning in warnings
122+
if "Doc/" in warning
123+
}
124+
125+
with Path("Doc/tools/.nitignore").open() as clean_files:
126+
files_with_expected_nits = {
127+
filename.strip()
128+
for filename in clean_files
129+
if filename.strip() and not filename.startswith("#")
130+
}
131+
132+
if args.check_and_annotate:
133+
check_and_annotate(warnings, args.check_and_annotate)
134+
135+
if args.fail_if_regression:
136+
exit_code += fail_if_regression(
137+
warnings, files_with_expected_nits, files_with_nits
138+
)
139+
140+
if args.fail_if_improved:
141+
exit_code += fail_if_improved(files_with_expected_nits, files_with_nits)
142+
143+
return exit_code
144+
145+
146+
if __name__ == "__main__":
147+
sys.exit(main())

Doc/tools/touch-clean-files.py

Lines changed: 0 additions & 65 deletions
This file was deleted.

Doc/tools/warnings-to-gh-actions.py

Lines changed: 0 additions & 25 deletions
This file was deleted.

0 commit comments

Comments
 (0)