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@xxxxxxxxxx>
---
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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
