git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Thomas Rast <trast@student.ethz.ch>
Cc: <git@vger.kernel.org>,
	"Martin von Zweigbergk" <martin.von.zweigbergk@gmail.com>,
	Shezan Baig <shezbaig.wk@gmail.com>
Subject: Re: [PATCH] rebase -i: avoid checking out $branch when possible
Date: Fri, 20 Apr 2012 13:06:19 -0700	[thread overview]
Message-ID: <xmqqmx66mamc.fsf@junio.mtv.corp.google.com> (raw)
In-Reply-To: <87lilqjstf.fsf@thomas.inf.ethz.ch> (Thomas Rast's message of "Fri, 20 Apr 2012 18:01:32 +0200")

Thomas Rast <trast@student.ethz.ch> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Thomas Rast <trast@student.ethz.ch> 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.

  reply	other threads:[~2012-04-20 20:06 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-20 15:05 [PATCH] rebase -i: avoid checking out $branch when possible Thomas Rast
2012-04-20 15:52 ` Junio C Hamano
2012-04-20 16:01   ` Thomas Rast
2012-04-20 20:06     ` Junio C Hamano [this message]
2012-05-15 16:26       ` Shezan Baig
2012-05-15 17:08         ` Junio C Hamano
2012-05-18  8:27           ` Thomas Rast
2012-05-18 15:25             ` Junio C Hamano
2012-04-20 16:45 ` Johannes Sixt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqmx66mamc.fsf@junio.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=martin.von.zweigbergk@gmail.com \
    --cc=shezbaig.wk@gmail.com \
    --cc=trast@student.ethz.ch \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).