Re: post-checkout hook aborts rebase

From: Elijah Newren <newren@gmail.com>
Date: Aug 27, 2020, 20:11 UTC
Message-ID: <CABPp-BEEmqdt2_OuNQFxyf6pkppBWptkcvuAYhWX8r+_Wr8-2g@mail.gmail.com>
In-Reply-To: <CAPx1GvfXOMFDsXE74LiG_BG5Bqb+seHDWX69xGe49C240ORi6A@mail.gmail.com>

On Thu, Aug 27, 2020 at 12:07 PM Chris Torek <chris.torek@gmail.com> wrote:

Show 8 quoted lines

>
> On Thu, Aug 27, 2020 at 8:51 AM Junio C Hamano <gitster@pobox.com> wrote:
> > I still suspect that the checkout run, merely as an implementation
> > detail of rebase (or any other git subcommand), should not trigger
> > any hook ...
>
> The *last* checkout from the finished rebase perhaps *should*, but
> other than that one, that seems logically correct.

What do you mean by the "last checkout"? I believe a typical non-interactive rebase of N patches has only one checkout, and I don't see why running hooks for the starting point is relevant.

If hooks are wanted for rebase, I'd still want to have rebase-specific ones, because most people who think of "checkout hooks" or "commit hooks" probably aren't going to think of them the way rebase or cherry-pick happen to use them. (And that might even be more true for --interactive and --rebase-merges cases.)

Show 26 quoted lines

> > but before any such code change, at least let's update the
> > documentation to clarify what we mean by "the outcome".
> >
> > Hopefully something like the below may be a good starting point?
> >
> >  Documentation/githooks.txt | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
> > index 31b601e4bc..cf95d6d02b 100644
> > --- a/Documentation/githooks.txt
> > +++ b/Documentation/githooks.txt
> > @@ -193,7 +193,9 @@ worktree.  The hook is given three parameters: the ref of the previous HEAD,
> >  the ref of the new HEAD (which may or may not have changed), and a flag
> >  indicating whether the checkout was a branch checkout (changing branches,
> >  flag=1) or a file checkout (retrieving a file from the index, flag=0).
> > -This hook cannot affect the outcome of `git switch` or `git checkout`.
> > +This hook cannot affect the outcome of `git switch` or `git checkout`,
> > +other than that the hook's exit status becomes the exit status of
> > +these two commands.
> >
> >  It is also run after linkgit:git-clone[1], unless the `--no-checkout` (`-n`) option is
> >  used. The first parameter given to the hook is the null-ref, the second the
>
> This looks good to me, and can either be independent of, or the first part of,
> any rebase update.

Patch looks good to me too.