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

From: Thomas Rast <trast@student.ethz.ch>
Date: Apr 20, 2012, 15:05 UTC
Message-ID: <fee3225e29915e1b61e29a5d2fe37db20fa4b596.1334933837.git.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