Re: [PATCH RFC v2 03/19] rebase -i: reword executes pre-commit hook on interim commit
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 8, 2014, 20:43 UTC
- Message-ID
- <xmqqlhs38tub.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <12171cf0323b3d10b12b02e3b7d42ff4b7964724.1404323078.git.bafain@gmail.com>
Fabian Ruch <bafain@gmail.com> writes:
Show 9 quoted lines
> The to-do list command `reword` replays a commit like `pick` but lets > the user also edit the commit's log message. This happens in two > steps. Firstly, the named commit is cherry-picked. Secondly, the > commit created in the first step is amended using an unchanged index > to edit the log message. The pre-commit hook is meant to verify the > changes introduced by a commit (for instance, catching inserted > trailing white space). Since the committed tree is not changed > between the two steps, do not execute the pre-commit hook in the > second step.
It is not like the second step is built as a child commit of the result from the first step, recording the same tree without any change. Why is it a good thing not to run the pre-commit hook (or other hooks for that matter)? After all, the result of the second step is what is recorded in the final history; it just feels wrong not to test that one, even if it were a good idea to test only once.
Show 44 quoted lines
> Specify the git-commit option `--no-verify` to disable the pre-commit
> hook when editing the log message. Because `--no-verify` also skips
> the commit-msg hook, execute the hook from within
> git-rebase--interactive after the commit is created. Fortunately, the
> commit message is still available in `$GIT_DIR/COMMIT_EDITMSG` after
> git-commit terminates. Caveat: In case the commit-msg hook finds the
> new log message ill-formatted, the user is only notified of the
> failed commit-msg hook but the log message is used for the commit
> anyway. git-commit ought to offer more fine-grained control over
> which hooks are executed.
>
> Signed-off-by: Fabian Ruch <bafain@gmail.com>
> ---
> git-rebase--interactive.sh | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 689400e..b50770d 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -504,10 +504,19 @@ do_next () {
>
> mark_action_done
> do_pick $sha1 "$rest"
> - git commit --allow-empty --amend --no-post-rewrite ${gpg_sign_opt:+"$gpg_sign_opt"} || {
> - warn "Could not amend commit after successfully picking $sha1... $rest"
> - exit_with_patch $sha1 1
> - }
> + # TODO: Work around the fact that git-commit lets us
> + # disable either both the pre-commit and the commit-msg
> + # hook or none. Disable the pre-commit hook because the
> + # tree is left unchanged but run the commit-msg hook
> + # from here because the log message is altered.
> + git commit --allow-empty --amend --no-post-rewrite -n ${gpg_sign_opt:+"$gpg_sign_opt"} &&
> + if test -x "$GIT_DIR"/hooks/commit-msg
> + then
> + "$GIT_DIR"/hooks/commit-msg "$GIT_DIR"/COMMIT_EDITMSG
> + fi || {
> + warn "Could not amend commit after successfully picking $sha1... $rest"
> + exit_with_patch $sha1 1
> + }
> record_in_rewritten $sha1
> ;;
> edit|e)