Skip to content

Commit

Permalink
Check Sphinx warnings and fail if improved
Browse files Browse the repository at this point in the history
  • Loading branch information
hugovk committed Jul 5, 2023
1 parent 70e2a42 commit a349080
Show file tree
Hide file tree
Showing 5 changed files with 159 additions and 110 deletions.
27 changes: 10 additions & 17 deletions .github/workflows/reusable-docs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,35 +28,28 @@ jobs:
cache-dependency-path: 'Doc/requirements.txt'
- name: 'Install build dependencies'
run: make -C Doc/ venv
- name: 'Build HTML documentation'
run: make -C Doc/ SPHINXOPTS="-q" SPHINXERRORHANDLING="-W --keep-going" html

# Add pull request annotations for Sphinx nitpicks (missing references)
# To annotate PRs with Sphinx nitpicks (missing references)
- name: 'Get list of changed files'
if: github.event_name == 'pull_request'
id: changed_files
uses: Ana06/get-changed-files@v2.2.0
with:
filter: "Doc/**"
format: csv # works for paths with spaces
- name: 'Build changed files in nit-picky mode'
if: github.event_name == 'pull_request'
- name: 'Build HTML documentation'
continue-on-error: true
run: |
set -Eeuo pipefail
# Mark files the pull request modified
python Doc/tools/touch-clean-files.py --clean '${{ steps.changed_files.outputs.added_modified }}'
# Build docs with the '-n' (nit-picky) option; convert warnings to annotations
make -C Doc/ PYTHON=../python SPHINXOPTS="-q -n --keep-going" html 2>&1 |
python Doc/tools/warnings-to-gh-actions.py
# Ensure some files always pass Sphinx nit-picky mode (no missing references)
- name: 'Build known-good files in nit-picky mode'
# Build docs with the '-n' (nit-picky) option; write warnings to file
make -C Doc/ PYTHON=../python SPHINXOPTS="-q -n -W --keep-going -w sphinx-warnings.txt" html
- name: 'Check warnings'
if: github.event_name == 'pull_request'
run: |
# Mark files that must pass nit-picky
python Doc/tools/touch-clean-files.py
# Build docs with the '-n' (nit-picky) option, convert warnings to errors (-W)
make -C Doc/ PYTHON=../python SPHINXOPTS="-q -n -W --keep-going" html 2>&1
python Doc/tools/check-warnings.py \
--check-and-annotate '${{ steps.changed_files.outputs.added_modified }}' \
--fail-if-regression \
--fail-if-improved
# This build doesn't use problem matchers or check annotations
build_doc_oldest_supported_sphinx:
Expand Down
5 changes: 2 additions & 3 deletions Doc/tools/.nitignore
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# All RST files under Doc/ -- except these -- must pass Sphinx nit-picky mode,
# as tested on the CI via touch-clean-files.py in doc.yml.
# Add blank lines between files and keep them sorted lexicographically
# to help avoid merge conflicts.
# as tested on the CI via check-warnings.py in reusable-docs.yml.
# Keep lines sorted lexicographically to help avoid merge conflicts.

Doc/c-api/allocation.rst
Doc/c-api/apiabiversion.rst
Expand Down
147 changes: 147 additions & 0 deletions Doc/tools/check-warnings.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
#!/usr/bin/env python3
"""
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

# Exclude these whether they're dirty or clean,
# because they trigger a rebuild of dirty files.
EXCLUDE_FILES = {
"Doc/whatsnew/changelog.rst",
}

# Subdirectories of Doc/ to exclude.
EXCLUDE_SUBDIRS = {
".env",
".venv",
"env",
"includes",
"venv",
}


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
"""
pattern = re.compile(r"(?P<file>[^:]+):(?P<line>\d+): WARNING: (?P<msg>.+)")
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("Doc/").rglob("*.rst")
if rst.parts[1] not in EXCLUDE_SUBDIRS
}
should_be_clean = all_rst - files_with_expected_nits - EXCLUDE_FILES
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:
print(warning)
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("Doc").exists() and Path("Doc").is_dir(), wrong_directory_msg

with Path("Doc/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
if "Doc/" in warning
}

with Path("Doc/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())
65 changes: 0 additions & 65 deletions Doc/tools/touch-clean-files.py

This file was deleted.

25 changes: 0 additions & 25 deletions Doc/tools/warnings-to-gh-actions.py

This file was deleted.

0 comments on commit a349080

Please sign in to comment.