Re: [PATCH v3 15/15] rebase: change the default backend from "am" to "merge"

> Hi,
> 
> Phillip Wood wrote:
> > On 11/01/2020 01:16, Elijah Newren wrote:
> 
> >> But the big question here, is what is correct behavior?  Should rebase
> >> call the post-commit hook, or should it skip it?  I haven't any clue
> >> what the answer to that is.
> >
> > It's creating a new commit so I lean towards thinking it should run the
> > post-commit hook. As an example I have a post-commit hook that prints a
> > warning if a commit is created on a branch that is being rewritten by one of
> > my scripts in another worktree. There are pre-commit and pre-rebase hooks to
> > try and prevent that, but the warning is there as a last resort if those
> > hooks are by-passed.
> 
> The initial design for the post-commit hook was
> 
>     commit 89e2c5f17b901edf28a8bb778ece3f6c18bbb721
>     Author: Junio C Hamano <gitster@pobox.com>
>     Date:   Thu Aug 18 17:20:08 2005 -0700
> 
>     Add commit hook and make the verification customizable.
> 
>     There are three hooks:
> 
>         - 'pre-commit' is given an opportunity to inspect what is
>           being committed, before we invoke the EDITOR for the
>           commit message;
> 
>         - 'commit-msg' is invoked on the commit log message after
>           the user prepares it;
> 
>         - 'post-commit' is run after a successful commit is made.
> 
>     The first two can interfere to stop the commit.  The last one is
>     for after-the-fact notification.
> 
> The initial implementation was only in "git commit" and didn't affect
> other commands, but that's an artifact of implementation, not
> intention.  The intention is more murky: certainly "creating a new
> commit" is not the event we want to notify about (for example, "git
> commit-tree" should not invoke the hook), but if I were designing it
> today then all operations that create a new commit and update the
> current branch with it might qualify.
> 
> Even that definition is a bit fuzzy: when I run "git rebase <upstream>
> <branch>", am I updating <branch> or the current branch?  Are
> cherry-picks that carry over changes that were committed previously
> new commits?
> 
> Worse, we have the body of existing post-commit hooks to contend with,
> and each one may have made different assumptions about the semantics.
> We do not have the luxury of designing the hook without regard to that
> body of existing hooks today.

> The hook that jiri installs is very simple.  It wants to check when a
> user has (interactively) made a commit on a detached HEAD, to let them
> know that they might want to use "git checkout -b" afterward.  With
> this particular hook, the behavior is better when git rebase does not
> invoke the hook, since in the context of a rebase, the user has no
> need to use "git checkout -b" afterward.  This is a regression, and I
> think we need to take that seriously.
> 
> It doesn't seem like jiri is doing anything weird here, so there's a
> reasonably high probability that other hooks would be affected in the
> same way.  How can we help authors of such hooks to handle this
> change?

> I don't think that starting to invoke the 'post-commit' hook even more
> without going through that thought process is an acceptable fix.
> Since the "right" semantics aren't obvious here, the first step is
> probably to collect some typical examples of post-commit hooks, like
> the example from the message I'm replying to (thank you!).  Ideas for
> where we can find more?