Re* git pull --rebase should use fast forward merge if possible

[Junio C Hamano <gitster@pobox.com>]

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

> die_no_merge_candidates() would have triggered, I would imagine.
>
> Note that I won't be applying this without test updates and proper log message,
> so no need to worry about the style yet ;-)

This time with a bit of explanation in the log and a trivial test.

I am no longer sure if this is a good idea myself, though.  The
trivial case the new test covers is not interesting, even though it
may be worth protecting the behaviour with the test.  What's more
interesting to think about is what should happen in various corner
cases.  E.g. what should happen when there are local changes that
would conflict with the fast-forwarding?  what should happen if the
user has rebase.autostash set in such a case?  etc.

-- >8 --
Subject: [PATCH] pull: fast-forward "pull --rebase=true"

"git pull --rebase" always runs "git rebase" after fetching the
commit to serve as the new base, even when the new base is a
descendant of the current HEAD, i.e. we haven't done any work.

In such a case, we can instead fast-forward to the new base without
invoking the rebase process.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/pull.c  | 22 ++++++++++++++++++----
 t/t5520-pull.sh | 17 +++++++++++++++++
 2 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index bf3fd3f9c8..2a41d415b2 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -878,10 +878,24 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 		if (merge_heads.nr > 1)
 			die(_("Cannot merge multiple branches into empty head."));
 		return pull_into_void(*merge_heads.sha1, curr_head);
-	} else if (opt_rebase) {
-		if (merge_heads.nr > 1)
-			die(_("Cannot rebase onto multiple branches."));
+	}
+	if (opt_rebase && merge_heads.nr > 1)
+		die(_("Cannot rebase onto multiple branches."));
+
+	if (opt_rebase) {
+		struct commit_list *list = NULL;
+		struct commit *merge_head, *head;
+
+		head = lookup_commit_reference(orig_head);
+		commit_list_insert(head, &list);
+		merge_head = lookup_commit_reference(merge_heads.sha1[0]);
+		if (is_descendant_of(merge_head, list)) {
+			/* we can fast-forward this without invoking rebase */
+			opt_ff = "--ff-only";
+			return run_merge();
+		}
 		return run_rebase(curr_head, *merge_heads.sha1, rebase_fork_point);
-	} else
+	} else {
 		return run_merge();
+	}
 }
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index a0013ee32f..7887b6d97b 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -237,6 +237,23 @@ test_expect_success '--rebase' '
 	test new = "$(git show HEAD:file2)"
 '
 
+test_expect_success '--rebase fast forward' '
+	git reset --hard before-rebase &&
+	git checkout -b ff &&
+	echo another modification >file &&
+	git commit -m third file &&
+
+	git checkout to-rebase &&
+	git pull --rebase . ff &&
+	test "$(git rev-parse HEAD)" = "$(git rev-parse ff)" &&
+
+	# The above only validates the result.  Did we actually bypass rebase?
+	git reflog -1 >reflog.actual &&
+	sed "s/^[0-9a-f][0-9a-f]*/OBJID/" reflog.actual >reflog.fuzzy &&
+	echo "OBJID HEAD@{0}: pull --rebase . ff: Fast-forward" >reflog.expected &&
+	test_cmp reflog.expected reflog.fuzzy
+'
+
 test_expect_success '--rebase fails with multiple branches' '
 	git reset --hard before-rebase &&
 	test_must_fail git pull --rebase . copy master 2>err &&
-- 
2.11.0-192-gbadfaabe38