Skip to content

Commit

Permalink
builtin-commit: use reduce_heads() only when appropriate
Browse files Browse the repository at this point in the history
Since commit 6bb6b03 (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>
  • Loading branch information
Miklos Vajna authored and spearce committed Oct 3, 2008
1 parent 52e8370 commit cf10f9f
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 2 deletions.
1 change: 1 addition & 0 deletions branch.c
Expand Up @@ -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"));
}
16 changes: 14 additions & 2 deletions builtin-commit.c
Expand Up @@ -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);

Expand All @@ -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)";
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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())
Expand Down
10 changes: 10 additions & 0 deletions builtin-merge.c
Expand Up @@ -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)
Expand Down Expand Up @@ -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) {
Expand Down
16 changes: 16 additions & 0 deletions t/t7600-merge.sh
Expand Up @@ -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

0 comments on commit cf10f9f

Please sign in to comment.