git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: Why git-revert doesn't invoke the pre-commit and the commit-msg hooks?

From
PWPhillip Wood <phillip.wood@talktalk.net>
Date
Feb 20, 2018, 10:28 UTC
Message-ID
<7ff6079d-5834-3bbd-781b-a2fc0659e7e5@talktalk.net>
In-Reply-To
<CAPx58qoS-J+yJ_J4QOOnKyG=EOrT5J=UoCrXfXxEijq4Z2Z_3w@mail.gmail.com>
Hi Gustavo
On 19/02/18 14:50, Gustavo Chaves wrote:
Show 29 quoted lines
> 
> I asked this question on StackOverflow and got an answer:
> https://stackoverflow.com/q/48852925/114983
> 
> The problem is that git-revert invokes git-commit with the -n flag,
> explicitly avoiding the pre-commit and the commit-msg hooks.
> 
> This was originally introduced on commit 9fa4db544e2e, by Junio
> Hamano, in 2005! The rationale in the commit message was the
> following:
> 
>>> Do not verify reverted/cherry-picked/rebased patches.
> 
>>> The original committer may have used validation criteria that is less
>>> stricter than yours.  You do not want to lose the changes even if they
>>> are done in substandard way from your 'commit -v' verifier's point of
>>> view.
> 
> I get it, but since by default you are allowed to edit the commit
> message during a git-revert I think there's a case to be made to make
> the pre-commit and the commit-msg being invoked by default. Also,
> git-revert introduces new lines in the original commit message, and
> they could be used to trigger specific checks, such as the one I
> wanted to implement, to deny commits reverting merge-commits.
> 
> Shouldn't git-revert work exactly as git-commit? Instead of disabling
> hooks by default, it could accept the --no-verify flag just like
> git-commit to disable the hooks if the user wants it.
> 

I think you're right that cherry-pick and revert should not be passing --no-verify when the commit message is being edited (I was surprised to see that they were). I wonder if it's worth getting rid of the VERIFY_MSG flag completely and just set --no-verify if EDIT_MSG isn't set to avoid these problems in the future. I'm worried though that someone out there is scripting with a non-interactive editor which may break if we start verifying the message so maybe the default should be changed and a --no-verify option added to cherry-pick and revert instead. They'd have to change their script but at least it would work and everyone else would get the behaviour they probably expect.

Best Wishes
Phillip
Previous: Gustavo ChavesNext: Junio C Hamano
Message 3 of 5 in “Why git-revert doesn't invoke the pre-commit and the commit-msg hooks?”
  1. Gustavo ChavesFeb 13, 2018
  2. Gustavo ChavesFeb 19, 2018
  3. Phillip WoodFeb 20, 2018
  4. Junio C HamanoFeb 20, 2018
  5. Gustavo ChavesFeb 21, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.