Re: [PATCH] rebase -i: avoid checking out $branch when possible

From: Thomas Rast <trast@inf.ethz.ch>
Date: May 18, 2012, 08:27 UTC
Message-ID: <87obplhp12.fsf@thomas.inf.ethz.ch>
In-Reply-To: <7vwr4de5j8.fsf@alter.siamese.dyndns.org>

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

Show 9 quoted lines

> Shezan Baig <shezbaig.wk@gmail.com> writes:
>
>> Just wondering if this patch is going to be available in an upcoming
>> version of git?
>
> As we can see from the exchange you quoted, I do not think we have nailed
> the details of desired behaviour in the updated code down.
>
> Thomas, how would you want to proceed?

Hrm.  You wrote:

> The discrepancy in the "abort" case may come only in the three cases:
> 
>  - EDITOR is pointing at something funny, [let's do the checkout to be
>    safe for scripts]

Agreed.

Show 8 quoted lines

>  - The user told us not to do the rebase by making insn sheet empty.  In
>    other words, this is "aborting the entire rebase", so ideally it
>    should come back to the state before the user ran "git rebase"
>    command (i.e. where she was before we switched to <branch>).
> 
>    I do not think this ideal behaviour is something neither batch or
>    interactive rebase has traditionally implemented, but I can see how
>    we can sell this as a bugfix to the end users.

That's a convincing argument, so let's make it so.

Show 7 quoted lines

>  - It turns out that everything is already contained and there is
>    nothing to apply, i.e. after this sequence:
> 
> 	git checkout branch
> 	git checkout $onto_or_merge_base_between_base_and_branch
> 
>    we find out that "git cherry $onto_or_merge_base branch" is empty.

Is there a command missing here? This alone does not make them the same, perhaps you meant some resetting.

I'll assume that you meant a case where the user is *not* on branch, but base/onto is an ancestor of the branch, e.g., in a sequence

  git checkout base
  git reset --hard branch~5
  # now branch..base is empty
  git rebase base branch

the rebase will not do anything unless forced. Similarly for the case where base==branch. And then

Show 5 quoted lines

>    Because you will be one commit ahead of $onto_or_merge_base if "git
>    cherry" were to give one commit to be replayed, I think it is
>    logically correct if you stayed at the $onto_or_merge_base without
>    checking out <branch>.  In other words, abort-with-checkout is not
>    ideal for this case; we would want to just abort in this case.

This would be hard to sell, because a script that runs

  git rebase base branch

could reasonably assume that after the rebase, the current branch is 'branch', unless some error occurred. And we'd be breaking that.

Or am I missing the point of your reasoning?

-- 
Thomas Rast
trast@{inf,student}.ethz.ch