git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Phillip Wood <phillip.wood@talktalk.net>
To: Gustavo Chaves <gustavo@gnustavo.com>,
	git@vger.kernel.org,
	Johannes Schindelin <johannes.schindelin@gmx.de>,
	Junio C Hamano <gitster@pobox.com>
Subject: Re: Why git-revert doesn't invoke the pre-commit and the commit-msg hooks?
Date: Tue, 20 Feb 2018 10:28:55 +0000	[thread overview]
Message-ID: <7ff6079d-5834-3bbd-781b-a2fc0659e7e5@talktalk.net> (raw)
In-Reply-To: <CAPx58qoS-J+yJ_J4QOOnKyG=EOrT5J=UoCrXfXxEijq4Z2Z_3w@mail.gmail.com>

Hi Gustavo

On 19/02/18 14:50, Gustavo Chaves wrote:
> 
> 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


  reply	other threads:[~2018-02-20 10:29 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-13 22:27 Why git-revert doesn't invoke the pre-commit and the commit-msg hooks? Gustavo Chaves
2018-02-19 14:50 ` Gustavo Chaves
2018-02-20 10:28   ` Phillip Wood [this message]
2018-02-20 18:00     ` Junio C Hamano
2018-02-21 13:33       ` Gustavo Chaves

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7ff6079d-5834-3bbd-781b-a2fc0659e7e5@talktalk.net \
    --to=phillip.wood@talktalk.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=gustavo@gnustavo.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=phillip.wood@dunelm.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).