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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Thomas Rast <trast@xxxxxxxxxxxxxxx> writes:

> Junio C Hamano <gitster@xxxxxxxxx> writes:
>
>> Thomas Rast <trast@xxxxxxxxxxxxxxx> writes:
>>
>>> I was a bit torn on whether I should abort with checkout, or without
>>> it.  The manual clearly states that rebase "will perform an automatic
>>> git checkout <branch> before doing anything else", which mandates at
>>> least *trying* the checkout in the error path, hence this version.
>>>
>>> However, in contrived cases this can lead to strange behavior.  For
>>> example, a checkout conflict with a file in the worktree may prevent
>>> the abort path from working correctly, even though going through with
>>> the rebase itself may succeed.
>>
>> Given all that contortion, is it even worth doing this?
>
> Well, the logic isn't new; 0cb0664 already does the same.  It just never
> carried over to interactive rebase.

OK.

The discrepancy in the "abort" case may come only in the three cases:

 - EDITOR is pointing at something funny; it is not worth introducing
   any backward incompatibility to optimize for this case, so
   abort-with-checkout is the right thing to do here.  One less thing we
   have to worry about (but see the third point below).

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

   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.

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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]