[PATCH] builtin-commit: use reduce_heads() only when appropriate

["SZEDER Gábor" <szeder@ira.uka.de>]

Since commit 6bb6b034 (builtin-commit: use commit_tree(), 2008-09-10),
builtin-commit performs a reduce_heads() unconditionally.  However,
it's not always needed, and in some cases even harmful.

reduce_heads() is not needed for the initial commit or for an
"ordinary" commit, because they don't have any or have only one
parent, respectively.

reduce_heads() must be avoided when 'git commit' is run after a 'git
merge --no-ff --no-commit', otherwise it will turn the
non-fast-forward merge into fast-forward.  For the same reason,
reduce_heads() must be avoided when amending such a merge commit.

To resolve this issue, 'git merge' will write info about whether
fast-forward is allowed or not to $GIT_DIR/MERGE_MODE.  Based on this
info, 'git commit' will only perform reduce_heads() when it's
committing a merge and fast-forward is enabled.

Also add test cases to ensure that non-fast-forward merges are
committed and amended properly.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---

  I think this should be squashed onto Miklós' patch.

  First, it fixes the segmentation fault caused by using the strbuf
  before initializing it.  Second, amending a no-ff merge commit has
  the same issues with reduce_heads();  see the new test case.  And the
  commit message is also more detailed ;)

  I'm not sure about putting these two new test into t7600-merge.sh.
  Although the infrastructure to keep these tests very short (repo with
  branches already set up, verify_parents) is there available, the first
  test tests not only merge, but the cooperation of merge and commit,
  and this second one tests only commit.

  Regards,
  Gábor


 builtin-commit.c |   21 +++++++++++----------
 t/t7600-merge.sh |    7 +++++++
 2 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/builtin-commit.c b/builtin-commit.c
index e69b4af..93f360f 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -953,6 +953,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		return 1;
 	}
 
+	strbuf_init(&sb, 0);
 	/* Determine parents */
 	if (initial_commit) {
 		reflog_msg = "commit (initial)";
@@ -986,22 +987,22 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		}
 		fclose(fp);
 		strbuf_release(&m);
+		if (!stat(git_path("MERGE_MODE"), &statbuf)) {
+			if (strbuf_read_file(&sb, git_path("MERGE_MODE"), 0) < 0)
+				die("could not read MERGE_MODE: %s",
+						strerror(errno));
+			if (!strcmp(sb.buf, "no-ff"))
+				allow_fast_forward = 0;
+		}
+		if (allow_fast_forward)
+			parents = reduce_heads(parents);
 	} else {
 		reflog_msg = "commit";
 		pptr = &commit_list_insert(lookup_commit(head_sha1), pptr)->next;
 	}
-	strbuf_reset(&sb);
-	if (!stat(git_path("MERGE_MODE"), &statbuf)) {
-		if (strbuf_read_file(&sb, git_path("MERGE_MODE"), 0) < 0)
-			die("could not read MERGE_MODE: %s", strerror(errno));
-		if (!strcmp(sb.buf, "no-ff"))
-			allow_fast_forward = 0;
-	}
-	if (allow_fast_forward)
-		parents = reduce_heads(parents);
 
 	/* Finally, get the commit message */
-	strbuf_init(&sb, 0);
+	strbuf_reset(&sb);
 	if (strbuf_read_file(&sb, git_path(commit_editmsg), 0) < 0) {
 		rollback_index_files();
 		die("could not read commit message");
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 98cfc53..209d7cd 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -520,4 +520,11 @@ test_expect_success 'merge --no-ff --no-commit && commit' '
 
 test_debug 'gitk --all'
 
+test_expect_success 'amending no-ff merge commit' '
+	EDITOR=: git commit --amend &&
+	verify_parents $c0 $c1
+'
+
+test_debug 'gitk --all'
+
 test_done
-- 
1.6.0.2.430.gfc53