Re: [PATCH] Fix rebase -p --onto

[Greg Price <price@ksplice.com>]

On Fri, Jul 17, 2009 at 10:40:08AM +0200, Johannes Sixt wrote:
> I have used rebase -i -p in the past to rewrite history that involves
> merges of topic branches like this:
> 
>   ---------Y--M--M--F     <-- master
>              /  /
>   ----a--a--a  /
>               /
>   --b--b--b--b
> 
> where F is a fixup that I want to insert between Y and M, and I thought
> rebase -i -p was intended for this use-case.

I don't believe rebase -i -p has ever worked with reordering commits.
It certainly doesn't now, as the tests below demonstrate both for your
case and for the most trivial possible situation without even any merges.

This is not a mere coding error, as in the general case with a hairy
set of merges it's not clear what an arbitrary reordering of the
commits would even mean.  To really fix it will require a richer TODO
file format, as in the sequencer or the discussion Johannes Schindelin
started in January about a rebase -i -p rework.


In any case, this has nothing to do with my patch to fix -p --onto,
which is a no-op when --onto is not used.  You want `git rebase -i -p Y`,
and the generation of the TODO file correctly gives you M, M, F.
It's the execution of the TODO file you give back, with F, M, M,
that does not do what you want.

Cheers,
Greg



>From 185fa9da4caee5a0a96105e60269d02ec832e876 Mon Sep 17 00:00:00 2001
From: Greg Price <price@ksplice.com>
Date: Fri, 17 Jul 2009 11:55:11 -0400
Subject: [PATCH] Add failing test for commit reordering in rebase -i -p

Signed-off-by: Greg Price <price@ksplice.com>
---
 t/t3411-rebase-preserve-around-merges.sh |   59 ++++++++++++++++++++++++++++++
 1 files changed, 59 insertions(+), 0 deletions(-)

diff --git a/t/t3411-rebase-preserve-around-merges.sh b/t/t3411-rebase-preserve-around-merges.sh
index 6533505..1759a98 100755
--- a/t/t3411-rebase-preserve-around-merges.sh
+++ b/t/t3411-rebase-preserve-around-merges.sh
@@ -71,4 +71,63 @@ test_expect_success 'rebase two levels of merge' '
 	test "$(git rev-parse HEAD^2^1^1)" = "$(git rev-parse HEAD^2^2^1)"
 '
 
+# Reorder commits while using -p.  Basic prerequisite for the next test
+# to have a hope of working.
+#
+# a---b---c
+#
+# want
+#
+# a---c---b
+
+test_expect_failure 'reorder with -p' '
+	test_commit a &&
+	test_commit b &&
+	test_commit c &&
+	FAKE_LINES="2 1" git rebase -i -p HEAD~2 &&
+	test "$(git rev-parse HEAD~2)" = "$(git rev-parse a)" &&
+	git log -n1 --pretty=format:%s HEAD | grep b &&
+	git log -n1 --pretty=format:%s HEAD^ | grep c
+'
+
+
+# Reorder a commit to before a merge.  From
+#
+# R---x---Ma--Mb--F
+#  \     /   /
+#   a1--a2  /
+#    \     /
+#     b1--b2
+#
+# we get
+#
+# R---x---F*--Ma*--Mb*
+#  \         /    /
+#   a1--a2--/    /
+#    \          /
+#     b1--b2---/
+
+test_expect_failure 'rewrite to before merge' '
+	test_commit R &&
+	test_commit a1 &&
+	test_commit b1 &&
+	test_commit b2 &&
+	git reset --hard a1 &&
+	test_commit a2 &&
+	git reset --hard R &&
+	test_commit x &&
+	test_merge Ma a2 &&
+	test_merge Mb b2 &&
+	test_commit F &&
+
+	FAKE_LINES="3 1 2" git rebase -i -p x &&
+	git log --pretty=oneline HEAD && echo &&
+	git log --pretty=oneline HEAD^2 && echo &&
+	git log --pretty=oneline HEAD^^2 && echo &&
+	git log --pretty=oneline HEAD^^ && echo &&
+	test "$(git rev-parse HEAD^2)" = "$(git rev-parse b2)" &&
+	test "$(git rev-parse HEAD^1^2)" = "$(git rev-parse a2)" &&
+	test "$(git rev-parse HEAD~3)" = "$(git rev-parse x)"
+'
+
 test_done
-- 
1.6.3.1.499.ge7b8da