From dff437b42ca53f884444e87f90ba3118e26b5517 Mon Sep 17 00:00:00 2001 From: Hugo van Kemenade Date: Wed, 20 Sep 2023 18:20:07 +0300 Subject: [PATCH 01/11] Run pre-commit as part of 'make patchcheck' --- Makefile.pre.in | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Makefile.pre.in b/Makefile.pre.in index d123fa3e6f4a47..af2b0b17363078 100644 --- a/Makefile.pre.in +++ b/Makefile.pre.in @@ -2772,6 +2772,8 @@ funny: .PHONY: patchcheck patchcheck: all $(RUNSHARED) ./$(BUILDPYTHON) $(srcdir)/Tools/patchcheck/patchcheck.py + @$(RUNSHARED) ./$(BUILDPYTHON) -m pre_commit --version >/dev/null 2>&1 || $(RUNSHARED) ./$(BUILDPYTHON) -m pip install pre-commit >/dev/null 2>&1 + $(RUNSHARED) ./$(BUILDPYTHON) -m pre_commit run --all-files .PHONY: check-limited-abi check-limited-abi: all From 97c6ccfe5061a0bd90edb8a8c0f248feefa58dcd Mon Sep 17 00:00:00 2001 From: Hugo van Kemenade Date: Wed, 20 Sep 2023 18:21:03 +0300 Subject: [PATCH 02/11] Use f-strings --- Tools/patchcheck/patchcheck.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Tools/patchcheck/patchcheck.py b/Tools/patchcheck/patchcheck.py index fa3a43af6e6048..c0b280364bada7 100755 --- a/Tools/patchcheck/patchcheck.py +++ b/Tools/patchcheck/patchcheck.py @@ -23,7 +23,7 @@ def n_files_str(count): """Return 'N file(s)' with the proper plurality on 'file'.""" - return "{} file{}".format(count, "s" if count != 1 else "") + return f"{count} file{'s' if count != 1 else ''}" def status(message, modal=False, info=None): @@ -77,7 +77,7 @@ def get_git_remote_default_branch(remote_name): It is typically called 'main', but may differ """ - cmd = "git remote show {}".format(remote_name).split() + cmd = f"git remote show {remote_name}".split() env = os.environ.copy() env['LANG'] = 'C' try: @@ -164,9 +164,9 @@ def report_modified_files(file_paths): if count == 0: return n_files_str(count) else: - lines = ["{}:".format(n_files_str(count))] + lines = [f"{n_files_str(count)}:"] for path in file_paths: - lines.append(" {}".format(path)) + lines.append(f" {path}") return "\n".join(lines) @@ -222,7 +222,7 @@ def normalize_docs_whitespace(file_paths): f.writelines(new_lines) fixed.append(path) except Exception as err: - print('Cannot fix %s: %s' % (path, err)) + print(f'Cannot fix {path}: {err}') return fixed From 029a9a4c16cda106cacd0fc70177880f84469c5f Mon Sep 17 00:00:00 2001 From: Hugo van Kemenade Date: Wed, 20 Sep 2023 18:21:42 +0300 Subject: [PATCH 03/11] Add whitespace --- Tools/patchcheck/patchcheck.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Tools/patchcheck/patchcheck.py b/Tools/patchcheck/patchcheck.py index c0b280364bada7..be099733dcebe0 100755 --- a/Tools/patchcheck/patchcheck.py +++ b/Tools/patchcheck/patchcheck.py @@ -207,6 +207,7 @@ def normalize_c_whitespace(file_paths): ws_re = re.compile(br'\s+(\r?\n)$') + @status("Fixing docs whitespace", info=report_modified_files) def normalize_docs_whitespace(file_paths): fixed = [] @@ -244,6 +245,7 @@ def reported_news(file_paths): return any(p.startswith(os.path.join('Misc', 'NEWS.d', 'next')) for p in file_paths) + @status("configure regenerated", modal=True, info=str) def regenerated_configure(file_paths): """Check if configure has been regenerated.""" @@ -252,6 +254,7 @@ def regenerated_configure(file_paths): else: return "not needed" + @status("pyconfig.h.in regenerated", modal=True, info=str) def regenerated_pyconfig_h_in(file_paths): """Check if pyconfig.h.in has been regenerated.""" @@ -260,6 +263,7 @@ def regenerated_pyconfig_h_in(file_paths): else: return "not needed" + def ci(pull_request): if pull_request == 'false': print('Not a pull request; skipping') @@ -281,6 +285,7 @@ def ci(pull_request): print('(on UNIX you can run `make patchcheck` to make the fixes)') sys.exit(1) + def main(): base_branch = get_base_branch() file_paths = changed_files(base_branch) From d7690a3588a7f32117a730c35a2697fa9cc17688 Mon Sep 17 00:00:00 2001 From: Hugo van Kemenade Date: Wed, 20 Sep 2023 18:22:08 +0300 Subject: [PATCH 04/11] Properly pluralise file(s), fix Unix typo --- Tools/patchcheck/patchcheck.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Tools/patchcheck/patchcheck.py b/Tools/patchcheck/patchcheck.py index be099733dcebe0..6e6d3cc749c3be 100755 --- a/Tools/patchcheck/patchcheck.py +++ b/Tools/patchcheck/patchcheck.py @@ -281,8 +281,9 @@ def ci(pull_request): if not fixed: print('No whitespace issues found') else: - print(f'Please fix the {len(fixed)} file(s) with whitespace issues') - print('(on UNIX you can run `make patchcheck` to make the fixes)') + count = len(fixed) + print(f'Please fix the {count} {n_files_str(count)} with whitespace issues') + print('(on Unix you can run `make patchcheck` to make the fixes)') sys.exit(1) From 5cfcd19a69f2f63dcad5e4be31cec478efad3e63 Mon Sep 17 00:00:00 2001 From: Hugo van Kemenade Date: Wed, 20 Sep 2023 18:47:55 +0300 Subject: [PATCH 05/11] Remove normalize_docs_whitespace from patchcheck, it's covered by pre-commit's trailing-whitespace --- Tools/patchcheck/patchcheck.py | 31 ------------------------------- 1 file changed, 31 deletions(-) diff --git a/Tools/patchcheck/patchcheck.py b/Tools/patchcheck/patchcheck.py index 6e6d3cc749c3be..7c5c6e2c18949e 100755 --- a/Tools/patchcheck/patchcheck.py +++ b/Tools/patchcheck/patchcheck.py @@ -205,28 +205,6 @@ def normalize_c_whitespace(file_paths): return fixed -ws_re = re.compile(br'\s+(\r?\n)$') - - -@status("Fixing docs whitespace", info=report_modified_files) -def normalize_docs_whitespace(file_paths): - fixed = [] - for path in file_paths: - abspath = os.path.join(SRCDIR, path) - try: - with open(abspath, 'rb') as f: - lines = f.readlines() - new_lines = [ws_re.sub(br'\1', line) for line in lines] - if new_lines != lines: - shutil.copyfile(abspath, abspath + '.bak') - with open(abspath, 'wb') as f: - f.writelines(new_lines) - fixed.append(path) - except Exception as err: - print(f'Cannot fix {path}: {err}') - return fixed - - @status("Docs modified", modal=True) def docs_modified(file_paths): """Report if any file in the Doc directory has been changed.""" @@ -272,12 +250,9 @@ def ci(pull_request): file_paths = changed_files(base_branch) python_files = [fn for fn in file_paths if fn.endswith('.py')] c_files = [fn for fn in file_paths if fn.endswith(('.c', '.h'))] - doc_files = [fn for fn in file_paths if fn.startswith('Doc') and - fn.endswith(('.rst', '.inc'))] fixed = [] fixed.extend(normalize_whitespace(python_files)) fixed.extend(normalize_c_whitespace(c_files)) - fixed.extend(normalize_docs_whitespace(doc_files)) if not fixed: print('No whitespace issues found') else: @@ -292,17 +267,11 @@ def main(): file_paths = changed_files(base_branch) python_files = [fn for fn in file_paths if fn.endswith('.py')] c_files = [fn for fn in file_paths if fn.endswith(('.c', '.h'))] - doc_files = [fn for fn in file_paths if fn.startswith('Doc') and - fn.endswith(('.rst', '.inc'))] misc_files = {p for p in file_paths if p.startswith('Misc')} # PEP 8 whitespace rules enforcement. normalize_whitespace(python_files) # C rules enforcement. normalize_c_whitespace(c_files) - # Doc whitespace enforcement. - normalize_docs_whitespace(doc_files) - # Docs updated. - docs_modified(doc_files) # Misc/ACKS changed. credit_given(misc_files) # Misc/NEWS changed. From 1eb8042bcd2f21dfdd1f7acc291e3edb45037e94 Mon Sep 17 00:00:00 2001 From: Hugo van Kemenade Date: Wed, 20 Sep 2023 18:48:53 +0300 Subject: [PATCH 06/11] Include inc files in pre-commit's trailing-whitespace --- .pre-commit-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 4c1fd20ea921b8..cd993d7b3d8b86 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -17,7 +17,7 @@ repos: types: [python] exclude: Lib/test/tokenizedata/coding20731.py - id: trailing-whitespace - types_or: [c, python, rst] + types_or: [c, inc, python, rst] - repo: https://github.com/sphinx-contrib/sphinx-lint rev: v0.6.8 From a85cd1c27ecde5e4e4d33afcae094b855b06a27e Mon Sep 17 00:00:00 2001 From: Hugo van Kemenade Date: Mon, 25 Sep 2023 19:41:50 +0300 Subject: [PATCH 07/11] Fix string Co-authored-by: Alex Waygood --- Tools/patchcheck/patchcheck.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tools/patchcheck/patchcheck.py b/Tools/patchcheck/patchcheck.py index 7c5c6e2c18949e..a0320d9c50416c 100755 --- a/Tools/patchcheck/patchcheck.py +++ b/Tools/patchcheck/patchcheck.py @@ -257,7 +257,7 @@ def ci(pull_request): print('No whitespace issues found') else: count = len(fixed) - print(f'Please fix the {count} {n_files_str(count)} with whitespace issues') + print(f'Please fix the {n_files_str(count)} with whitespace issues') print('(on Unix you can run `make patchcheck` to make the fixes)') sys.exit(1) From 433153d6fd65581e1cb538c45c4e04dad874cc72 Mon Sep 17 00:00:00 2001 From: Hugo van Kemenade Date: Mon, 25 Sep 2023 12:35:45 -0600 Subject: [PATCH 08/11] Use "s" variable for readability Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com> --- Tools/patchcheck/patchcheck.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Tools/patchcheck/patchcheck.py b/Tools/patchcheck/patchcheck.py index a0320d9c50416c..73d958a9505101 100755 --- a/Tools/patchcheck/patchcheck.py +++ b/Tools/patchcheck/patchcheck.py @@ -23,7 +23,8 @@ def n_files_str(count): """Return 'N file(s)' with the proper plurality on 'file'.""" - return f"{count} file{'s' if count != 1 else ''}" + s = "s" if count != 1 else "" + return f"{count} file{s}" def status(message, modal=False, info=None): From a4293442f25a2078ca54b2e68b219da1f6841264 Mon Sep 17 00:00:00 2001 From: Hugo van Kemenade Date: Mon, 25 Sep 2023 22:02:31 +0300 Subject: [PATCH 09/11] Reinstate docs_modified --- Tools/patchcheck/patchcheck.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Tools/patchcheck/patchcheck.py b/Tools/patchcheck/patchcheck.py index 73d958a9505101..41d6dbb2db54a0 100755 --- a/Tools/patchcheck/patchcheck.py +++ b/Tools/patchcheck/patchcheck.py @@ -273,6 +273,8 @@ def main(): normalize_whitespace(python_files) # C rules enforcement. normalize_c_whitespace(c_files) + # Docs updated. + docs_modified(doc_files) # Misc/ACKS changed. credit_given(misc_files) # Misc/NEWS changed. From 63c6d17a895e7a0abb19f08781ac82390c283d88 Mon Sep 17 00:00:00 2001 From: Hugo van Kemenade Date: Tue, 26 Sep 2023 13:00:35 +0300 Subject: [PATCH 10/11] Reinstate doc_files for docs_modified Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com> --- Tools/patchcheck/patchcheck.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Tools/patchcheck/patchcheck.py b/Tools/patchcheck/patchcheck.py index 41d6dbb2db54a0..504849c884a483 100755 --- a/Tools/patchcheck/patchcheck.py +++ b/Tools/patchcheck/patchcheck.py @@ -268,6 +268,8 @@ def main(): file_paths = changed_files(base_branch) python_files = [fn for fn in file_paths if fn.endswith('.py')] c_files = [fn for fn in file_paths if fn.endswith(('.c', '.h'))] + doc_files = [fn for fn in file_paths if fn.startswith('Doc') and + fn.endswith(('.rst', '.inc'))] misc_files = {p for p in file_paths if p.startswith('Misc')} # PEP 8 whitespace rules enforcement. normalize_whitespace(python_files) From 05a5d357d83932441d1e7834523d07c686c6aa6d Mon Sep 17 00:00:00 2001 From: Hugo van Kemenade Date: Wed, 27 Sep 2023 22:49:28 +0300 Subject: [PATCH 11/11] Revert makefile changes --- Makefile.pre.in | 2 -- 1 file changed, 2 deletions(-) diff --git a/Makefile.pre.in b/Makefile.pre.in index af2b0b17363078..d123fa3e6f4a47 100644 --- a/Makefile.pre.in +++ b/Makefile.pre.in @@ -2772,8 +2772,6 @@ funny: .PHONY: patchcheck patchcheck: all $(RUNSHARED) ./$(BUILDPYTHON) $(srcdir)/Tools/patchcheck/patchcheck.py - @$(RUNSHARED) ./$(BUILDPYTHON) -m pre_commit --version >/dev/null 2>&1 || $(RUNSHARED) ./$(BUILDPYTHON) -m pip install pre-commit >/dev/null 2>&1 - $(RUNSHARED) ./$(BUILDPYTHON) -m pre_commit run --all-files .PHONY: check-limited-abi check-limited-abi: all