Re: [PATCH 2/2] built-in rebase --autostash: leave the current branch alone if possible
- Date: Wed, 7 Nov 2018 15:53:29 -0500
- From: Jeff King <peff@xxxxxxxx>
- Subject: Re: [PATCH 2/2] built-in rebase --autostash: leave the current branch alone if possible
On Wed, Nov 07, 2018 at 06:00:50AM -0800, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@xxxxxx>
> When we converted a `git reset --hard` call in the original Unix shell
> script to built-in code, we asked to reset the worktree and the index
> and explicitly *not* to detach the HEAD. By mistake, though, we still
> did. Let's fix this.
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index 0ee06aa363..4a608d0a78 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -613,7 +613,8 @@ static int reset_head(struct object_id *oid, const char *action,
> reflog_head = msg.buf;
> if (!switch_to_branch)
> - ret = update_ref(reflog_head, "HEAD", oid, orig, REF_NO_DEREF,
> + ret = update_ref(reflog_head, "HEAD", oid, orig,
> + detach_head ? REF_NO_DEREF : 0,
This makes sense. There are actually a bunch of calls that pass
detach_head==0, besides the one related to autostash. I suspect for most
of them it does not matter, because either:
1. We are already on a detached HEAD, since we detach as the first
step of the rebase. So for the call in ACTION_SKIP, for example, we
probably cannot trigger the problem.
2. They pass a switch_to_branch arg, so we do not hit this code path
anyway (the call to fast-forward is like this, for example).
So there may be other ways to trigger the problem, but I didn't dig.
Either way, your fix is clearly the right thing to do.