Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

rebase -i -p: include non-first-parent commits in todo list

Consider this graph:

        D---E    (topic, HEAD)
       /   /
  A---B---C      (master)
   \
    F            (topic2)

and the following three commands:
  1. git rebase -i -p A
  2. git rebase -i -p --onto F A
  3. git rebase -i -p B

Currently, (1) and (2) will pick B, D, C, and E onto A and F,
respectively.  However, (3) will only pick D and E onto B, but not C,
which is inconsistent with (1) and (2).  As a result, we cannot modify C
during the interactive-rebase.

The current behavior also creates a bug if we do:
  4. git rebase -i -p C

In (4), E is never picked.  And since interactive-rebase resets "HEAD"
to "onto" before picking any commits, D and E are lost after the
interactive-rebase.

This patch fixes the inconsistency and bug by ensuring that all children
of upstream are always picked.  This essentially reverts the commit:
  d80d6bc

When compiling the todo list, commits reachable from "upstream" should
never be skipped under any conditions.  Otherwise, we lose the ability
to modify them like (3), and create a bug like (4).

Two of the tests contain a scenario like (3).  Since the new behavior
added more commits for picking, these tests need to be updated to
account for the additional pick lines.  A new test has also been added
for (4).

Signed-off-by: Andrew Wong <andrew.kw.w@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information...
commit 12bf828348922935b1d5ca10f2d829c9a64b6e41 1 parent 2c162b5
Andrew Wong authored June 18, 2011 gitster committed June 19, 2011
3  git-rebase--interactive.sh
@@ -713,7 +713,6 @@ then
713 713
 	# parents to rewrite and skipping dropped commits would
714 714
 	# prematurely end our probe
715 715
 	merges_option=
716  
-	first_after_upstream="$(git rev-list --reverse --first-parent $upstream..$orig_head | head -n 1)"
717 716
 else
718 717
 	merges_option="--no-merges --cherry-pick"
719 718
 fi
@@ -746,7 +745,7 @@ do
746 745
 			preserve=t
747 746
 			for p in $(git rev-list --parents -1 $sha1 | cut -d' ' -s -f2-)
748 747
 			do
749  
-				if test -f "$rewritten"/$p -a \( $p != $onto -o $sha1 = $first_after_upstream \)
  748
+				if test -f "$rewritten"/$p
750 749
 				then
751 750
 					preserve=f
752 751
 				fi
2  t/t3404-rebase-interactive.sh
@@ -295,7 +295,7 @@ test_expect_success 'preserve merges with -p' '
295 295
 '
296 296
 
297 297
 test_expect_success 'edit ancestor with -p' '
298  
-	FAKE_LINES="1 edit 2 3 4" git rebase -i -p HEAD~3 &&
  298
+	FAKE_LINES="1 2 edit 3 4" git rebase -i -p HEAD~3 &&
299 299
 	echo 2 > unrelated-file &&
300 300
 	test_tick &&
301 301
 	git commit -m L2-modified --amend unrelated-file &&
28  t/t3409-rebase-preserve-merges.sh
@@ -37,7 +37,15 @@ export GIT_AUTHOR_EMAIL
37 37
 #      \
38 38
 #       B2     <-- origin/topic
39 39
 #
40  
-# In all cases, 'topic' is rebased onto 'origin/topic'.
  40
+# Clone 4 (merge using second parent as base):
  41
+#
  42
+# A1--A2--B3   <-- origin/master
  43
+#  \
  44
+#   B1--A3--M  <-- topic
  45
+#    \     /
  46
+#     \--A4    <-- topic2
  47
+#      \
  48
+#       B2     <-- origin/topic
41 49
 
42 50
 test_expect_success 'setup for merge-preserving rebase' \
43 51
 	'echo First > A &&
@@ -57,6 +65,13 @@ test_expect_success 'setup for merge-preserving rebase' \
57 65
 	git merge origin/master
58 66
 	) &&
59 67
 
  68
+	git clone ./. clone4 &&
  69
+	(
  70
+		cd clone4 &&
  71
+		git checkout -b topic origin/topic &&
  72
+		git merge origin/master
  73
+	) &&
  74
+
60 75
 	echo Fifth > B &&
61 76
 	git add B &&
62 77
 	git commit -m "Add different B" &&
@@ -123,4 +138,15 @@ test_expect_success 'rebase -p preserves no-ff merges' '
123 138
 	)
124 139
 '
125 140
 
  141
+test_expect_success 'rebase -p works when base inside second parent' '
  142
+	(
  143
+	cd clone4 &&
  144
+	git fetch &&
  145
+	git rebase -p HEAD^2 &&
  146
+	test 1 = $(git rev-list --all --pretty=oneline | grep "Modify A" | wc -l) &&
  147
+	test 1 = $(git rev-list --all --pretty=oneline | grep "Modify B" | wc -l) &&
  148
+	test 1 = $(git rev-list --all --pretty=oneline | grep "Merge remote-tracking branch " | wc -l)
  149
+	)
  150
+'
  151
+
126 152
 test_done
2  t/t3411-rebase-preserve-around-merges.sh
@@ -37,7 +37,7 @@ test_expect_success 'setup' '
37 37
 #        -- C1 --
38 38
 #
39 39
 test_expect_success 'squash F1 into D1' '
40  
-	FAKE_LINES="1 squash 3 2" git rebase -i -p B1 &&
  40
+	FAKE_LINES="1 squash 4 2 3" git rebase -i -p B1 &&
41 41
 	test "$(git rev-parse HEAD^2)" = "$(git rev-parse C1)" &&
42 42
 	test "$(git rev-parse HEAD~2)" = "$(git rev-parse B1)" &&
43 43
 	git tag E2

0 notes on commit 12bf828

Please sign in to comment.
Something went wrong with that request. Please try again.