diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index a41ddc148ce..403feb3dff1 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -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 diff --git a/.github/workflows/render.yml b/.github/workflows/render.yml index e331b15e544..7d6418607a3 100644 --- a/.github/workflows/render.yml +++ b/.github/workflows/render.yml @@ -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/get-changed-files@v2.2.0 + 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 diff --git a/.gitignore b/.gitignore index ae1196cb165..5d1002df567 100644 --- a/.gitignore +++ b/.gitignore @@ -3,6 +3,7 @@ pep-0000.txt pep-0000.rst pep-????.html peps.rss +sphinx-warnings.txt __pycache__ *.pyc *.pyo diff --git a/tools/.nitignore b/tools/.nitignore new file mode 100644 index 00000000000..c254d78a4f4 --- /dev/null +++ b/tools/.nitignore @@ -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 diff --git a/tools/check-warnings.py b/tools/check-warnings.py new file mode 100644 index 00000000000..860f29d06c6 --- /dev/null +++ b/tools/check-warnings.py @@ -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[^:]+):(?P\d+): WARNING: (?P.+)") + + +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())