git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH/RFC] rebase -p: do not redo the merge, but cherry-pick first-parent changes
@ 2012-05-21 20:19 Johannes Sixt
  2012-05-22 18:23 ` Junio C Hamano
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Johannes Sixt @ 2012-05-21 20:19 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Stephen Haberman, Andrew Wong

When rebase -p had to replay a merge commit, it used to redo the merge.
But this has drawbacks:

- When the merge was evil, i.e., contained changes that are in neither of
  the parents, that change was not preserved.

- The 'git merge' invocation passed the commit message of the old merge
  commit, but it still obeyed the merge.log option. If it was set, the log
  ended up twice in the commit message.

Replace the 'git merge' by a cherry-pick that picks the changes that the
merge introduces with respect to the first parent.

The change in t3409 reflects the new situation after a conflicting merge
is replayed: Earlier, 'git merge' found a conflict in an add/add
situation with file B only in stage 2 and 3, now it is a regular three-way
merge conflict.

A test in  t3410 fails now with this new implementation. The branch that
is merged in by the old history and that is to be preserved is already
merged into the new upstream. In the old implementation, 'git merge'
detected this situation and succeeded with "Already up to date", but the
new implementation does not notice that it is pointless to keep the merge.
It just tries to follow instructions, but fails due to a conflicting
cherry-pick. This is a corner case whose correct solution should be that
the "pick" instruction is not generated in the first place.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 This is RFC because of the new failure addressed in the last paragraph
 above (see also the art work in the patch context below), how serious
 the failure is, and what to do about it.

 See also this recent thread for further motivation:
 http://thread.gmane.org/gmane.comp.version-control.git/194434/focus=194737

 git-rebase--interactive.sh                | 12 +++++++-----
 t/t3409-rebase-preserve-merges.sh         |  2 +-
 t/t3410-rebase-preserve-dropped-merges.sh |  4 ++--
 3 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 0c19b7c..642daab 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -312,12 +312,14 @@ pick_one_preserving_merges () {
 			msg_content="$(commit_message $sha1)"
 			# No point in merging the first parent, that's HEAD
 			new_parents=${new_parents# $first_parent}
-			if ! do_with_author output \
-				git merge --no-ff ${strategy:+-s $strategy} -m \
-					"$msg_content" $new_parents
+			printf "%s\n" $new_parents >"$GIT_DIR"/MERGE_HEAD
+			printf "%s\n" "$msg_content" >"$GIT_DIR"/MERGE_MSG
+			if output git cherry-pick -m 1 -n "$sha1"
 			then
-				printf "%s\n" "$msg_content" > "$GIT_DIR"/MERGE_MSG
-				die_with_patch $sha1 "Error redoing merge $sha1"
+				do_with_author output git commit --no-verify -F "$GIT_DIR"/MERGE_MSG ||
+					die_with_patch $sha1 "Could not replay merge $sha1"
+			else
+				die_with_patch $sha1 "Could not pick merge $sha1"
 			fi
 			echo "$sha1 $(git rev-parse HEAD^0)" >> "$rewritten_list"
 			;;
diff --git a/t/t3409-rebase-preserve-merges.sh b/t/t3409-rebase-preserve-merges.sh
index 6de4e22..7161b99 100755
--- a/t/t3409-rebase-preserve-merges.sh
+++ b/t/t3409-rebase-preserve-merges.sh
@@ -116,7 +116,7 @@ test_expect_success '--continue works after a conflict' '
 	cd clone2 &&
 	git fetch &&
 	test_must_fail git rebase -p origin/topic &&
-	test 2 = $(git ls-files B | wc -l) &&
+	test 3 = $(git ls-files B | wc -l) &&
 	echo Resolved again > B &&
 	test_must_fail git rebase --continue &&
 	grep "^@@@ " .git/rebase-merge/patch &&
diff --git a/t/t3410-rebase-preserve-dropped-merges.sh b/t/t3410-rebase-preserve-dropped-merges.sh
index 6f73b95..36c69b2 100755
--- a/t/t3410-rebase-preserve-dropped-merges.sh
+++ b/t/t3410-rebase-preserve-dropped-merges.sh
@@ -60,12 +60,12 @@ test_expect_success 'skip same-resolution merges with -p' '
 # A - B - C - D - E
 #   \             \ \
 #     F - G - H -- L2 \        -->   L2
 #       \             |                \
 #         I -- G3 --- J2 -- K2           I -- G3 -- K2
-# G2 = different changes as G
-test_expect_success 'keep different-resolution merges with -p' '
+# G3 = different changes as G
+test_expect_failure 'keep different-resolution merges with -p' '
 	git checkout H &&
 	test_must_fail git merge E &&
 	test_commit L2 file1 23 &&
 	git checkout I &&
 	test_commit G3 file1 4 &&
-- 
1.7.10.2.529.g0c18cfd

^ permalink raw reply related	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2012-05-25 20:03 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-05-21 20:19 [PATCH/RFC] rebase -p: do not redo the merge, but cherry-pick first-parent changes Johannes Sixt
2012-05-22 18:23 ` Junio C Hamano
2012-05-22 19:30   ` Johannes Sixt
2012-05-22 23:38 ` Jonathan Nieder
2012-05-23 15:37 ` Martin von Zweigbergk
2012-05-23 18:53   ` Junio C Hamano
2012-05-23 20:41     ` Martin von Zweigbergk
2012-05-24 17:31       ` Junio C Hamano
2012-05-24 17:47         ` Martin von Zweigbergk
2012-05-24 20:09           ` Junio C Hamano
2012-05-24 20:32           ` Johannes Sixt
2012-05-24 21:34             ` Junio C Hamano
2012-05-25 15:58               ` Johannes Sixt
2012-05-25 16:58                 ` Junio C Hamano
2012-05-25 20:03                   ` Johannes Sixt
2012-05-23 18:59   ` Johannes Sixt

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).