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: d80d6bc146232d81f1bb4bc58e5d89263fd228d4 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>

Andrew Wong committed Jun 18, 2011 at 18:12 UTC 12bf828348922935b1d5ca10f2d829c9a64b6e41
4 files changed +30 -5
git-rebase--interactive.sh
+1 -2
@@ -713,7 +713,6 @@ then
713 # parents to rewrite and skipping dropped commits would
714 # prematurely end our probe
715 merges_option=
716 - first_after_upstream="$(git rev-list --reverse --first-parent $upstream..$orig_head | head -n 1)"
716 else
717 merges_option="--no-merges --cherry-pick"
718 fi
@@ -746,7 +745,7 @@ do
745 preserve=t
746 for p in $(git rev-list --parents -1 $sha1 | cut -d' ' -s -f2-)
747 do
749 - if test -f "$rewritten"/$p -a \( $p != $onto -o $sha1 = $first_after_upstream \)
748 + if test -f "$rewritten"/$p
749 then
750 preserve=f
751 fi
t/t3404-rebase-interactive.sh
+1 -1
@@ -295,7 +295,7 @@ test_expect_success 'preserve merges with -p' '
295 '
296
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 echo 2 > unrelated-file &&
300 test_tick &&
301 git commit -m L2-modified --amend unrelated-file &&
t/t3409-rebase-preserve-merges.sh
+27 -1
@@ -37,7 +37,15 @@ export GIT_AUTHOR_EMAIL
37 # \
38 # B2 <-- origin/topic
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
49
50 test_expect_success 'setup for merge-preserving rebase' \
51 'echo First > A &&
@@ -57,6 +65,13 @@ test_expect_success 'setup for merge-preserving rebase' \
65 git merge origin/master
66 ) &&
67
68 + git clone ./. clone4 &&
69 + (
70 + cd clone4 &&
71 + git checkout -b topic origin/topic &&
72 + git merge origin/master
73 + ) &&
74 +
75 echo Fifth > B &&
76 git add B &&
77 git commit -m "Add different B" &&
@@ -123,4 +138,15 @@ test_expect_success 'rebase -p preserves no-ff merges' '
138 )
139 '
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 +
152 test_done
t/t3411-rebase-preserve-around-merges.sh
+1 -1
@@ -37,7 +37,7 @@ test_expect_success 'setup' '
37 # -- C1 --
38 #
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 test "$(git rev-parse HEAD^2)" = "$(git rev-parse C1)" &&
42 test "$(git rev-parse HEAD~2)" = "$(git rev-parse B1)" &&
43 git tag E2