Re: New Git hooks for checking file names in commit messages

[Jim Porter <jporterbugs@gmail.com>]

I've pushed a couple of fixes for the hooks (described in more detail 
below). I *think* this should resolve all the issues, though I'll still 
do more testing locally, especially with merge commits.

More generally, if people find these hooks to be annoying or unhelpful, 
I truly won't be offended if they get backed out. I wrote them in the 
hopes that it will make it easier to maintain Emacs, but if they can't 
meet that goal, then they shouldn't stick around causing problems.

On 4/23/2023 12:37 AM, Eli Zaretskii wrote:
>> Date: Sun, 23 Apr 2023 00:07:49 -0700
>> Cc: bjorn.bidar@thaodan.de, arsen@aarsen.me, acm@muc.de, emacs-devel@gnu.org
>> From: Jim Porter <jporterbugs@gmail.com>
>>
>> Ok, it should be easy to restrict the pre-push hook to master/emacs-NN
>> branches. ...
> 
> It's a compromise (ideally, other mistakes in file names should also
> be detected), but I think it's the best we can do without too many
> false positives.

Ok, done.

>> This part should be easy enough to handle: if a commit is a merge
>> commit, we just won't validate any file names listed in the log message.
> 
> This is not what I meant, though.  If a merge-commit does have a
> meaningful log message, its file names should be validated.

It turns out this is actually very easy to do, now that I've learned 
about the "--first-parent" option in Git. Using that on a merge commit, 
I can get a list of all the changes that were merged into the target 
branch (i.e. everything from the feature branch that's now on, say, master).

With this fix, commit 289b457cac1 (the merge of 'abort-redisplay') now 
passes the tests, as it should.

>> Suppose I make commits A, B, and C on a branch, and then merge to master
>> with merge-commit D. Does your message mean that the only commit that
>> ends up in the ChangeLog file is commit D? (Or that commit D is the only
>> one that authors.el looks at?)
> 
> The only one that it is reasonable to validate is commit D, yes.  The
> other commits should not be rejected due to their log messages, at
> least not ideally.

I believe "--first-parent" also solves this case. In particular, by 
using this option, the hook correctly ignores the commits from the Eglot 
branch that were merged in. This means that any merge like that in the 
future won't run afoul of these hooks.

I'll have to do a bit more testing here to try out other edge cases, but 
this should only result in false negatives, not false positives. (False 
negatives would just require some fixup when making the tarball.)