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

[Thomas Rast <trast@student.ethz.ch>]

The command

  git rebase [-i] [--onto $onto] $base $branch

is defined to be equivalent to

  git checkout $branch && git rebase [-i] [--onto $onto] $base

However, we do not have to actually perform the checkout.  The rebase
starts building on top of $base (or $onto, if given).  The tree
_state_ (not diff) of $branch is irrelevant.  Actually performing the
checkout has some downsides: $branch may potentially be way out of
touch with HEAD, and thus e.g. trigger a full rebuild in a timestamp-
based build system, even if $base..$branch only edits the README.

In the event that $branch is already up-to-date w.r.t. $base, we still
have to check out, however.  git-rebase.sh has had the corresponding
lazy-checkout logic since approximately forever (0cb0664, rebase
[--onto O] A B: omit needless checkout, 2008-03-15).

This logic has also been used for interactive since Martin's
refactoring around cc1453e (rebase: factor out call to pre-rebase
hook, 2011-02-06).  However, an unconditional checkout was carried
over into the new interactive rebase code.  Remove it.  HEAD is only
used in the rev-parse invocation immediately after, which is easy
enough to fix.

Note that this does change the state of the repository if something
bad happens and we 'die'.  Noninteractive rebase already behaves the
same way.  The catch here is that "there is nothing to rebase", as
well as "the user cleared the TODO file", both go through an error
path.  We need to ensure that a checkout happens in these cases, to
keep it equivalent to checkout&&rebase.

Noticed-by: Shezan Baig <shezbaig.wk@gmail.com>
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

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.

 git-rebase--interactive.sh |   23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 2e13258..3b40c2a 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -163,6 +163,15 @@ die_abort () {
 	die "$1"
 }
 
+die_abort_with_checkout () {
+	if test -n "$switch_to"
+	then
+		git checkout "$switch_to" -- ||
+			die_abort "$1, and failed to check out $switch_to"
+	fi
+	die_abort "$1"
+}
+
 has_action () {
 	sane_grep '^[^#]' "$1" >/dev/null
 }
@@ -728,13 +737,7 @@ git var GIT_COMMITTER_IDENT >/dev/null ||
 
 comment_for_reflog start
 
-if test ! -z "$switch_to"
-then
-	output git checkout "$switch_to" -- ||
-		die "Could not checkout $switch_to"
-fi
-
-orig_head=$(git rev-parse --verify HEAD) || die "No HEAD?"
+orig_head=$(git rev-parse --verify "${switch_to:-HEAD}") || die "No HEAD?"
 mkdir "$state_dir" || die "Could not create temporary $state_dir"
 
 : > "$state_dir"/interactive || die "Could not mark as interactive"
@@ -854,14 +857,14 @@ cat >> "$todo" << EOF
 EOF
 
 has_action "$todo" ||
-	die_abort "Nothing to do"
+	die_abort_with_checkout "Nothing to do"
 
 cp "$todo" "$todo".backup
 git_sequence_editor "$todo" ||
-	die_abort "Could not execute editor"
+	die_abort_with_checkout "Could not execute editor"
 
 has_action "$todo" ||
-	die_abort "Nothing to do"
+	die_abort_with_checkout "Nothing to do"
 
 test -d "$rewritten" || test -n "$force_rebase" || skip_unnecessary_picks
 
-- 
1.7.10.323.g7b65b