Skip to content

Infra: Check Sphinx warnings on CI #3213

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/CODEOWNERS
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
Makefile @AA-Turner
requirements.txt @AA-Turner
infra/ @ewdurbin
tools/ @hugovk

pep_sphinx_extensions/ @AA-Turner
AUTHOR_OVERRIDES.csv @AA-Turner
Expand Down
19 changes: 18 additions & 1 deletion .github/workflows/render.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,24 @@ jobs:
python -m pip install --upgrade pip

- name: 🔧 Render PEPs
run: make dirhtml JOBS=$(nproc)
run: make dirhtml JOBS=$(nproc) SPHINXOPTS="-n -w sphinx-warnings.txt"

# To annotate PRs with Sphinx nitpicks (missing references)
- name: 🗄️Get list of changed files
if: (github.event_name == 'pull_request') && (matrix.python-version == '3.x')
id: changed_files
uses: Ana06/[email protected]
with:
filter: "pep-*.*"
format: csv

- name: 🔎 Check warnings
if: (github.event_name == 'pull_request') && (matrix.python-version == '3.x')
run: |
python tools/check-warnings.py \
--check-and-annotate '${{ steps.changed_files.outputs.added_modified }}' \
--fail-if-regression \
--fail-if-improved

# remove the .doctrees folder when building for deployment as it takes two thirds of disk space
- name: 🔥 Clean up files
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ pep-0000.txt
pep-0000.rst
pep-????.html
peps.rss
sphinx-warnings.txt
__pycache__
*.pyc
*.pyo
Expand Down
44 changes: 44 additions & 0 deletions tools/.nitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
# All PEP files -- except these -- must pass Sphinx nit-picky mode,
# as tested on the CI via check-warnings.py in render.yml.
# Keep lines sorted lexicographically to help avoid merge conflicts.

pep-0232.txt
pep-0262.txt
pep-0277.txt
pep-0292.txt
pep-0299.txt
pep-0323.txt
pep-0326.txt
pep-0329.txt
pep-0336.txt
pep-0340.txt
pep-0343.txt
pep-0355.txt
pep-0361.txt
pep-0370.txt
pep-0371.txt
pep-0419.txt
pep-0425.txt
pep-0448.txt
pep-0480.txt
pep-0488.txt
pep-0491.txt
pep-0492.txt
pep-0502.txt
pep-0505.rst
pep-0545.txt
pep-0571.rst
pep-0572.rst
pep-0617.rst
pep-0622.rst
pep-0644.rst
pep-0645.rst
pep-0654.rst
pep-3100.txt
pep-3104.txt
pep-3114.txt
pep-3115.txt
pep-3119.txt
pep-3135.txt
pep-3145.txt
pep-3147.txt
132 changes: 132 additions & 0 deletions tools/check-warnings.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
"""
Check the output of running Sphinx in nit-picky mode (missing references).
"""
import argparse
import csv
import os
import re
import sys
from pathlib import Path

PATTERN = re.compile(r"(?P<file>[^:]+):(?P<line>\d+): WARNING: (?P<msg>.+)")


def check_and_annotate(warnings: list[str], files_to_check: str) -> None:
"""
Convert Sphinx warning messages to GitHub Actions.

Converts lines like:
.../Doc/library/cgi.rst:98: WARNING: reference target not found
to:
::warning file=.../Doc/library/cgi.rst,line=98::reference target not found

Non-matching lines are echoed unchanged.

see:
https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#setting-a-warning-message
"""
files_to_check = next(csv.reader([files_to_check]))
for warning in warnings:
if any(filename in warning for filename in files_to_check):
if match := PATTERN.fullmatch(warning):
print("::warning file={file},line={line}::{msg}".format_map(match))


def fail_if_regression(
warnings: list[str], files_with_expected_nits: set[str], files_with_nits: set[str]
) -> int:
"""
Ensure some files always pass Sphinx nit-picky mode (no missing references).
These are files which are *not* in .nitignore.
"""
all_rst = {
str(rst)
for rst in Path(".").glob("pep-*.*")
if rst.suffix in (".rst", ".txt")
}
should_be_clean = all_rst - files_with_expected_nits
problem_files = sorted(should_be_clean & files_with_nits)
if problem_files:
print("\nError: must not contain warnings:\n")
for filename in problem_files:
print(filename)
for warning in warnings:
if filename in warning:
if match := PATTERN.fullmatch(warning):
print(" {line}: {msg}".format_map(match))
return -1
return 0


def fail_if_improved(
files_with_expected_nits: set[str], files_with_nits: set[str]
) -> int:
"""
We may have fixed warnings in some files so that the files are now completely clean.
Good news! Let's add them to .nitignore to prevent regression.
"""
files_with_no_nits = files_with_expected_nits - files_with_nits
if files_with_no_nits:
print("\nCongratulations! You improved:\n")
for filename in sorted(files_with_no_nits):
print(filename)
print("\nPlease remove from Doc/tools/.nitignore\n")
return -1
return 0


def main() -> int:
parser = argparse.ArgumentParser()
parser.add_argument(
"--check-and-annotate",
help="Comma-separated list of files to check, "
"and annotate those with warnings on GitHub Actions",
)
parser.add_argument(
"--fail-if-regression",
action="store_true",
help="Fail if known-good files have warnings",
)
parser.add_argument(
"--fail-if-improved",
action="store_true",
help="Fail if new files with no nits are found",
)
args = parser.parse_args()
exit_code = 0

wrong_directory_msg = "Must run this script from the repo root"
assert Path("tools").exists() and Path("tools").is_dir(), wrong_directory_msg

with Path("sphinx-warnings.txt").open() as f:
warnings = f.read().splitlines()

cwd = str(Path.cwd()) + os.path.sep
files_with_nits = {
warning.removeprefix(cwd).split(":")[0]
for warning in warnings
}

with Path("tools/.nitignore").open() as clean_files:
files_with_expected_nits = {
filename.strip()
for filename in clean_files
if filename.strip() and not filename.startswith("#")
}

if args.check_and_annotate:
check_and_annotate(warnings, args.check_and_annotate)

if args.fail_if_regression:
exit_code += fail_if_regression(
warnings, files_with_expected_nits, files_with_nits
)

if args.fail_if_improved:
exit_code += fail_if_improved(files_with_expected_nits, files_with_nits)

return exit_code


if __name__ == "__main__":
sys.exit(main())