Re: Mistakes in commit log messages

[Jim Porter <jporterbugs@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1488 bytes --]

On 4/11/2023 5:20 PM, Jim Porter wrote:
> If we did something like this, I think both post-commit and pre-push 
> hooks would be useful. The former gives committers immediate feedback on 
> their commit message (if they read the output), while the latter is what 
> ensures that the upstream commit messages are properly-formatted.

Attached is a mostly-working pair of hooks (post-commit and pre-push) to 
check this. There's still a bit more work to do though:

  1. Clean up the output a bit
  2. Make it work on non-gawk awks

Both of those should be pretty easy though. Here's an example of the 
error if you try to push:

--------------------------------------------------

   In commit 89f2dbb773: My commit message
     File lisp/bad.el listed in commit message, but not in diff
   Push aborted; please see the file 'CONTRIBUTE'
   error: failed to push some refs to 
'jporter@git.sv.gnu.org:/srv/git/emacs.git'

--------------------------------------------------

For the post-commit hook, we just pass the current commit SHA to our awk 
script, which checks that SHA to make sure its commit message is 
correct. For the pre-push hook, we need to compare the local ref that 
we're pushing to the remote ref, and then check every commit between the 
two (the Git documentation explains a bit about how the hook works[1]).

Assuming the general strategy looks ok, I'll finish this up and add more 
documentation for what's going on.

[1] https://git-scm.com/docs/githooks#_pre_push

[-- Attachment #2: 0001-WIP-Add-Git-hooks-to-check-filenames-listed-in-the-c.patch --]
[-- Type: text/plain, Size: 4068 bytes --]

From ac0e2bbcc99b07ea5185041f5a066c36cc1e42ac Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Wed, 12 Apr 2023 23:03:31 -0700
Subject: [PATCH] [WIP] Add Git hooks to check filenames listed in the commit
 message

* build-aux/git-hooks/commit-msg-files.awk:
* build-aux/git-hooks/post-commit:
* build-aux/git-hooks/pre-push: New files.
---
 build-aux/git-hooks/commit-msg-files.awk | 39 +++++++++++++++++++
 build-aux/git-hooks/post-commit          | 19 +++++++++
 build-aux/git-hooks/pre-push             | 49 ++++++++++++++++++++++++
 3 files changed, 107 insertions(+)
 create mode 100644 build-aux/git-hooks/commit-msg-files.awk
 create mode 100644 build-aux/git-hooks/post-commit
 create mode 100644 build-aux/git-hooks/pre-push

diff --git a/build-aux/git-hooks/commit-msg-files.awk b/build-aux/git-hooks/commit-msg-files.awk
new file mode 100644
index 00000000000..1dd89dcdea0
--- /dev/null
+++ b/build-aux/git-hooks/commit-msg-files.awk
@@ -0,0 +1,39 @@
+function get_commit_changes(commit_sha, changes,    i, j, len, bits, filename) {
+  # Collect all the files touched in the specified commit.
+  while ((("git log -1 --name-status --format= " commit_sha) | getline) > 0) {
+    for (i = 2; i <= NF; i++) {
+      len = split($i, bits, "/")
+      for (j = 1; j <= len; j++) {
+        if (j == 1)
+          filename = bits[j]
+        else
+          filename = filename "/" bits[j]
+        changes[filename] = 1
+      }
+    }
+  }
+}
+
+function check_commit_msg_files(commit_sha,    changes, good, msg, filename) {
+  get_commit_changes(commit_sha, changes)
+  good = 1
+
+  while ((("git log -1 --format=%B " commit_sha) | getline) > 0) {
+    if (! msg)
+      msg = $0
+
+    if (/^\* / && match($2, "[^:/][^:]*")) {
+      filename = substr($2, RSTART, RLENGTH)
+      if (! (filename in changes)) {
+        if (good) {
+          printf("In commit %s: %s\n", substr(commit_sha, 1, 10), msg)
+        }
+        printf("  File %s listed in commit message, but not in diff\n",
+               filename)
+        good = 0
+      }
+    }
+  }
+
+  return good
+}
diff --git a/build-aux/git-hooks/post-commit b/build-aux/git-hooks/post-commit
new file mode 100644
index 00000000000..35c902d8270
--- /dev/null
+++ b/build-aux/git-hooks/post-commit
@@ -0,0 +1,19 @@
+#!/bin/sh
+
+git rev-parse HEAD | awk '
+  # FIXME: Do this a POSIX-compatible way.
+  @include ".git/hooks/commit-msg-files.awk"
+
+  /^[a-z0-9]{40}$/ {
+    if (! check_commit_msg_files($0)) {
+      status = 1
+    }
+  }
+
+  END {
+    if (status != 0) {
+      print "Bad commit message; please see the file '\''CONTRIBUTE'\''"
+    }
+    exit status
+  }
+'
diff --git a/build-aux/git-hooks/pre-push b/build-aux/git-hooks/pre-push
new file mode 100644
index 00000000000..3329bf59e1d
--- /dev/null
+++ b/build-aux/git-hooks/pre-push
@@ -0,0 +1,49 @@
+#!/bin/sh
+
+awk -v origin_name="$1" '
+  @include ".git/hooks/commit-msg-files.awk"
+
+  # If the local SHA is all zeroes, ignore it.
+  $2 ~ /^0{40}$/ {
+    next
+  }
+
+  $2 ~ /^[a-z0-9]{40}$/ {
+    newref = $2
+
+    # If the remote SHA is all zeroes, go backwards until we find a SHA on
+    # an origin branch.
+    if ($4 ~ /^0{40}$/) {
+      back = 0
+      while ((("git branch -r -l '\''" origin_name "/*'\'' --contains " \
+               newref "~" back) | getline) == 0) {
+        back++
+      }
+
+      ("git rev-parse " newref "~" back) | getline oldref
+      if (!(oldref ~ /^[a-z0-9]{40}$/)) {
+        # The SHA is misformatted?!
+        exit 2
+      }
+    } else if ($4 ~ /^[a-z0-9]{40}$/)  {
+      oldref = $4
+    } else {
+      # The remote SHA is misformatted?!
+      exit 2
+    }
+
+    # Iterate over every SHA after oldref, up to (and including) newref.
+    while ((("git rev-list --reverse " oldref ".." newref) | getline) > 0) {
+      if (! check_commit_msg_files($0)) {
+        status = 1
+      }
+    }
+  }
+
+  END {
+    if (status != 0) {
+      print "Push aborted; please see the file '\''CONTRIBUTE'\''"
+    }
+    exit status
+  }
+'
-- 
2.25.1