post-checkout hook aborts rebase

post-checkout hook aborts rebase

[Tom Rutherford]

Hi Git community,

My team encountered an issue today which I believe is a bug, and I'm
interested to know if those more familiar with the codebase and
documentation agree with me. Here's a bug report. Thanks for your
attention.

Tom

===

Thank you for filling out a Git bug report!
Please answer the following questions to help us understand your issue.

What did you do before the bug happened? (Steps to reproduce your issue)

1. Have a post-checkout hook that returns a nonzero exit status.
2. `git pull --rebase` where there is at least one local commit to be
rebased onto an incoming commit. (I assume any rebase would exhibit
similar behavior, but this is how I encountered the issue).

What did you expect to happen? (Expected behavior)

The rebase succeeds.

I expect this because the documentation for the post-checkout hook
states, "This hook cannot affect the outcome of git switch or git
checkout" https://www.git-scm.com/docs/githooks#_post_checkout

What happened instead? (Actual behavior)

`error: could not detach HEAD` is displayed, and I end up in a
detached head state.

What's different between what you expected and what actually happened?

I do not expect the post-checkout hook to prevent rebase from
succeeding. The documentation suggests this should succeed. It seems
either the documentation is wrong, or git's use of the hook is in this
case.

Anything else you want to add:

The issue is elegantly explained in this stack overflow answer:
https://stackoverflow.com/a/25562688/1286571

It's not clear to me if this is a bug, or an omission from the
documentation. I reproduced it on 2.28, but I haven't tried building
from source.

Please review the rest of the bug report below.
You can delete any lines you don't wish to share.


[System Info]
git version:
git version 2.28.0
cpu: x86_64
no commit associated with this build
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
uname: Linux 5.4.0-42-generic #46~18.04.1-Ubuntu SMP Fri Jul 10
07:21:24 UTC 2020 x86_64
compiler info: gnuc: 7.5
libc info: glibc: 2.27
$SHELL (typically, interactive shell): /usr/bin/fish


[Enabled Hooks]
pre-commit
post-checkout

Re: post-checkout hook aborts rebase

[Junio C Hamano]

Tom Rutherford <tmrutherford@gmail.com> writes:

> What did you expect to happen? (Expected behavior)
>
> The rebase succeeds.
>
> I expect this because the documentation for the post-checkout hook
> states, "This hook cannot affect the outcome of git switch or git
> checkout" https://www.git-scm.com/docs/githooks#_post_checkout

In general, pre-<anything> hook is run before <anything> subcommand
is executed, and by exiting with non-zero status, the hook can tell
the <anything> subcommand not to proceed.  post-<anything> hook, on
the other hand, runs _after_ <anything> has done its thing already,
so by definition, it cannot say "Hey, <anything>, don't continue".

That is the primary meaning of "cannot affect" in that description. 

Without looking at the contents of the actual hook is, nobody can
say anything definite, but it also means that "Your hook is NOT
ALLOWED TO do any extra modification to files and the index 'git
switch' or 'git checkout' made".  If "git rebase" or whatever
command wanted to place files and the index into some state by using
"git checkout" command, and if the post-checkout hook mucked with
the state in such a way that contradicts with what the "git rebase"
command wanted them to be in, it is not surprising the hook's behavior
broke "git rebase"'s operation.

Re: post-checkout hook aborts rebase

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> ...  If "git rebase" or whatever
> command wanted to place files and the index into some state by using
> "git checkout" command, and if the post-checkout hook mucked with
> the state in such a way that contradicts with what the "git rebase"
> command wanted them to be in, it is not surprising the hook's behavior
> broke "git rebase"'s operation.

Having said all that, I actually think that "rebase" shouldn't be
invoking "git checkout" (and its equivalent) internally when
switching to a specific version, in such a way that it would trigger
any end-user specified hooks and allow them to muck with the working
tree and the index state.

I haven't checked the actual implementation of "git rebase" for
quite some time to be sure, but we have lower-level plumbing
commands that are not affected by the end-user hooks for exactly
that kind of "build higher-level commands by synthesis of
lower-level machinery", and it is very possible that what we are
looking at is actually a bug that needs to be fixed.  I dunno.

Thanks.

Re: post-checkout hook aborts rebase

[Tom Rutherford]

Thank you for the response Junio.

For what it's worth, my hook does not make changes to the repo. It's
running a command to check that the installed version of our
dependencies match the version specified in the commit being checked
out, and merely warns if the two don't match (then exits with a
nonzero return code).

For this reason it's been convenient that the hook runs during
rebases, but I find it surprising that the nonzero return code would
impact the rebase.


Tom

On Wed, Aug 26, 2020 at 5:22 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Junio C Hamano <gitster@pobox.com> writes:
>
> > ...  If "git rebase" or whatever
> > command wanted to place files and the index into some state by using
> > "git checkout" command, and if the post-checkout hook mucked with
> > the state in such a way that contradicts with what the "git rebase"
> > command wanted them to be in, it is not surprising the hook's behavior
> > broke "git rebase"'s operation.
>
> Having said all that, I actually think that "rebase" shouldn't be
> invoking "git checkout" (and its equivalent) internally when
> switching to a specific version, in such a way that it would trigger
> any end-user specified hooks and allow them to muck with the working
> tree and the index state.
>
> I haven't checked the actual implementation of "git rebase" for
> quite some time to be sure, but we have lower-level plumbing
> commands that are not affected by the end-user hooks for exactly
> that kind of "build higher-level commands by synthesis of
> lower-level machinery", and it is very possible that what we are
> looking at is actually a bug that needs to be fixed.  I dunno.
>
> Thanks.

Re: post-checkout hook aborts rebase

[Chris Torek]

On Wed, Aug 26, 2020 at 5:48 PM Tom Rutherford <tmrutherford@gmail.com> wrote:
>
> Thank you for the response Junio.
>
> For what it's worth, my hook does not make changes to the repo. It's
> running a command to check that the installed version of our
> dependencies match the version specified in the commit being checked
> out, and merely warns if the two don't match (then exits with a
> nonzero return code).

I've run into this before myself.  The core of the bug is that when
`git checkout` runs the post-checkout hook, whatever exit status
that hook has, `git checkout` has the same exit status.

This might be intended as a feature, but if so, the documentation
needs a tweak: the githooks docs say in part

    This hook cannot affect the outcome of git checkout.

If "outcome" includes exit status -- to me, it does -- either the docs
are wrong or the code is wrong.

> For this reason it's been convenient that the hook runs during
> rebases, but I find it surprising that the nonzero return code would
> impact the rebase.

I have to agree with this, too.

(The simplest fix would be to have `git checkout` ignore the status
from the post-checkout hook, of course, and just exit 0 for success.)

Chris

Re: post-checkout hook aborts rebase

[Junio C Hamano]

Chris Torek <chris.torek@gmail.com> writes:

> This might be intended as a feature, but if so, the documentation
> needs a tweak: the githooks docs say in part
>
>     This hook cannot affect the outcome of git checkout.
>
> If "outcome" includes exit status -- to me, it does -- either the docs
> are wrong or the code is wrong.

I suspected that this was a case of adding the hook that just runs
without affecting the exit code and then somebody later changed the
behaviour (either deliberately or by mistake) and forgot to update
the documentation.  But it seems that ever since the hook support
was introduced at 1abbe475 (post-checkout hook, tests, and docs,
2007-09-26), the command was made to exit with the same status from
the hook.

So I agree that depending on reader's view on "the outcome", this is
documented incorrectly (if the exit code is part of "the outcome")
or just fine (otherwise).  Apparently, the author of the patch and
the reviewers back then thought the latter.

I still suspect that the checkout run, merely as an implementation
detail of rebase (or any other git subcommand), should not trigger
any hook, 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

Re: post-checkout hook aborts rebase

[Chris Torek]

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.

> 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.

Chris

Re: post-checkout hook aborts rebase

[Elijah Newren]

On Thu, Aug 27, 2020 at 12:07 PM Chris Torek <chris.torek@gmail.com> wrote:
>
> 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.)

> > 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.

Re: post-checkout hook aborts rebase

[Chris Torek]

> On Thu, Aug 27, 2020 at 12:07 PM Chris Torek <chris.torek@gmail.com> wrote:
> > The *last* checkout from the finished rebase perhaps *should* [invoke
a hook]

On Thu, Aug 27, 2020 at 1:11 PM Elijah Newren <newren@gmail.com> wrote:
> 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.

I meant for the final state after rewriting the commits -- but that's
actually the post-rewrite hook, not the post-checkout hook.  I had them
conflated mentally when I wrote the above.

> 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.)

I agree with this.

Chris

Re: post-checkout hook aborts rebase

[Phillip Wood]

Hi Tom

On 27/08/2020 01:44, Tom Rutherford wrote:
> Thank you for the response Junio.
> 
> For what it's worth, my hook does not make changes to the repo. It's
> running a command to check that the installed version of our
> dependencies match the version specified in the commit being checked
> out, and merely warns if the two don't match (then exits with a
> nonzero return code).
> 
> For this reason it's been convenient that the hook runs during
> rebases, but I find it surprising that the nonzero return code would
> impact the rebase.

If the checkout succeeds that rebase does not print any of checkout's
output so unfortunately you wouldn't see the message from your hook.

I tend to agree with Junio that we shouldn't be running the 
post-checkout hook when rebasing.

Best Wishes

Phillip

> 
> Tom
> 
> On Wed, Aug 26, 2020 at 5:22 PM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> ...  If "git rebase" or whatever
>>> command wanted to place files and the index into some state by using
>>> "git checkout" command, and if the post-checkout hook mucked with
>>> the state in such a way that contradicts with what the "git rebase"
>>> command wanted them to be in, it is not surprising the hook's behavior
>>> broke "git rebase"'s operation.
>>
>> Having said all that, I actually think that "rebase" shouldn't be
>> invoking "git checkout" (and its equivalent) internally when
>> switching to a specific version, in such a way that it would trigger
>> any end-user specified hooks and allow them to muck with the working
>> tree and the index state.
>>
>> I haven't checked the actual implementation of "git rebase" for
>> quite some time to be sure, but we have lower-level plumbing
>> commands that are not affected by the end-user hooks for exactly
>> that kind of "build higher-level commands by synthesis of
>> lower-level machinery", and it is very possible that what we are
>> looking at is actually a bug that needs to be fixed.  I dunno.
>>
>> Thanks.

Re: post-checkout hook aborts rebase

[Tom Rutherford]

Thanks Phillip,

Yes, I've now noticed that's the case. :)

I also didn't intend to suggest that post-checkout *should* run during
rebases, just that it would be convenient for this particular use case
if it did.
I think I'll need to use multiple hooks to accomplish what I want to do.


Tom

On Wed, Sep 9, 2020 at 2:44 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> Hi Tom
>
> On 27/08/2020 01:44, Tom Rutherford wrote:
> > Thank you for the response Junio.
> >
> > For what it's worth, my hook does not make changes to the repo. It's
> > running a command to check that the installed version of our
> > dependencies match the version specified in the commit being checked
> > out, and merely warns if the two don't match (then exits with a
> > nonzero return code).
> >
> > For this reason it's been convenient that the hook runs during
> > rebases, but I find it surprising that the nonzero return code would
> > impact the rebase.
>
> If the checkout succeeds that rebase does not print any of checkout's
> output so unfortunately you wouldn't see the message from your hook.
>
> I tend to agree with Junio that we shouldn't be running the
> post-checkout hook when rebasing.
>
> Best Wishes
>
> Phillip
>
> >
> > Tom
> >
> > On Wed, Aug 26, 2020 at 5:22 PM Junio C Hamano <gitster@pobox.com> wrote:
> >>
> >> Junio C Hamano <gitster@pobox.com> writes:
> >>
> >>> ...  If "git rebase" or whatever
> >>> command wanted to place files and the index into some state by using
> >>> "git checkout" command, and if the post-checkout hook mucked with
> >>> the state in such a way that contradicts with what the "git rebase"
> >>> command wanted them to be in, it is not surprising the hook's behavior
> >>> broke "git rebase"'s operation.
> >>
> >> Having said all that, I actually think that "rebase" shouldn't be
> >> invoking "git checkout" (and its equivalent) internally when
> >> switching to a specific version, in such a way that it would trigger
> >> any end-user specified hooks and allow them to muck with the working
> >> tree and the index state.
> >>
> >> I haven't checked the actual implementation of "git rebase" for
> >> quite some time to be sure, but we have lower-level plumbing
> >> commands that are not affected by the end-user hooks for exactly
> >> that kind of "build higher-level commands by synthesis of
> >> lower-level machinery", and it is very possible that what we are
> >> looking at is actually a bug that needs to be fixed.  I dunno.
> >>
> >> Thanks.
>

Re: post-checkout hook aborts rebase

[Philippe Blain]

Hello everyone,

> Le 9 sept. 2020 à 05:43, Phillip Wood <phillip.wood123@gmail.com> a écrit :
> 
> Hi Tom
> 
> On 27/08/2020 01:44, Tom Rutherford wrote:
>> Thank you for the response Junio.
>> 
>> For what it's worth, my hook does not make changes to the repo. It's
>> running a command to check that the installed version of our
>> dependencies match the version specified in the commit being checked
>> out, and merely warns if the two don't match (then exits with a
>> nonzero return code).
>> 
>> For this reason it's been convenient that the hook runs during
>> rebases, but I find it surprising that the nonzero return code would
>> impact the rebase.
> 
> If the checkout succeeds that rebase does not print any of checkout's
> output so unfortunately you wouldn't see the message from your hook.

According to git-rebase's doc [1], that's true only for the new "merge" backend,
though, (which would be in use here in 2.28 unless 'rebase.backend' is set to 'apply').

On a slightly related note, I noticed that that part of the documentation is not 100% exact. I states:
"Further, both backends only call the post-checkout hook with the starting point commit of the rebase, 
not the intermediate commits nor the final commit."

but I noticed that in an interactive rebase, the post-checkout hook *is* called for commits
marked as "edit", when the rebase stops to edit them (I haven't checked if it's called when the rebase stops
for other reasons, like conflicts.)

Maybe we should tweak the wording of the doc  ?

Cheers,

Philippe.

[1] https://git-scm.com/docs/git-rebase#_hooks

Re: post-checkout hook aborts rebase

[Junio C Hamano]

Philippe Blain <levraiphilippeblain@gmail.com> writes:

> On a slightly related note, I noticed that that part of the documentation is not 100% exact. I states:
> "Further, both backends only call the post-checkout hook with the starting point commit of the rebase, 
> not the intermediate commits nor the final commit."
>
> but I noticed that in an interactive rebase, the post-checkout hook *is* called for commits
> marked as "edit", when the rebase stops to edit them (I haven't checked if it's called when the rebase stops
> for other reasons, like conflicts.)
>
> Maybe we should tweak the wording of the doc  ?

Yeah, we should do either that, or fix the code and make it not to
call the post-checkout hook so aggressively, as suggested during the
discussion.

Thanks.

Re: post-checkout hook aborts rebase

[Elijah Newren]

On Wed, Aug 26, 2020 at 5:24 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Junio C Hamano <gitster@pobox.com> writes:
>
> > ...  If "git rebase" or whatever
> > command wanted to place files and the index into some state by using
> > "git checkout" command, and if the post-checkout hook mucked with
> > the state in such a way that contradicts with what the "git rebase"
> > command wanted them to be in, it is not surprising the hook's behavior
> > broke "git rebase"'s operation.
>
> Having said all that, I actually think that "rebase" shouldn't be
> invoking "git checkout" (and its equivalent) internally when
> switching to a specific version, in such a way that it would trigger
> any end-user specified hooks and allow them to muck with the working
> tree and the index state.
>
> I haven't checked the actual implementation of "git rebase" for
> quite some time to be sure, but we have lower-level plumbing
> commands that are not affected by the end-user hooks for exactly
> that kind of "build higher-level commands by synthesis of
> lower-level machinery", and it is very possible that what we are
> looking at is actually a bug that needs to be fixed.  I dunno.
>
> Thanks.

Yes, and I think we should also make rebase stop invoking "git commit" too.

Re: post-checkout hook aborts rebase

[Junio C Hamano]

Elijah Newren <newren@gmail.com> writes:

> On Wed, Aug 26, 2020 at 5:24 PM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>> > ...  If "git rebase" or whatever
>> > command wanted to place files and the index into some state by using
>> > "git checkout" command, and if the post-checkout hook mucked with
>> > the state in such a way that contradicts with what the "git rebase"
>> > command wanted them to be in, it is not surprising the hook's behavior
>> > broke "git rebase"'s operation.
>>
>> Having said all that, I actually think that "rebase" shouldn't be
>> invoking "git checkout" (and its equivalent) internally when
>> switching to a specific version, in such a way that it would trigger
>> any end-user specified hooks and allow them to muck with the working
>> tree and the index state.
>>
>> I haven't checked the actual implementation of "git rebase" for
>> quite some time to be sure, but we have lower-level plumbing
>> commands that are not affected by the end-user hooks for exactly
>> that kind of "build higher-level commands by synthesis of
>> lower-level machinery", and it is very possible that what we are
>> looking at is actually a bug that needs to be fixed.  I dunno.
>>
>> Thanks.
>
> Yes, and I think we should also make rebase stop invoking "git commit" too.

Note that I didn't say we should make it stop invoking "git
checkout".

We could invent a mechanism that disables all the hook invocations
and other customizations [*1*] (done e.g. via the configuration
variables) for internal use of the Porcelain commands, and use it
when "rebase" invokes Porcelains like "checkout", "commit" as its
implementation detail, for example (some "invocations" I think
bypass the run_command() inteface and instead done by directly
calling the implementation detail of "checkout" and "commit", but
the principles are the same).

[Footnote]

*1* of course, it becomes a balancing act to decide what kind of
    customizations are OK to honor under such a mode.

Re: post-checkout hook aborts rebase

[Elijah Newren]

On Thu, Aug 27, 2020 at 10:28 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Elijah Newren <newren@gmail.com> writes:
>
> > On Wed, Aug 26, 2020 at 5:24 PM Junio C Hamano <gitster@pobox.com> wrote:
> >>
> >> Junio C Hamano <gitster@pobox.com> writes:
> >>
> >> > ...  If "git rebase" or whatever
> >> > command wanted to place files and the index into some state by using
> >> > "git checkout" command, and if the post-checkout hook mucked with
> >> > the state in such a way that contradicts with what the "git rebase"
> >> > command wanted them to be in, it is not surprising the hook's behavior
> >> > broke "git rebase"'s operation.
> >>
> >> Having said all that, I actually think that "rebase" shouldn't be
> >> invoking "git checkout" (and its equivalent) internally when
> >> switching to a specific version, in such a way that it would trigger
> >> any end-user specified hooks and allow them to muck with the working
> >> tree and the index state.
> >>
> >> I haven't checked the actual implementation of "git rebase" for
> >> quite some time to be sure, but we have lower-level plumbing
> >> commands that are not affected by the end-user hooks for exactly
> >> that kind of "build higher-level commands by synthesis of
> >> lower-level machinery", and it is very possible that what we are
> >> looking at is actually a bug that needs to be fixed.  I dunno.
> >>
> >> Thanks.
> >
> > Yes, and I think we should also make rebase stop invoking "git commit" too.
>
> Note that I didn't say we should make it stop invoking "git
> checkout".

Understood that you didn't say that, but I am of the opinion that we
should do that.

Invoking "git checkout" and "git commit" were convenient
implementation details when rebase was written as a script.  When it
was rewritten in C, forking out to these processes made for an easy
conversion path (even if slightly ugly).  But forking other processes
is costly, it has given us multiple reports of unwanted side-effects
from hooks[1], it makes the code more difficult to debug in a
debugger, etc.  I think these are all problems we could avoid by no
longer calling these external commands.

> We could invent a mechanism that disables all the hook invocations
> and other customizations [*1*] (done e.g. via the configuration
> variables) for internal use of the Porcelain commands, and use it
> when "rebase" invokes Porcelains like "checkout", "commit" as its
> implementation detail, for example (some "invocations" I think
> bypass the run_command() inteface and instead done by directly
> calling the implementation detail of "checkout" and "commit", but
> the principles are the same).
>
> [Footnote]
>
> *1* of course, it becomes a balancing act to decide what kind of
>     customizations are OK to honor under such a mode.

That'd be one way to solve it, but it feels like it'd push maintenance
burden onto future folks that people touching the commit and checkout
builtins have to be aware of what other commands are using them under
the covers and tweak them appropriately.  I'd rather checkout and
commit just shared the relevant bits of important code with sequencer,
and then if people want to add configuration bits (be that hooks or
config settings or whatever), then it only gets added to the commands
that people explicitly add them to, rather than them getting added to
rebase via accident.