From cf10f9fdd5e673a163847f0e931ce9731507e03b Mon Sep 17 00:00:00 2001 From: Miklos Vajna Date: Fri, 3 Oct 2008 14:04:47 +0200 Subject: [PATCH] builtin-commit: use reduce_heads() only when appropriate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: SZEDER Gábor Signed-off-by: Shawn O. Pearce --- branch.c | 1 + builtin-commit.c | 16 ++++++++++++++-- builtin-merge.c | 10 ++++++++++ t/t7600-merge.sh | 16 ++++++++++++++++ 4 files changed, 41 insertions(+), 2 deletions(-) diff --git a/branch.c b/branch.c index b1e59f2196..205b89dc69 100644 --- 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 b920257524..93f360f768 100644 --- 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 5c65a58699..4c9ed5da09 100644 --- 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 9516f541e9..209d7cd4b8 100755 --- 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