Skip to content

Commit daed54d

Browse files
CAM-Gerlachhugovk
andauthored
[3.12] gh-101100: Docs: Check Sphinx warnings and fail if improved (GH-106460) (#108116)
* gh-101100: Docs: Check Sphinx warnings and fail if improved (#106460) (cherry picked from commit 806d7c9) * [3.12] gh-101100: Docs: Check Sphinx warnings and fail if improved (GH-106460). (cherry picked from commit 806d7c9) Co-authored-by: Hugo van Kemenade <[email protected]> --------- Co-authored-by: Hugo van Kemenade <[email protected]>
1 parent 359cff5 commit daed54d

File tree

5 files changed

+161
-113
lines changed

5 files changed

+161
-113
lines changed

.github/workflows/reusable-docs.yml

+10-17
Original file line numberDiff line numberDiff line change
@@ -26,35 +26,28 @@ jobs:
2626
cache-dependency-path: 'Doc/requirements.txt'
2727
- name: 'Install build dependencies'
2828
run: make -C Doc/ venv
29-
- name: 'Build HTML documentation'
30-
run: make -C Doc/ SPHINXOPTS="-q" SPHINXERRORHANDLING="-W --keep-going" html
3129

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

Doc/tools/.nitignore

+2-6
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,14 @@
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/arg.rst
76
Doc/c-api/datetime.rst
87
Doc/c-api/descriptor.rst
9-
Doc/c-api/dict.rst
108
Doc/c-api/exceptions.rst
119
Doc/c-api/file.rst
1210
Doc/c-api/float.rst
1311
Doc/c-api/gcsupport.rst
14-
Doc/c-api/import.rst
1512
Doc/c-api/init.rst
1613
Doc/c-api/init_config.rst
1714
Doc/c-api/intro.rst
@@ -58,7 +55,6 @@ Doc/library/bz2.rst
5855
Doc/library/calendar.rst
5956
Doc/library/cgi.rst
6057
Doc/library/chunk.rst
61-
Doc/library/cmath.rst
6258
Doc/library/cmd.rst
6359
Doc/library/codecs.rst
6460
Doc/library/collections.abc.rst

Doc/tools/check-warnings.py

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

Doc/tools/touch-clean-files.py

-65
This file was deleted.

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

-25
This file was deleted.

0 commit comments

Comments
 (0)