{"thread":{"id":"11116","subject":"[PATCH] git-commit: Allow to amend a merge commit that does not change the tree","startedAt":"2007-12-03T07:24:50Z","lastAt":"2007-12-03T08:26:21Z","messageCount":3,"participants":["Johannes Sixt","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"61742","messageId":"1196666690-22187-1-git-send-email-johannes.sixt@telecom.at","threadId":"11116","inReplyTo":null,"subject":"[PATCH] git-commit: Allow to amend a merge commit that does not change the tree","fromName":"Johannes Sixt","fromEmail":"johannes.sixt@telecom.at","sentAt":"2007-12-03T07:24:50Z","receivedAt":"2007-12-03T07:24:50Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Normally, it should not be allowed to generate an empty commit. A merge\ncommit generated with git 'merge -s ours' does not change the tree (along\nthe first parent), but merges are not \"empty\" even if they do not change\nthe tree. Hence, commit 8588452ceb7 allowed to amend a merge commit that\ndoes not change the tree, but 4fb5fd5d301 disallowed it again in an\nattempt to avoid that an existing commit is amended such that it becomes\nempty. With this change, a commit can be edited (create a new one or amend\nan existing one) either if there are changes or if there are at least two\nparents.\n\nSigned-off-by: Johannes Sixt <johannes.sixt@telecom.at>\n---\n\tI need this patch because I sometimes do 'git merge -s ours' and\n\tthen want to change the commit message.\n\n\tI haven't gotten around to write a test case for this scenario,\n\tso I'm sending out the fix alone, in order to draw attention\n\tto the issue and have builtin-commit fixed by its authors, if\n\tnecessary ;)\n\n\tThanks,\n\t-- Hannes\n\n git-commit.sh |    9 ++++++---\n 1 files changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/git-commit.sh b/git-commit.sh\nindex 4853397..1a07278 100755\n--- a/git-commit.sh\n+++ b/git-commit.sh\n@@ -515,13 +515,16 @@ else\n \t# we need to check if there is anything to commit\n \trun_status >/dev/null\n fi\n-if [ \"$?\" != \"0\" -a ! -f \"$GIT_DIR/MERGE_HEAD\" ]\n-then\n+case \"$?,$PARENTS\" in\n+0,* | *,-p*-p*)\n+\t:\t# ok, go ahead\n+\t;;\n+*)\n \trm -f \"$GIT_DIR/COMMIT_EDITMSG\" \"$GIT_DIR/SQUASH_MSG\"\n \tuse_status_color=t\n \trun_status\n \texit 1\n-fi\n+esac\n \n case \"$no_edit\" in\n '')\n-- \n1.5.3.6.969.g3cdf46\n"},{"id":"61747","messageId":"7vtzn0md0h.fsf@gitster.siamese.dyndns.org","threadId":"11116","inReplyTo":"1196666690-22187-1-git-send-email-johannes.sixt@telecom.at","subject":"Re: [PATCH] git-commit: Allow to amend a merge commit that does not change the tree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-03T07:51:58Z","receivedAt":"2007-12-03T07:51:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt <johannes.sixt@telecom.at> writes:\n\n> \tI haven't gotten around to write a test case for this scenario,\n> \tso I'm sending out the fix alone, in order to draw attention\n> \tto the issue and have builtin-commit fixed by its authors, if\n> \tnecessary ;)\n\nUntested but something like this ought to do.\n\n builtin-commit.c |   11 ++++++++++-\n 1 files changed, 10 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex f37a90f..6c2dc39 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -676,6 +676,14 @@ int git_commit_config(const char *k, const char *v)\n \treturn git_status_config(k, v);\n }\n \n+static int is_a_merge(const unsigned char *sha1)\n+{\n+\tstruct commit *commit = lookup_commit(sha1);\n+\tif (!commit || parse_commit(commit))\n+\t\tdie(\"could not parse HEAD commit\");\n+\treturn !!(commit->parents && commit->parents->next);\n+}\n+\n static const char commit_utf8_warn[] =\n \"Warning: commit message does not conform to UTF-8.\\n\"\n \"You may want to amend it after fixing the message, or set the config\\n\"\n@@ -701,7 +709,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\treturn 1;\n \t}\n \n-\tif (!prepare_log_message(index_file, prefix) && !in_merge) {\n+\tif (!prepare_log_message(index_file, prefix) && !in_merge &&\n+\t    !(amend && is_a_merge(head_sha1))) {\n \t\trun_status(stdout, index_file, prefix);\n \t\trollback_index_files();\n \t\tunlink(commit_editmsg);\n"},{"id":"61749","messageId":"7vir3gmbf6.fsf@gitster.siamese.dyndns.org","threadId":"11116","inReplyTo":"7vtzn0md0h.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] git-commit: Allow to amend a merge commit that does not change the tree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-03T08:26:21Z","receivedAt":"2007-12-03T08:26:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Will roll in this as a new test.\n\n--\n\n t/t7501-commit.sh |   32 ++++++++++++++++++++++++++++++++\n 1 files changed, 32 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t7501-commit.sh b/t/t7501-commit.sh\nindex 31a6f63..2e7bcb0 100755\n--- a/t/t7501-commit.sh\n+++ b/t/t7501-commit.sh\n@@ -244,4 +244,36 @@ test_expect_success 'multiple -m' '\n \n '\n \n+test_expect_success 'same tree (single parent)' '\n+\n+\tif git commit -m empty\n+\tthen\n+\t\techo oops -- should have complained\n+\t\tfalse\n+\telse\n+\t\t: happy\n+\tfi\n+\n+'\n+\n+test_expect_success 'same tree (merge and amend merge)' '\n+\n+\tgit checkout -b side HEAD^ &&\n+\techo zero >zero &&\n+\tgit add zero &&\n+\tgit commit -m \"add zero\" &&\n+\tgit checkout master &&\n+\n+\tgit merge -s ours side -m \"empty ok\" &&\n+\tgit diff HEAD^ HEAD >actual &&\n+\t: >expected &&\n+\tdiff -u expected actual &&\n+\n+\tgit commit --amend -m \"empty really ok\" &&\n+\tgit diff HEAD^ HEAD >actual &&\n+\t: >expected &&\n+\tdiff -u expected actual\n+\n+'\n+\n test_done\n-- \n1.5.3.7-2077-ga07a\n"}]}