[bug] git `next' does not do trivial merges

I had already posted this bug report yesterday but it was hidden in a 
cover letter at 
http://permalink.gmane.org/gmane.comp.version-control.git/93143 -- so 
I'll copy the relevant info here:

	The included testcases demonstrate that trivial merges are
	currently broken.  The failing test is:

	  git init
	  echo a > a
	  git add a
	  git commit -ma 
	  git checkout -b branch
	  echo b > b
	  git add b
	  git commit -mb
	  git checkout master
	  git merge --no-ff -s resolve branch

	Running this in 1.5.5 shows: 

	  Trying really trivial in-index merge...
	  Wonderful.
	  In-index merge

	while `next' gives

	  Trying really trivial in-index merge...
	  error: Untracked working tree file 'a' would be overwritten by merge.
	  Nope.
	  Trying simple merge.
	  Merge made by resolve.

Paolo

On Fri, Aug 22, 2008 at 08:36:39AM +0200, Paolo Bonzini wrote:

I had already posted this bug report yesterday but it was hidden in a  
cover letter at  
http://permalink.gmane.org/gmane.comp.version-control.git/93143 -- so  
I'll copy the relevant info here:

Sadly, this bisects to 1c7b76b (Build in merge).

-Peff

On Fri, Aug 22, 2008 at 03:31:17PM -0400, Jeff King [off-list ref] wrote:

On Fri, Aug 22, 2008 at 08:36:39AM +0200, Paolo Bonzini wrote:

quoted

I had already posted this bug report yesterday but it was hidden in a  
cover letter at  
http://permalink.gmane.org/gmane.comp.version-control.git/93143 -- so  
I'll copy the relevant info here:

Sadly, this bisects to 1c7b76b (Build in merge).

I guessed the bug is in builtin-merge.c::read_tree_trivial(), but I
don't see how it is different to builtin-read-tree.c::cmd_read_tree(),
at least the unpack_trees_options struct is the same.

I'm on it..

There were 3 issues:

1) We need read_cache() before using unpack_trees().

2) commit_tree() frees the parents, so we need to malloc them.

3) The current HEAD was missing from the parent list.

Signed-off-by: Miklos Vajna <redacted>
---

On Fri, Aug 22, 2008 at 08:36:39AM +0200, Paolo Bonzini [off-list ref] wrote:

I had already posted this bug report yesterday but it was hidden in a
cover
letter at
http://permalink.gmane.org/gmane.comp.version-control.git/93143

This should fix the issue.

 builtin-merge.c          |   11 +++++++----
 t/t7607-merge-inindex.sh |   29 +++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 4 deletions(-)
 create mode 100755 t/t7607-merge-inindex.sh

diff --git a/builtin-merge.c b/builtin-merge.c
index a201c66..dffe4b8 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c

@@ -469,6 +469,7 @@ static int read_tree_trivial(unsigned char *common, unsigned char *head,
 	struct tree_desc t[MAX_UNPACK_TREES];
 	struct unpack_trees_options opts;
 
+	read_cache();
 	memset(&opts, 0, sizeof(opts));
 	opts.head_idx = 2;
 	opts.src_index = &the_index;

@@ -651,13 +652,15 @@ static void add_strategies(const char *string, unsigned attr)
 static int merge_trivial(void)
 {
 	unsigned char result_tree[20], result_commit[20];
-	struct commit_list parent;
+	struct commit_list *parent = xmalloc(sizeof(struct commit_list *));
 
 	write_tree_trivial(result_tree);
 	printf("Wonderful.\n");
-	parent.item = remoteheads->item;
-	parent.next = NULL;
-	commit_tree(merge_msg.buf, result_tree, &parent, result_commit);
+	parent->item = lookup_commit(head);
+	parent->next = xmalloc(sizeof(struct commit_list *));
+	parent->next->item = remoteheads->item;
+	parent->next->next = NULL;
+	commit_tree(merge_msg.buf, result_tree, parent, result_commit);
 	finish(result_commit, "In-index merge");
 	drop_save();
 	return 0;

diff --git a/t/t7607-merge-inindex.sh b/t/t7607-merge-inindex.sh
new file mode 100755
index 0000000..98b778f
--- /dev/null
+++ b/t/t7607-merge-inindex.sh

@@ -0,0 +1,29 @@
+#!/bin/sh
+
+test_description='git-merge
+
+Testing in-index merge.'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	echo a > a &&
+	git add a &&
+	git commit -m a &&
+	git tag a &&
+	git checkout -b branch
+	echo b > b &&
+	git add b &&
+	git commit -m b &&
+	git tag b
+'
+
+test_expect_success 'in-index merge' '
+	git checkout master &&
+	git merge --no-ff -s resolve branch > out &&
+	grep Wonderful. out &&
+	test "$(git rev-parse a)" = "$(git rev-parse HEAD^1)" &&
+	test "$(git rev-parse b)" = "$(git rev-parse HEAD^2)"
+'
+
+test_done

-- 
1.6.0.rc3.17.gc14c8.dirty

Using unmerged_cache() without reading the cache first never will return
anything. However, if we read the cache early then we have to discard it
when we want to read it again from the disk.

Signed-off-by: Miklos Vajna <redacted>
---

The code part is pretty much from you, I wanted to add your signoff, but
then realized that you'll do it anyway. ;-)

 builtin-merge.c        |    6 +++---
 t/t7608-merge-dirty.sh |   33 +++++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 3 deletions(-)
 create mode 100755 t/t7608-merge-dirty.sh

diff --git a/builtin-merge.c b/builtin-merge.c
index dffe4b8..71bd13b 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c

@@ -565,8 +565,6 @@ static int checkout_fast_forward(unsigned char *head, unsigned char *remote)
 	struct dir_struct dir;
 	struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));
 
-	if (read_cache_unmerged())
-		die("you need to resolve your current index first");
 	refresh_cache(REFRESH_QUIET);
 
 	fd = hold_locked_index(lock_file, 1);

@@ -746,6 +744,7 @@ static int evaluate_result(void)
 	int cnt = 0;
 	struct rev_info rev;
 
+	discard_cache();
 	if (read_cache() < 0)
 		die("failed to read the cache");
 

@@ -779,7 +778,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	struct commit_list **remotes = &remoteheads;
 
 	setup_work_tree();
-	if (unmerged_cache())
+	if (read_cache_unmerged())
 		die("You are in the middle of a conflicted merge.");
 
 	/*

@@ -1076,6 +1075,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		}
 
 		/* Automerge succeeded. */
+		discard_cache();
 		write_tree_trivial(result_tree);
 		automerge_was_ok = 1;
 		break;

diff --git a/t/t7608-merge-dirty.sh b/t/t7608-merge-dirty.sh
new file mode 100755
index 0000000..fb567f6
--- /dev/null
+++ b/t/t7608-merge-dirty.sh

@@ -0,0 +1,33 @@
+#!/bin/sh
+
+test_description='git-merge
+
+Merge should fail if the index has unresolved entries.'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	echo a > a &&
+	git add a &&
+	git commit -m a &&
+	git tag a &&
+	git checkout -b branch1
+	echo b > a &&
+	git add a &&
+	git commit -m b &&
+	git tag b
+	git checkout -b branch2 HEAD~1
+	echo c > a &&
+	git add a &&
+	git commit -m c &&
+	git tag c
+'
+
+test_expect_success 'in-index merge' '
+	git checkout branch1 &&
+	test_must_fail git merge branch2 &&
+	test_must_fail git merge branch2 2> out &&
+	grep "You are in the middle of a conflicted merge" out
+'
+
+test_done

-- 
1.6.0.rc3.17.gc14c8.dirty

Miklos Vajna wrote:

There were 3 issues:

1) We need read_cache() before using unpack_trees().

2) commit_tree() frees the parents, so we need to malloc them.

3) The current HEAD was missing from the parent list.

Unfortunately it does not really work yet, since the files only present
on the second branch are not added.  I do get

Trying really trivial in-index merge...
Wonderful.
In-index merge

but then "git status" gives:

# On branch master
# Untracked files:
#   (use "git add <file>..." to include in what will be committed)
#
#       b

(in my testcase for pre-merge, this results in a failure of the
"octopus" testcase).

Paolo