tools: Add pre-commit support.

projectgus · dpgeorge · commit 0e35c4de9b9d · 2022-10-04T14:52:48.000+11:00
Tweak the existing codeformat.py and verifygitlog.py to allow them to be
easily called by pre-commit.

(This turned out to be easier than using any existing pre-commit hooks,
without making subtle changes in the formatting.)

This work was funded through GitHub Sponsors.

Signed-off-by: Angus Gratton &lt;angus@redyak.com.au&gt;
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -0,0 +1,13 @@
+repos:
+  - repo: local
+    hooks:
+      - id: codeformat
+        name: MicroPython codeformat.py for changed files
+        entry: tools/codeformat.py -v -f
+        language: python
+      - id: verifygitlog
+        name: MicroPython git commit message format checker
+        entry: tools/verifygitlog.py --check-file --ignore-rebase
+        language: python
+        verbose: true
+        stages: [commit-msg]
diff --git a/CODECONVENTIONS.md b/CODECONVENTIONS.md
@@ -69,6 +69,38 @@ the tool the files that changed and it will only reformat those.
 v0.71 or v0.72 for MicroPython. Different uncrustify versions produce slightly
 different formatting, and the configuration file formats are often incompatible.
 
+Automatic Pre-Commit Hooks
+==========================
+
+To have code formatting and commit message conventions automatically checked
+using [pre-commit](https://pre-commit.com/), run the following commands in your
+local MicroPython directory:
+
+```
+$ pip install pre-commit
+
+$ pre-commit install
+
+$ pre-commit install --hook-type commit-msg
+```
+
+pre-commit will now automatically run during `git commit` for both code and
+commit message formatting.
+
+The same formatting checks will be run by CI for any Pull Request submitted to
+MicroPython. Pre-commit allows you to see any failure more quickly, and in many
+cases will automatically correct it in your local working copy.
+
+Tips:
+
+* To skip pre-commit checks on a single commit, use `git commit -n` (for
+  `--no-verify`).
+* To ignore the pre-commit message format check temporarily, start the commit
+  message subject line with "WIP" (for "Work In Progress").
+
+(It is also possible to install pre-commit using Brew or other sources, see
+[the docs](https://pre-commit.com/index.html#install) for details.)
+
 Python code conventions
 =======================
 
diff --git a/tools/codeformat.py b/tools/codeformat.py
@@ -151,6 +151,11 @@ def main():
     cmd_parser.add_argument("-c", action="store_true", help="Format C code only")
     cmd_parser.add_argument("-p", action="store_true", help="Format Python code only")
     cmd_parser.add_argument("-v", action="store_true", help="Enable verbose output")
+    cmd_parser.add_argument(
+        "-f",
+        action="store_true",
+        help="Filter files provided on the command line against the default list of files to check.",
+    )
     cmd_parser.add_argument("files", nargs="*", help="Run on specific globs")
     args = cmd_parser.parse_args()
 
@@ -162,6 +167,16 @@ def main():
     files = []
     if args.files:
         files = list_files(args.files)
+        if args.f:
+            # Filter against the default list of files. This is a little fiddly
+            # because we need to apply both the inclusion globs given in PATHS
+            # as well as the EXCLUSIONS, and use absolute paths
+            files = set(os.path.abspath(f) for f in files)
+            all_files = set(list_files(PATHS, EXCLUSIONS, TOP))
+            if args.v:  # In verbose mode, log any files we're skipping
+                for f in files - all_files:
+                    print("Not checking: {}".format(f))
+            files = list(files & all_files)
     else:
         files = list_files(PATHS, EXCLUSIONS, TOP)
 
diff --git a/tools/verifygitlog.py b/tools/verifygitlog.py
@@ -7,6 +7,8 @@
 verbosity = 0  # Show what's going on, 0 1 or 2.
 suggestions = 1  # Set to 0 to not include lengthy suggestions in error messages.
 
+ignore_prefixes = []
+
 
 def verbose(*args):
     if verbosity:
@@ -18,6 +20,22 @@ def very_verbose(*args):
         print(*args)
 
 
+class ErrorCollection:
+    # Track errors and warnings as the program runs
+    def __init__(self):
+        self.has_errors = False
+        self.has_warnings = False
+        self.prefix = ""
+
+    def error(self, text):
+        print("error: {}{}".format(self.prefix, text))
+        self.has_errors = True
+
+    def warning(self, text):
+        print("warning: {}{}".format(self.prefix, text))
+        self.has_warnings = True
+
+
 def git_log(pretty_format, *args):
     # Delete pretty argument from user args so it doesn't interfere with what we do.
     args = ["git", "log"] + [arg for arg in args if "--pretty" not in args]
@@ -28,83 +46,88 @@ def git_log(pretty_format, *args):
         yield line.decode().rstrip("\r\n")
 
 
-def verify(sha):
+def verify(sha, err):
     verbose("verify", sha)
-    errors = []
-    warnings = []
-
-    def error_text(err):
-        return "commit " + sha + ": " + err
-
-    def error(err):
-        errors.append(error_text(err))
-
-    def warning(err):
-        warnings.append(error_text(err))
+    err.prefix = "commit " + sha + ": "
 
     # Author and committer email.
     for line in git_log("%ae%n%ce", sha, "-n1"):
         very_verbose("email", line)
         if "noreply" in line:
-            error("Unwanted email address: " + line)
+            err.error("Unwanted email address: " + line)
 
     # Message body.
     raw_body = list(git_log("%B", sha, "-n1"))
+    verify_message_body(raw_body, err)
+
+
+def verify_message_body(raw_body, err):
     if not raw_body:
-        error("Message is empty")
-        return errors, warnings
+        err.error("Message is empty")
+        return
 
     # Subject line.
     subject_line = raw_body[0]
+    for prefix in ignore_prefixes:
+        if subject_line.startswith(prefix):
+            verbose("Skipping ignored commit message")
+            return
     very_verbose("subject_line", subject_line)
     subject_line_format = r"^[^!]+: [A-Z]+.+ .+\.$"
     if not re.match(subject_line_format, subject_line):
-        error("Subject line should match " + repr(subject_line_format) + ": " + subject_line)
+        err.error("Subject line should match " + repr(subject_line_format) + ": " + subject_line)
     if len(subject_line) >= 73:
-        error("Subject line should be 72 or less characters: " + subject_line)
+        err.error("Subject line should be 72 or less characters: " + subject_line)
 
     # Second one divides subject and body.
     if len(raw_body) > 1 and raw_body[1]:
-        error("Second message line should be empty: " + raw_body[1])
+        err.error("Second message line should be empty: " + raw_body[1])
 
     # Message body lines.
     for line in raw_body[2:]:
         # Long lines with URLs are exempt from the line length rule.
         if len(line) >= 76 and "://" not in line:
-            error("Message lines should be 75 or less characters: " + line)
+            err.error("Message lines should be 75 or less characters: " + line)
 
     if not raw_body[-1].startswith("Signed-off-by: ") or "@" not in raw_body[-1]:
-        warning("Message should be signed-off")
-
-    return errors, warnings
+        err.warning("Message should be signed-off")
 
 
 def run(args):
     verbose("run", *args)
-    has_errors = False
-    has_warnings = False
-    for sha in git_log("%h", *args):
-        errors, warnings = verify(sha)
-        has_errors |= any(errors)
-        has_warnings |= any(warnings)
-        for err in errors:
-            print("error:", err)
-        for err in warnings:
-            print("warning:", err)
-    if has_errors or has_warnings:
+
+    err = ErrorCollection()
+
+    if "--check-file" in args:
+        filename = args[-1]
+        verbose("checking commit message from", filename)
+        with open(args[-1]) as f:
+            lines = [line.rstrip("\r\n") for line in f]
+            verify_message_body(lines, err)
+    else:  # Normal operation, pass arguments to git log
+        for sha in git_log("%h", *args):
+            verify(sha, err)
+
+    if err.has_errors or err.has_warnings:
         if suggestions:
             print("See https://github.com/micropython/micropython/blob/master/CODECONVENTIONS.md")
     else:
         print("ok")
-    if has_errors:
+    if err.has_errors:
         sys.exit(1)
 
 
 def show_help():
-    print("usage: verifygitlog.py [-v -n -h] ...")
+    print("usage: verifygitlog.py [-v -n -h --check-file] ...")
     print("-v  : increase verbosity, can be speficied multiple times")
     print("-n  : do not print multi-line suggestions")
     print("-h  : print this help message and exit")
+    print(
+        "--check-file : Pass a single argument which is a file containing a candidate commit message"
+    )
+    print(
+        "--ignore-rebase : Skip checking commits with git rebase autosquash prefixes or WIP as a prefix"
+    )
     print("... : arguments passed to git log to retrieve commits to verify")
     print("      see https://www.git-scm.com/docs/git-log")
     print("      passing no arguments at all will verify all commits")
@@ -117,6 +140,10 @@ def show_help():
     args = sys.argv[1:]
     verbosity = args.count("-v")
     suggestions = args.count("-n") == 0
+    if "--ignore-rebase" in args:
+        args.remove("--ignore-rebase")
+        ignore_prefixes = ["squash!", "fixup!", "amend!", "WIP"]
+
     if "-h" in args:
         show_help()
     else:
