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

From: Jonathan Nieder <[email-masked]>
Date: Thu, 16 Jan 2020 20:35:22 +0000

Elijah Newren wrote:

> Thought about this ctags case a bit more.  In contrast to the
> deployment cases you brought up (which appear to be protected by a
> branch check anyway),

To be clear, one of the two deployment cases I mentioned (gh-pages) is
protected by a branch check, but the other (git --work-tree=srvroot
checkout) is not.

>                       doesn't this one argue more that post-commit
> should be run for all rebase types?  Sure, it'd take more computation
> time, but if a user runs "git pull --rebase" and then is frustrated
> that all their tags are extremely out of date then they may be kind of
> frustrated.

Yes.  In fact I think all of these examples argue for having the
post-commit hook run once on rebase completion (and on "git am"
completion, "git merge --ff-only" completion, etc).

Of course it's not hard to find other examples that argue against
that.  We provided a hook "here's a notification that runs after git
commit runs" without tailoring it to a particular need, and on one
hand people came up with various creative ways to use it (yay!) but on
the other hand it's hard to maintain as Git's ways of making new
commits evolve.

> Of course, arguing against myself, we could always tell them to just
> implement a post-rewrite hook.

That's an interesting facet I hadn't thought about before.

It's tempting since it kind of minimizes the blast surface while
providing people a way to keep doing what they had wanted.

Historically "git cherry-pick" ran "git commit", causing it to run the
post-commit hook.  There's some related discussion at
https://lore.kernel.org/git/[email-masked]/
of other side effects of having run "git commit".

... oh! git cherry-pick doesn't run post-rewrite.  Interesting.

To summarize what we've discussed so far:

 A. run post-commit hook consistently in rebase --am and --merge modes

    Advantages:
    * consistent with historic "rebase -i" behavior
    * supports hooks like ctags generation that want to update state to
      match what has been committed in the worktree (especially when
      preparing for the rebase to stop due to conflicts)
    * consistent with other commands like "git cherry-pick" that run the
      post-commit hook

    Disadvantages:
    * invokes hooks more often in a setting they are not used to being
      invoked in
    * slows down rebase
    * when post-commit hooks are used for deployment, exposes
      intermediate states in the middle of a rebase to the deployment
      environment
    * inconsistent with "git am"

    Mitigations:
    * could go through the normal warn about change, opt-in config,
      opt-out config cycle to make the change smoother
    * could provide new hooks (e.g., one run after a batch of objects
      is created for applications similar to "git gc --auto", one run
      when HEAD is updated for applications like the ctags one, one
      run on ref update for applications like the deployment case) and
      encourage authors to migrate to them
    * could run the post-commit hook in "git am", too

 B. stop running the post-commit hook in rebase

    Advantages:
    * makes --am and --merge consistent with minimal user impact

    Disadvantages:
    * meaning of the post-commit hook remains a muddle
    * in particular, "git am" and "git rebase" would not be consistent
      with "git cherry-pick"

    Mitigations:
    * could provide new hooks for people to migrate to to replace that
      muddle

 C. stop running the post-commit hook in rebase --merge or commit --amend

    Advantages:
    * produces a consistent definition of post-commit ("it's about new
      commits only")
    * hook authors get the same expressiveness using the post-rewrite hook

    Disadvantages:
    * the change to historical "git commit --amend" behavior is likely
      to be surprising

    Mitigations:
    * could go through the warn, opt-in config, opt-out config cycle

Any bits I've missed?

Thanks,
Jonathan