Re: [PATCH 2/2] built-in rebase --autostash: leave the current branch alone if possible

From: Jeff King <peff@peff.net>
Date: Nov 7, 2018, 20:53 UTC
Message-ID: <20181107205329.GB30078@sigill.intra.peff.net>
In-Reply-To: <a2f0b53db269e9a6d7c29b665c9be53bd2f30e42.1541599246.git.gitgitgadget@gmail.com>

On Wed, Nov 07, 2018 at 06:00:50AM -0800, Johannes Schindelin via GitGitGadget wrote:

Show 19 quoted lines

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> 
> 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,
>  				 UPDATE_REFS_MSG_ON_ERR);

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.

-Peff