threads / patch / 11116

patchgit-commit: Allow to amend a merge commit that does not change the tree

Subject: [PATCH] git-commit: Allow to amend a merge commit that does not change the tree

## tl;dr

3 messages between Dec 3, 2007 and Dec 3, 2007. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Johannes Sixt· Dec 3, 2007, 07:24 UTC · lore

Normally, it should not be allowed to generate an empty commit. A merge commit generated with git 'merge -s ours' does not change the tree (along the first parent), but merges are not "empty" even if they do not change the tree. Hence, commit 8588452ceb7 allowed to amend a merge commit that does not change the tree, but 4fb5fd5d301 disallowed it again in an attempt to avoid that an existing commit is amended such that it becomes empty. With this change, a commit can be edited (create a new one or amend an existing one) either if there are changes or if there are at least two parents.

Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
	I need this patch because I sometimes do 'git merge -s ours' and
	then want to change the commit message.

	I haven't gotten around to write a test case for this scenario,
	so I'm sending out the fix alone, in order to draw attention
	to the issue and have builtin-commit fixed by its authors, if
	necessary ;)

	Thanks,
	-- Hannes

 git-commit.sh |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

Show changes to git-commit.sh +6 −3

diff --git a/git-commit.sh b/git-commit.sh
index 4853397..1a07278 100755
--- a/git-commit.sh
+++ b/git-commit.sh
@@ -515,13 +515,16 @@ else
 	# we need to check if there is anything to commit
 	run_status >/dev/null
 fi
-if [ "$?" != "0" -a ! -f "$GIT_DIR/MERGE_HEAD" ]
-then
+case "$?,$PARENTS" in
+0,* | *,-p*-p*)
+	:	# ok, go ahead
+	;;
+*)
 	rm -f "$GIT_DIR/COMMIT_EDITMSG" "$GIT_DIR/SQUASH_MSG"
 	use_status_color=t
 	run_status
 	exit 1
-fi
+esac
 
 case "$no_edit" in
 '')

-- 
1.5.3.6.969.g3cdf46

Junio C Hamano· Dec 3, 2007, 07:51 UTC · re: Johannes Sixt · lore

Re: [PATCH] git-commit: Allow to amend a merge commit that does not change the tree

Johannes Sixt <johannes.sixt@telecom.at> writes:

> 	I haven't gotten around to write a test case for this scenario,
> 	so I'm sending out the fix alone, in order to draw attention
> 	to the issue and have builtin-commit fixed by its authors, if
> 	necessary ;)

Untested but something like this ought to do.

 builtin-commit.c |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

Show changes to builtin-commit.c +10 −1

diff --git a/builtin-commit.c b/builtin-commit.c
index f37a90f..6c2dc39 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -676,6 +676,14 @@ int git_commit_config(const char *k, const char *v)
 	return git_status_config(k, v);
 }
 
+static int is_a_merge(const unsigned char *sha1)
+{
+	struct commit *commit = lookup_commit(sha1);
+	if (!commit || parse_commit(commit))
+		die("could not parse HEAD commit");
+	return !!(commit->parents && commit->parents->next);
+}
+
 static const char commit_utf8_warn[] =
 "Warning: commit message does not conform to UTF-8.\n"
 "You may want to amend it after fixing the message, or set the config\n"
@@ -701,7 +709,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		return 1;
 	}
 
-	if (!prepare_log_message(index_file, prefix) && !in_merge) {
+	if (!prepare_log_message(index_file, prefix) && !in_merge &&
+	    !(amend && is_a_merge(head_sha1))) {
 		run_status(stdout, index_file, prefix);
 		rollback_index_files();
 		unlink(commit_editmsg);

Junio C Hamano· Dec 3, 2007, 08:26 UTC · re: Junio C Hamano · lore

Re: [PATCH] git-commit: Allow to amend a merge commit that does not change the tree

Will roll in this as a new test.

--

 t/t7501-commit.sh |   32 ++++++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)

Show changes to t/t7501-commit.sh +32 −0

diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
index 31a6f63..2e7bcb0 100755
--- a/t/t7501-commit.sh
+++ b/t/t7501-commit.sh
@@ -244,4 +244,36 @@ test_expect_success 'multiple -m' '
 
 '
 
+test_expect_success 'same tree (single parent)' '
+
+	if git commit -m empty
+	then
+		echo oops -- should have complained
+		false
+	else
+		: happy
+	fi
+
+'
+
+test_expect_success 'same tree (merge and amend merge)' '
+
+	git checkout -b side HEAD^ &&
+	echo zero >zero &&
+	git add zero &&
+	git commit -m "add zero" &&
+	git checkout master &&
+
+	git merge -s ours side -m "empty ok" &&
+	git diff HEAD^ HEAD >actual &&
+	: >expected &&
+	diff -u expected actual &&
+
+	git commit --amend -m "empty really ok" &&
+	git diff HEAD^ HEAD >actual &&
+	: >expected &&
+	diff -u expected actual
+
+'
+
 test_done

-- 
1.5.3.7-2077-ga07a