builtin-commit: use reduce_heads() only when appropriate

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: Miklos Vajna <vmiklos@frugalware.org>
Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>

diff --git a/branch.c b/branch.c
index b1e59f2196b933ab7169a30efc5d1d340f8f9c5c..205b89dc697eb4475355ab5b2a6528e404901c0c 100644 (file)
--- a/branch.c
+++ b/branch.c
@@ -168,5 +168,6 @@ void remove_branch_state(void)
        unlink(git_path("MERGE_HEAD"));
        unlink(git_path("MERGE_RR"));
        unlink(git_path("MERGE_MSG"));
+       unlink(git_path("MERGE_MODE"));
        unlink(git_path("SQUASH_MSG"));
 }

diff --git a/builtin-commit.c b/builtin-commit.c
index b920257524850ea04bc940411d342e9a622e7094..93f360f768b828298826a6c088df456e6d2648ba 100644 (file)
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -937,6 +937,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
        unsigned char commit_sha1[20];
        struct ref_lock *ref_lock;
        struct commit_list *parents = NULL, **pptr = &parents;
+       struct stat statbuf;
+       int allow_fast_forward = 1;
 
        git_config(git_commit_config, NULL);
 
@@ -951,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)";
@@ -984,14 +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;
        }
-       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");
@@ -1040,6 +1051,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 
        unlink(git_path("MERGE_HEAD"));
        unlink(git_path("MERGE_MSG"));
+       unlink(git_path("MERGE_MODE"));
        unlink(git_path("SQUASH_MSG"));
 
        if (commit_index_files())

diff --git a/builtin-merge.c b/builtin-merge.c
index 5c65a5869900ad1a1014fb3aad88c874a5410bf7..4c9ed5da0949f60a20b2e18ac1d892bb5c6af4a4 100644 (file)
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -180,6 +180,7 @@ static void drop_save(void)
 {
        unlink(git_path("MERGE_HEAD"));
        unlink(git_path("MERGE_MSG"));
+       unlink(git_path("MERGE_MODE"));
 }
 
 static void save_state(void)
@@ -1210,6 +1211,15 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
                        merge_msg.len)
                        die("Could not write to %s", git_path("MERGE_MSG"));
                close(fd);
+               fd = open(git_path("MERGE_MODE"), O_WRONLY | O_CREAT | O_TRUNC, 0666);
+               if (fd < 0)
+                       die("Could open %s for writing", git_path("MERGE_MODE"));
+               strbuf_reset(&buf);
+               if (!allow_fast_forward)
+                       strbuf_addf(&buf, "no-ff");
+               if (write_in_full(fd, buf.buf, buf.len) != buf.len)
+                       die("Could not write to %s", git_path("MERGE_MODE"));
+               close(fd);
        }
 
        if (merge_was_ok) {

diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 9516f541e9c47f83fed2fc8d3baa065a9bb206de..209d7cd4b8f9497cef160a1a1b11f93cb7347c8f 100755 (executable)
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -511,4 +511,20 @@ test_expect_success 'in-index merge' '
 
 test_debug 'gitk --all'
 
+test_expect_success 'merge --no-ff --no-commit && commit' '
+       git reset --hard c0 &&
+       git merge --no-ff --no-commit c1 &&
+       EDITOR=: git commit &&
+       verify_parents $c0 $c1
+'
+
+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