From 62ceddb1bc11ac59ab7225498fb45c1371aeadda Mon Sep 17 00:00:00 2001 From: Hugo van Kemenade Date: Thu, 27 Jul 2023 17:40:39 +0300 Subject: [PATCH 1/5] Check Sphinx warnings --- .github/CODEOWNERS | 1 + .github/workflows/render.yml | 19 ++++- .gitignore | 1 + tools/.nitignore | 54 ++++++++++++++ tools/check-warnings.py | 138 +++++++++++++++++++++++++++++++++++ 5 files changed, 212 insertions(+), 1 deletion(-) create mode 100644 tools/.nitignore create mode 100644 tools/check-warnings.py diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 95cf05c26a7..724f1ad1a08 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..9bf624bd0c3 --- /dev/null +++ b/tools/.nitignore @@ -0,0 +1,54 @@ +# 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-0394.txt +pep-0400.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-0544.txt +pep-0545.txt +pep-0553.rst +pep-0554.rst +pep-0557.rst +pep-0571.rst +pep-0572.rst +pep-0617.rst +pep-0622.rst +pep-0636.rst +pep-0642.rst +pep-0644.rst +pep-0645.rst +pep-0649.rst +pep-0654.rst +pep-0697.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..28b2ae530fc --- /dev/null +++ b/tools/check-warnings.py @@ -0,0 +1,138 @@ +""" +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", +} + +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 - 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: + 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()) From ddef2ca1d59499b892af161e609ac37deb97ac65 Mon Sep 17 00:00:00 2001 From: Hugo van Kemenade Date: Thu, 27 Jul 2023 18:06:23 +0300 Subject: [PATCH 2/5] Remove pep-0554.rst from .nitignore --- tools/.nitignore | 1 - 1 file changed, 1 deletion(-) diff --git a/tools/.nitignore b/tools/.nitignore index 9bf624bd0c3..9c836168562 100644 --- a/tools/.nitignore +++ b/tools/.nitignore @@ -31,7 +31,6 @@ pep-0505.rst pep-0544.txt pep-0545.txt pep-0553.rst -pep-0554.rst pep-0557.rst pep-0571.rst pep-0572.rst From 94e41396ec1f083cbbc5be091c5852f7ac8818d9 Mon Sep 17 00:00:00 2001 From: Hugo van Kemenade Date: Sat, 29 Jul 2023 11:12:42 +0300 Subject: [PATCH 3/5] EXCLUDE_FILES not required for this repo --- tools/check-warnings.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/tools/check-warnings.py b/tools/check-warnings.py index 28b2ae530fc..860f29d06c6 100644 --- a/tools/check-warnings.py +++ b/tools/check-warnings.py @@ -8,12 +8,6 @@ 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", -} - PATTERN = re.compile(r"(?P[^:]+):(?P\d+): WARNING: (?P.+)") @@ -50,7 +44,7 @@ def fail_if_regression( for rst in Path(".").glob("pep-*.*") if rst.suffix in (".rst", ".txt") } - should_be_clean = all_rst - files_with_expected_nits - EXCLUDE_FILES + 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") From 3c83bdfb49288af32e1201c0f457ab41dd71f781 Mon Sep 17 00:00:00 2001 From: Hugo van Kemenade Date: Mon, 31 Jul 2023 11:41:12 -0600 Subject: [PATCH 4/5] Remove cleaned files from .nitignore Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com> --- tools/.nitignore | 8 -------- 1 file changed, 8 deletions(-) diff --git a/tools/.nitignore b/tools/.nitignore index 9c836168562..39c13827729 100644 --- a/tools/.nitignore +++ b/tools/.nitignore @@ -17,8 +17,6 @@ pep-0355.txt pep-0361.txt pep-0370.txt pep-0371.txt -pep-0394.txt -pep-0400.txt pep-0419.txt pep-0425.txt pep-0448.txt @@ -28,19 +26,13 @@ pep-0491.txt pep-0492.txt pep-0502.txt pep-0505.rst -pep-0544.txt pep-0545.txt -pep-0553.rst -pep-0557.rst pep-0571.rst pep-0572.rst pep-0617.rst pep-0622.rst -pep-0636.rst -pep-0642.rst pep-0644.rst pep-0645.rst -pep-0649.rst pep-0654.rst pep-0697.rst pep-3100.txt From 338fc72e13a591ce6fad3108115a9a5f9e3874d5 Mon Sep 17 00:00:00 2001 From: Hugo van Kemenade Date: Mon, 31 Jul 2023 11:44:16 -0600 Subject: [PATCH 5/5] Remove cleaned files from .nitignore Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com> --- tools/.nitignore | 1 - 1 file changed, 1 deletion(-) diff --git a/tools/.nitignore b/tools/.nitignore index 39c13827729..c254d78a4f4 100644 --- a/tools/.nitignore +++ b/tools/.nitignore @@ -34,7 +34,6 @@ pep-0622.rst pep-0644.rst pep-0645.rst pep-0654.rst -pep-0697.rst pep-3100.txt pep-3104.txt pep-3114.txt