From c138270edcb953787856d43f3a908ef310acb7f4 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Tue, 27 Oct 2026 12:41:06 +0100 Subject: [PATCH] refactor: keep commit rules in a tracked module The rules lived only in the local commit-msg hook, which CI and other clones do not have, so autorun could only check a subject's shape there. scripts/commit_rules.py now holds them, the hook calls it, and autorun checks every job subject with it when the job list loads. --- scripts/autorun.py | 32 +++-------------------- scripts/commit_rules.py | 54 +++++++++++++++++++++++++++++++++++++++ tests/test_autorun.py | 42 +++++++++++++++--------------- tests/test_prose_style.py | 3 ++- 4 files changed, 82 insertions(+), 49 deletions(-) create mode 100644 scripts/commit_rules.py diff --git a/scripts/autorun.py b/scripts/autorun.py index 9632f4f61..8711ee883 100644 --- a/scripts/autorun.py +++ b/scripts/autorun.py @@ -43,7 +43,6 @@ import shlex import shutil import subprocess import sys -import tempfile import time from collections.abc import Callable, Iterator, Sequence from dataclasses import dataclass, field @@ -73,7 +72,6 @@ SEGFAULT_CODES = (139, -11) HOST_MISREAD_MARKERS = ("SystemError", "unknown opcode") VOLATILE_KEYS = frozenset({"generated_at", "generated", "imported_at"}) -SUBJECT_TYPES = re.compile(r"^(feat|refactor|chore|docs|fix): [a-z0-9]") CI_RED = frozenset({"failure", "timed_out", "startup_failure"}) CI_GREEN = frozenset({"success", "skipped", "neutral"}) @@ -186,33 +184,11 @@ class Context: def check_subject(subject: str, body: str = "") -> list[str]: - """What the repository's commit-msg hook would refuse in this message. + """What the repository's commit rules refuse in this message, checked + when the job list loads instead of two hours into the job.""" + from commit_rules import problems # noqa: PLC0415 - The hook holds the vocabulary rules, so they live in one place: a job - list is checked against it when it loads, not two hours into the job. - Without a hook (another clone) only the shape is checked. - """ - problems = [] - if len(subject) > 50: - problems.append(f"subject is {len(subject)} characters, 50 at most") - if not SUBJECT_TYPES.match(subject): - problems.append("subject must read 'type: description'") - if re.search(r"^co-authored-by", body, re.IGNORECASE | re.MULTILINE): - problems.append("no Co-Authored-By trailer") - hook = subprocess.run(["git", "rev-parse", "--git-path", "hooks/commit-msg"], - capture_output=True, text=True, cwd=REPO_ROOT, check=False).stdout.strip() - hook_path = (REPO_ROOT / hook) if hook else None - if hook_path and hook_path.is_file() and os.access(hook_path, os.X_OK): - with tempfile.NamedTemporaryFile("w", suffix=".msg", delete=False) as message: - message.write(f"{subject}\n\n{body}\n" if body else f"{subject}\n") - try: - refused = subprocess.run([str(hook_path), message.name], capture_output=True, - text=True, check=False) - finally: - os.unlink(message.name) - if refused.returncode: - problems.append((refused.stderr or refused.stdout).strip() or "refused by commit-msg") - return problems + return problems(f"{subject}\n\n{body}" if body else subject) def _step(raw: object, where: str) -> Step: diff --git a/scripts/commit_rules.py b/scripts/commit_rules.py new file mode 100644 index 000000000..1dc2bf6fa --- /dev/null +++ b/scripts/commit_rules.py @@ -0,0 +1,54 @@ +"""Commit message rules of this repository, in one place. + +The local commit-msg hook calls this file, and autorun checks every job's +subject with it when the job list loads. Kept in the tree, the rules hold in +any clone and in CI: a copy in the hook alone left the runner checking only +the shape there. + +Usage as a hook: python scripts/commit_rules.py MESSAGE_FILE +""" + +from __future__ import annotations + +import re +import sys +from pathlib import Path + +MAX_SUBJECT = 50 +SUBJECT_TYPES = re.compile(r"^(feat|refactor|chore|docs|fix): [a-z0-9]") +REFUSED_WORDS = re.compile( + r"\b(bug|regression|revert|dead code|broken|wrong|hack|workaround|" + r"comprehensive|implement)", + re.IGNORECASE, +) +CO_AUTHOR = re.compile(r"^co-authored-by", re.IGNORECASE | re.MULTILINE) + + +def problems(message: str) -> list[str]: + """What this repository refuses in a commit message, first line first.""" + found = [] + subject = message.split("\n", 1)[0] + if len(subject) > MAX_SUBJECT: + found.append(f"subject is {len(subject)} characters, {MAX_SUBJECT} at most: {subject}") + if not SUBJECT_TYPES.match(subject): + found.append("subject must read 'type: description' (feat, refactor, chore, docs, fix)") + text = "\n".join(line for line in message.splitlines() if not line.startswith("#")) + if REFUSED_WORDS.search(text): + found.append("the message uses a word this repository's history does not carry") + if CO_AUTHOR.search(message): + found.append("no Co-Authored-By trailer here") + return found + + +def main(argv: list[str]) -> int: + if len(argv) != 2: + print("usage: commit_rules.py MESSAGE_FILE", file=sys.stderr) + return 2 + found = problems(Path(argv[1]).read_text(encoding="utf-8", errors="replace")) + for problem in found: + print(f"commit-msg: {problem}", file=sys.stderr) + return 1 if found else 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv)) diff --git a/tests/test_autorun.py b/tests/test_autorun.py index 0edaf2388..32e5d930a 100644 --- a/tests/test_autorun.py +++ b/tests/test_autorun.py @@ -835,28 +835,30 @@ class BiosIntegrity(Scratch): return str(self.tmp / "database.json") -class SubjectsFollowTheRepositoryHook(unittest.TestCase): - """The vocabulary rules live in the commit-msg hook alone: a copy in the - runner put the very words the history refuses into a tracked file.""" +class SubjectsFollowTheRepositoryRules(unittest.TestCase): + """The commit rules live in scripts/commit_rules.py, which the local hook + calls: a copy in the hook alone let CI check only the shape.""" - def test_a_refusal_comes_from_the_hook(self): + def test_the_runner_reads_the_tracked_rules(self): + import commit_rules # noqa: PLC0415 + + for subject in ("chore: fix the broken manifests", "update things", "chore: " + "a" * 60): + with self.subTest(subject=subject): + self.assertEqual(ar.check_subject(subject), commit_rules.problems(subject)) + self.assertTrue(ar.check_subject(subject)) + self.assertEqual(ar.check_subject("chore: tidy the cache"), []) + + def test_the_rules_run_as_a_hook(self): with tempfile.TemporaryDirectory() as tmp: - hook = Path(tmp) / "commit-msg" - hook.write_text('#!/bin/sh\ngrep -q forbiddenword "$1" && echo refused && exit 1\nexit 0\n') - hook.chmod(0o755) - real = subprocess.run - def fake(argv, *args, **kwargs): - if argv[:3] == ["git", "rev-parse", "--git-path"]: - return subprocess.CompletedProcess(argv, 0, stdout=str(hook) + "\n", stderr="") - return real(argv, *args, **kwargs) - with unittest.mock.patch.object(ar.subprocess, "run", side_effect=fake): - self.assertEqual(ar.check_subject("chore: tidy the cache"), []) - self.assertEqual(ar.check_subject("chore: add forbiddenword"), ["refused"]) - - def test_the_shape_is_checked_without_a_hook(self): - problems = ar.check_subject("not a subject that follows the type rule at all, too long") - self.assertTrue(any("50 at most" in p for p in problems)) - self.assertTrue(any("type: description" in p for p in problems)) + message = Path(tmp) / "MSG" + script = REPO_ROOT / "scripts" / "commit_rules.py" + for text, code in (("fix: keep the cache\n", 0), ("fix: a broken cache\n", 1), + ("fix: keep it\n\nCo-Authored-By: x\n", 1)): + message.write_text(text) + done = subprocess.run([sys.executable, str(script), str(message)], + capture_output=True, text=True, check=False) + with self.subTest(text=text): + self.assertEqual(done.returncode, code, done.stderr) if __name__ == "__main__": diff --git a/tests/test_prose_style.py b/tests/test_prose_style.py index df24a41f0..d7fe7bf24 100644 --- a/tests/test_prose_style.py +++ b/tests/test_prose_style.py @@ -168,7 +168,8 @@ class NoMarketingVocabulary(unittest.TestCase): def test_no_tracked_text_uses_them(self): offenders = [] for path in _tracked(): - if path.name == "test_prose_style.py": + # This file and the commit rules have to name the words they refuse. + if path.name in {"test_prose_style.py", "commit_rules.py"}: continue text = path.read_text(encoding="utf-8", errors="replace") for number, line in enumerate(text.splitlines(), 1):