{"thread":{"id":"41621","subject":"Commit message not helpful after merge squash with conflicts","startedAt":"2016-03-05T10:38:22Z","lastAt":"2016-03-21T22:34:13Z","messageCount":13,"participants":["Sven Strickroth","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"280244","messageId":"56DAB71E.6000509@cs-ware.de","threadId":"41621","inReplyTo":null,"subject":"Commit message not helpful after merge squash with conflicts","fromName":"Sven Strickroth","fromEmail":"sven@cs-ware.de","sentAt":"2016-03-05T10:38:22Z","receivedAt":"2016-03-05T10:38:22Z","isPatch":false,"sender":{"key":"sven@cs-ware.de","avatar":null},"body":"Hi,\n\nafter a \"git merge --squash\" with a conflict the commit message is not\nhelpful as it only includes the conflicted files information, however, I\nexpect to see the content of SQUASH_MSG which contains the summary of\nthe merged commits. SQUASH_MSG seems to be just ignored.\n\nI think git should either read SQUASH_MSG and append COMMIT_MSG or add\nthe conflict information into SQUASH_MSG and create no COMMIT_MSG.\n\nReferences:\n *\nhttps://stackoverflow.com/questions/3605385/merge-conflicts-ruin-my-commit-message-while-squashing-commits\n * https://gitlab.com/tortoisegit/tortoisegit/issues/1902\n\n-- \nBest regards,\n Sven Strickroth\n PGP key id F5A9D4C4 @ any key-server\n"},{"id":"280349","messageId":"56DE5272.2080009@cs-ware.de","threadId":"41621","inReplyTo":"56DAB71E.6000509@cs-ware.de","subject":"[PATCH] Also read SQUASH_MSG if a conflict on a merge squash occurred","fromName":"Sven Strickroth","fromEmail":"sven@cs-ware.de","sentAt":"2016-03-08T04:17:54Z","receivedAt":"2016-03-08T04:17:54Z","isPatch":true,"sender":{"key":"sven@cs-ware.de","avatar":null},"body":"After a merge --squash with a conflict the commit message did\nnot contain the information about the squashed commits, but\nonly the \"# Conflicts:\" information.\n\nSigned-off-by: Sven Strickroth <sven@cs-ware.de>\n---\n builtin/commit.c | 6 ++++++\n 1 file changed, 6 insertions(+)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex d054f84..0405d68 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -729,6 +729,12 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\tif (strbuf_read_file(&sb, git_path_merge_msg(), 0) < 0)\n \t\t\tdie_errno(_(\"could not read MERGE_MSG\"));\n \t\thook_arg1 = \"merge\";\n+\t\t/* append SQUASH_MSG here if it exists and a merge --squash was originally performed */\n+\t\tif (!stat(git_path_squash_msg(), &statbuf)) {\n+\t\t\tif (strbuf_read_file(&sb, git_path_squash_msg(), 0) < 0)\n+\t\t\t\tdie_errno(_(\"could not read SQUASH_MSG\"));\n+\t\t\thook_arg1 = \"squash\";\n+\t\t}\n \t} else if (!stat(git_path_squash_msg(), &statbuf)) {\n \t\tif (strbuf_read_file(&sb, git_path_squash_msg(), 0) < 0)\n \t\t\tdie_errno(_(\"could not read SQUASH_MSG\"));\n-- \nBest regards,\n Sven Strickroth\n PGP key id F5A9D4C4 @ any key-server\n"},{"id":"280381","messageId":"xmqq60wwlt0s.fsf@gitster.mtv.corp.google.com","threadId":"41621","inReplyTo":"56DE5272.2080009@cs-ware.de","subject":"Re: [PATCH] Also read SQUASH_MSG if a conflict on a merge squash occurred","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-08T18:32:51Z","receivedAt":"2016-03-08T18:32:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sven Strickroth <sven@cs-ware.de> writes:\n\n> Subject: Re: [PATCH] Also read SQUASH_MSG if a conflict on a merge squash occurred\n\nA reader sees this line in the output of \"git shortlog --no-merges\";\ndoes it sufficiently tell her which Git subcommand is affected by\nthis change, if this is a bugfix or a new feature, i.e. enough for\nher to decide how important the change is?\n\nWe often prefix our log message with the name of the area followed\nby a colon and describe the purpose of the change, not the means how\nthe objective is achieved, e.g.\n\n    Subject: [PATCH] commit: do not lose SQUASH_MSG contents\n\n    When concluding a conflicted \"git merge --squash\", the command\n    failed to read SQUASH_MSG that was prepared by \"git merge\", and\n    showed only the \"# Conflicts:\" list of conflicted paths.\n\n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index d054f84..0405d68 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -729,6 +729,12 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n>  \t\tif (strbuf_read_file(&sb, git_path_merge_msg(), 0) < 0)\n>  \t\t\tdie_errno(_(\"could not read MERGE_MSG\"));\n>  \t\thook_arg1 = \"merge\";\n> +\t\t/* append SQUASH_MSG here if it exists and a merge --squash was originally performed */\n\n\t/*\n         * Our multi-line comment reads more like\n         * this.  That is, the first slash-asterisk is on its\n         * own line, so is the last asterisk-slash.\n         */\n\n> +\t\tif (!stat(git_path_squash_msg(), &statbuf)) {\n> +\t\t\tif (strbuf_read_file(&sb, git_path_squash_msg(), 0) < 0)\n> +\t\t\t\tdie_errno(_(\"could not read SQUASH_MSG\"));\n> +\t\t\thook_arg1 = \"squash\";\n> +\t\t}\n>  \t} else if (!stat(git_path_squash_msg(), &statbuf)) {\n>  \t\tif (strbuf_read_file(&sb, git_path_squash_msg(), 0) < 0)\n>  \t\t\tdie_errno(_(\"could not read SQUASH_MSG\"));\n\nThis reads MERGE_MSG first and then SQUASH_MSG; is that what we\nreally want?  When you are resolving a conflicted rebase, you would\nsee the original log message and then conflicts section.  What is in\nthe SQUASH_MSG is the moral equivalent of the \"original log message\"\nbut in a less summarized form, so I suspect that the list of conflicts\nshould come to end.\n\nThe duplicated code to read the same file bothers me somewhat.\n\nI wondered if it makes the result easier to follow (and easier to\nupdate) if this part of the code is restructured like this:\n\n\tif (file_exists(git_path_merge_msg()) ||\n            file_exists(git_path_squash_msg())) {\n\t    if (file_exists(git_path_squash_msg())) {\n\t\tread SQUASH_MSG;\n\t    }\n            if (file_exists(git_path_merge_msg()))\n            \tread MERGE_MSG;\n\t    }\n            hook_arg1 = \"merge\";\n\t}\n\nbut I am not sure if that structure is better.\n\nThanks.\n"},{"id":"280382","messageId":"56DF1EBD.8050503@cs-ware.de","threadId":"41621","inReplyTo":"xmqq60wwlt0s.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] Also read SQUASH_MSG if a conflict on a merge squash occurred","fromName":"Sven Strickroth","fromEmail":"sven@cs-ware.de","sentAt":"2016-03-08T18:49:33Z","receivedAt":"2016-03-08T18:49:33Z","isPatch":true,"sender":{"key":"sven@cs-ware.de","avatar":null},"body":"Am 08.03.2016 um 19:32 schrieb Junio C Hamano:\n>> +\t\tif (!stat(git_path_squash_msg(), &statbuf)) {\n>> +\t\t\tif (strbuf_read_file(&sb, git_path_squash_msg(), 0) < 0)\n>> +\t\t\t\tdie_errno(_(\"could not read SQUASH_MSG\"));\n>> +\t\t\thook_arg1 = \"squash\";\n>> +\t\t}\n>>  \t} else if (!stat(git_path_squash_msg(), &statbuf)) {\n>>  \t\tif (strbuf_read_file(&sb, git_path_squash_msg(), 0) < 0)\n>>  \t\t\tdie_errno(_(\"could not read SQUASH_MSG\"));\n> \n> This reads MERGE_MSG first and then SQUASH_MSG; is that what we\n> really want?  When you are resolving a conflicted rebase, you would\n> see the original log message and then conflicts section.  What is in\n> the SQUASH_MSG is the moral equivalent of the \"original log message\"\n> but in a less summarized form, so I suspect that the list of conflicts\n> should come to end.\n\nI put them first because the squash commit list could be really long.\nI'll put MERGE_MSG at the end...\n\n> The duplicated code to read the same file bothers me somewhat.\n> \n> I wondered if it makes the result easier to follow (and easier to\n> update) if this part of the code is restructured like this:\n> \n> \tif (file_exists(git_path_merge_msg()) ||\n>             file_exists(git_path_squash_msg())) {\n> \t    if (file_exists(git_path_squash_msg())) {\n> \t\tread SQUASH_MSG;\n> \t    }\n>             if (file_exists(git_path_merge_msg()))\n>             \tread MERGE_MSG;\n> \t    }\n>             hook_arg1 = \"merge\";\n> \t}\n\nHere hook_arg1 would be always \"merge\" and never \"squash\"... Before my\nchange it was only \"squash\" if no conflict occurred.\n\n-- \nBest regards,\n Sven Strickroth\n PGP key id F5A9D4C4 @ any key-server\n"},{"id":"280383","messageId":"xmqq1t7kls58.fsf@gitster.mtv.corp.google.com","threadId":"41621","inReplyTo":"56DF1EBD.8050503@cs-ware.de","subject":"Re: [PATCH] Also read SQUASH_MSG if a conflict on a merge squash occurred","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-08T18:51:47Z","receivedAt":"2016-03-08T18:51:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sven Strickroth <sven@cs-ware.de> writes:\n\n> Here hook_arg1 would be always \"merge\" and never \"squash\"... Before my\n> change it was only \"squash\" if no conflict occurred.\n\nOh, that wasn't an intended change.  It was merely an illustration\nof a possible restructuring of the flow to avoid having to have\nidentical read functions in two separate places.\n"},{"id":"280384","messageId":"56DF21E8.2060209@cs-ware.de","threadId":"41621","inReplyTo":"xmqq60wwlt0s.fsf@gitster.mtv.corp.google.com","subject":"[PATCH] commit: do not lose SQUASH_MSG contents","fromName":"Sven Strickroth","fromEmail":"sven@cs-ware.de","sentAt":"2016-03-08T19:03:04Z","receivedAt":"2016-03-08T19:03:04Z","isPatch":true,"sender":{"key":"sven@cs-ware.de","avatar":null},"body":"When concluding a conflicted \"git merge --squash\", the command\nfailed to read SQUASH_MSG that was prepared by \"git merge\", and\nshowed only the \"# Conflicts:\" list of conflicted paths.\n\nSigned-off-by: Sven Strickroth <email@cs-ware.de>\n---\n builtin/commit.c | 20 ++++++++++++--------\n 1 file changed, 12 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex d054f84..0e48e1d 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -725,14 +725,18 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\tformat_commit_message(commit, \"fixup! %s\\n\\n\",\n \t\t\t\t      &sb, &ctx);\n \t\thook_arg1 = \"message\";\n-\t} else if (!stat(git_path_merge_msg(), &statbuf)) {\n-\t\tif (strbuf_read_file(&sb, git_path_merge_msg(), 0) < 0)\n-\t\t\tdie_errno(_(\"could not read MERGE_MSG\"));\n-\t\thook_arg1 = \"merge\";\n-\t} else if (!stat(git_path_squash_msg(), &statbuf)) {\n-\t\tif (strbuf_read_file(&sb, git_path_squash_msg(), 0) < 0)\n-\t\t\tdie_errno(_(\"could not read SQUASH_MSG\"));\n-\t\thook_arg1 = \"squash\";\n+\t} else if (!stat(git_path_squash_msg(), &statbuf) ||\n+\t\t\t   !stat(git_path_merge_msg(), &statbuf)) {\n+\t\tif (!stat(git_path_squash_msg(), &statbuf)) {\n+\t\t\tif (strbuf_read_file(&sb, git_path_squash_msg(), 0) < 0)\n+\t\t\t\tdie_errno(_(\"could not read SQUASH_MSG\"));\n+\t\t\thook_arg1 = \"squash\";\n+\t\t} else\n+\t\t\thook_arg1 = \"merge\";\n+\t\tif (!stat(git_path_merge_msg(), &statbuf)) {\n+\t\t\tif (strbuf_read_file(&sb, git_path_merge_msg(), 0) < 0)\n+\t\t\t\tdie_errno(_(\"could not read MERGE_MSG\"));\n+\t\t}\n \t} else if (template_file) {\n \t\tif (strbuf_read_file(&sb, template_file, 0) < 0)\n \t\t\tdie_errno(_(\"could not read '%s'\"), template_file);\n-- \n2.7.0.windows.1\n"},{"id":"280475","messageId":"xmqqfuvzil3y.fsf@gitster.mtv.corp.google.com","threadId":"41621","inReplyTo":"xmqq60wwlt0s.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] Also read SQUASH_MSG if a conflict on a merge squash occurred","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-09T18:04:17Z","receivedAt":"2016-03-09T18:04:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> The duplicated code to read the same file bothers me somewhat.\n>\n> I wondered if it makes the result easier to follow (and easier to\n> update) if this part of the code is restructured like this:\n>\n> \tif (file_exists(git_path_merge_msg()) ||\n>             file_exists(git_path_squash_msg())) {\n> \t    if (file_exists(git_path_squash_msg())) {\n> \t\tread SQUASH_MSG;\n> \t    }\n>             if (file_exists(git_path_merge_msg()))\n>             \tread MERGE_MSG;\n> \t    }\n>             hook_arg1 = \"merge\";\n> \t}\n>\n> but I am not sure if that structure is better.\n\n... as this duplicates file_exists() call to the same thing, which\nis no better than duplicated calls to read *_MSG files.\n"},{"id":"280505","messageId":"xmqqziu7h01f.fsf@gitster.mtv.corp.google.com","threadId":"41621","inReplyTo":"xmqqfuvzil3y.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] Also read SQUASH_MSG if a conflict on a merge squash occurred","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-09T20:24:44Z","receivedAt":"2016-03-09T20:24:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> The duplicated code to read the same file bothers me somewhat.\n>>\n>> I wondered if it makes the result easier to follow (and easier to\n>> update) if this part of the code is restructured like this:\n>>\n>> \tif (file_exists(git_path_merge_msg()) ||\n>>             file_exists(git_path_squash_msg())) {\n>> \t    if (file_exists(git_path_squash_msg())) {\n>> \t\tread SQUASH_MSG;\n>> \t    }\n>>             if (file_exists(git_path_merge_msg()))\n>>             \tread MERGE_MSG;\n>> \t    }\n>>             hook_arg1 = \"merge\";\n>> \t}\n>>\n>> but I am not sure if that structure is better.\n>\n> ... as this duplicates file_exists() call to the same thing, which\n> is no better than duplicated calls to read *_MSG files.\n\nSo, let's take the program structure from your original, but fix the\norder of the inclusion (and the log message), perhaps like the\nattached patch.\n\nDon't we also want to have a new test so that this \"contents from\nboth files are included in the result in the expected order\" feature\nwill not get broken in the future?\n\n-- >8 --\nSubject: [PATCH] commit: do not lose SQUASH_MSG contents\n\nWhen concluding a conflicted \"git merge --squash\", the command\nfailed to read SQUASH_MSG that was prepared by \"git merge\", and\nshowed only the \"# Conflicts:\" list of conflicted paths.\n\nPlace the contents from SQUASH_MSG at the beginning, just like we\nshow the commit log skeleton first when concluding a normal merge,\nand then show the \"# Conflicts:\" list, to help the user write the\nlog message for the resulting commit.\n\nSigned-off-by: Sven Strickroth <sven@cs-ware.de>\n---\n\n builtin/commit.c | 12 +++++++++++-\n 1 file changed, 11 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex b3bd2d4..4ad3931 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -726,9 +726,19 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\t\t\t      &sb, &ctx);\n \t\thook_arg1 = \"message\";\n \t} else if (!stat(git_path_merge_msg(), &statbuf)) {\n+\t\thook_arg1 = \"merge\";\n+\n+\t\t/*\n+\t\t * In a conflicted 'merge squash', the material to help\n+\t\t * writing the log message is found in SQUASH_MSG.\n+\t\t */\n+\t\tif (!stat(git_path_squash_msg(), &statbuf)) {\n+\t\t\tif (strbuf_read_file(&sb, git_path_squash_msg(), 0) < 0)\n+\t\t\t\tdie_errno(_(\"could not read SQUASH_MSG\"));\n+\t\t\thook_arg1 = \"squash\";\n+\t\t}\n \t\tif (strbuf_read_file(&sb, git_path_merge_msg(), 0) < 0)\n \t\t\tdie_errno(_(\"could not read MERGE_MSG\"));\n-\t\thook_arg1 = \"merge\";\n \t} else if (!stat(git_path_squash_msg(), &statbuf)) {\n \t\tif (strbuf_read_file(&sb, git_path_squash_msg(), 0) < 0)\n \t\t\tdie_errno(_(\"could not read SQUASH_MSG\"));\n-- \n2.8.0-rc1-141-gbaa22e3\n"},{"id":"280689","messageId":"56E5B3F9.6070404@cs-ware.de","threadId":"41621","inReplyTo":"xmqqziu7h01f.fsf@gitster.mtv.corp.google.com","subject":"[PATCH] commit: do not lose SQUASH_MSG contents","fromName":"Sven Strickroth","fromEmail":"sven@cs-ware.de","sentAt":"2016-03-13T18:39:53Z","receivedAt":"2016-03-13T18:39:53Z","isPatch":true,"sender":{"key":"sven@cs-ware.de","avatar":null},"body":"When concluding a conflicted \"git merge --squash\", the command\nfailed to read SQUASH_MSG that was prepared by \"git merge\", and\nshowed only the \"# Conflicts:\" list of conflicted paths.\n\nPlace the contents from SQUASH_MSG at the beginning, just like we\nshow the commit log skeleton first when concluding a normal merge,\nand then show the \"# Conflicts:\" list, to help the user write the\nlog message for the resulting commit.\n\nSigned-off-by: Sven Strickroth <sven@cs-ware.de>\n---\n builtin/commit.c | 11 ++++++++++-\n 1 file changed, 10 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex d054f84..d40b788 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -726,9 +726,18 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\t\t\t      &sb, &ctx);\n \t\thook_arg1 = \"message\";\n \t} else if (!stat(git_path_merge_msg(), &statbuf)) {\n+\t\t/*\n+\t\t * prepend SQUASH_MSG here if it exists and a\n+\t\t * \"merge --squash\" was originally performed\n+\t\t*/\n+\t\tif (!stat(git_path_squash_msg(), &statbuf)) {\n+\t\t\tif (strbuf_read_file(&sb, git_path_squash_msg(), 0) < 0)\n+\t\t\t\tdie_errno(_(\"could not read SQUASH_MSG\"));\n+\t\t\thook_arg1 = \"squash\";\n+\t\t} else\n+\t\t\thook_arg1 = \"merge\";\n \t\tif (strbuf_read_file(&sb, git_path_merge_msg(), 0) < 0)\n \t\t\tdie_errno(_(\"could not read MERGE_MSG\"));\n-\t\thook_arg1 = \"merge\";\n \t} else if (!stat(git_path_squash_msg(), &statbuf)) {\n \t\tif (strbuf_read_file(&sb, git_path_squash_msg(), 0) < 0)\n \t\t\tdie_errno(_(\"could not read SQUASH_MSG\"));\n-- \nBest regards,\n Sven Strickroth\n PGP key id F5A9D4C4 @ any key-server\n"},{"id":"280726","messageId":"xmqqpouwapnd.fsf@gitster.mtv.corp.google.com","threadId":"41621","inReplyTo":"56E5B3F9.6070404@cs-ware.de","subject":"Re: [PATCH] commit: do not lose SQUASH_MSG contents","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-14T18:19:18Z","receivedAt":"2016-03-14T18:19:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sven Strickroth <sven@cs-ware.de> writes:\n\n> When concluding a conflicted \"git merge --squash\", the command\n> failed to read SQUASH_MSG that was prepared by \"git merge\", and\n> showed only the \"# Conflicts:\" list of conflicted paths.\n>\n> Place the contents from SQUASH_MSG at the beginning, just like we\n> show the commit log skeleton first when concluding a normal merge,\n> and then show the \"# Conflicts:\" list, to help the user write the\n> log message for the resulting commit.\n>\n> Signed-off-by: Sven Strickroth <sven@cs-ware.de>\n> ---\n>  builtin/commit.c | 11 ++++++++++-\n>  1 file changed, 10 insertions(+), 1 deletion(-)\n\nThe updated code looks good to me; sorry for misleading you with\nfuzzy comments earlier.\n\nWe may want to have a test to prevent this from getting broken in\nthe future updates.\n\n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index d054f84..d40b788 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -726,9 +726,18 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n>  \t\t\t\t      &sb, &ctx);\n>  \t\thook_arg1 = \"message\";\n>  \t} else if (!stat(git_path_merge_msg(), &statbuf)) {\n> +\t\t/*\n> +\t\t * prepend SQUASH_MSG here if it exists and a\n> +\t\t * \"merge --squash\" was originally performed\n> +\t\t*/\n> +\t\tif (!stat(git_path_squash_msg(), &statbuf)) {\n> +\t\t\tif (strbuf_read_file(&sb, git_path_squash_msg(), 0) < 0)\n> +\t\t\t\tdie_errno(_(\"could not read SQUASH_MSG\"));\n> +\t\t\thook_arg1 = \"squash\";\n> +\t\t} else\n> +\t\t\thook_arg1 = \"merge\";\n>  \t\tif (strbuf_read_file(&sb, git_path_merge_msg(), 0) < 0)\n>  \t\t\tdie_errno(_(\"could not read MERGE_MSG\"));\n> -\t\thook_arg1 = \"merge\";\n>  \t} else if (!stat(git_path_squash_msg(), &statbuf)) {\n>  \t\tif (strbuf_read_file(&sb, git_path_squash_msg(), 0) < 0)\n>  \t\t\tdie_errno(_(\"could not read SQUASH_MSG\"));\n"},{"id":"280734","messageId":"xmqq4mc8ak3n.fsf@gitster.mtv.corp.google.com","threadId":"41621","inReplyTo":"xmqqpouwapnd.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] commit: do not lose SQUASH_MSG contents","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-14T20:19:08Z","receivedAt":"2016-03-14T20:19:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n>> Place the contents from SQUASH_MSG at the beginning, just like we\n>> show the commit log skeleton first when concluding a normal merge,\n>> and then show the \"# Conflicts:\" list, to help the user write the\n>> log message for the resulting commit.\n>>\n>> Signed-off-by: Sven Strickroth <sven@cs-ware.de>\n>> ---\n>>  builtin/commit.c | 11 ++++++++++-\n>>  1 file changed, 10 insertions(+), 1 deletion(-)\n>\n> The updated code looks good to me; sorry for misleading you with\n> fuzzy comments earlier.\n>\n> We may want to have a test to prevent this from getting broken in\n> the future updates.\n\nPerhaps like so:\n\n t/t7600-merge.sh | 28 ++++++++++++++++++++++++++++\n 1 file changed, 28 insertions(+)\n\ndiff --git a/t/t7600-merge.sh b/t/t7600-merge.sh\nindex 75c50ee..55b9da4 100755\n--- a/t/t7600-merge.sh\n+++ b/t/t7600-merge.sh\n@@ -33,9 +33,11 @@ printf '%s\\n' 1 2 3 4 5 6 7 8 9 >file\n printf '%s\\n' '1 X' 2 3 4 5 6 7 8 9 >file.1\n printf '%s\\n' 1 2 3 4 '5 X' 6 7 8 9 >file.5\n printf '%s\\n' 1 2 3 4 5 6 7 8 '9 X' >file.9\n+printf '%s\\n' 1 2 3 4 5 6 7 8 '9 Y' >file.9y\n printf '%s\\n' '1 X' 2 3 4 5 6 7 8 9 >result.1\n printf '%s\\n' '1 X' 2 3 4 '5 X' 6 7 8 9 >result.1-5\n printf '%s\\n' '1 X' 2 3 4 '5 X' 6 7 8 '9 X' >result.1-5-9\n+printf '%s\\n' 1 2 3 4 5 6 7 8 '9 Z' >result.9z\n >empty\n \n create_merge_msgs () {\n@@ -128,6 +130,12 @@ test_expect_success 'setup' '\n \tgit tag c2 &&\n \tc2=$(git rev-parse HEAD) &&\n \tgit reset --hard \"$c0\" &&\n+\tcp file.9y file &&\n+\tgit add file &&\n+\ttest_tick &&\n+\tgit commit -m \"commit 7\" &&\n+\tgit tag c7 &&\n+\tgit reset --hard \"$c0\" &&\n \tcp file.9 file &&\n \tgit add file &&\n \ttest_tick &&\n@@ -218,6 +226,26 @@ test_expect_success 'merge c1 with c2' '\n \tverify_parents $c1 $c2\n '\n \n+test_expect_success 'merge --squash c3 with c7' '\n+\tgit reset --hard c3 &&\n+\ttest_must_fail git merge --squash c7 &&\n+\tcat result.9z >file &&\n+\tgit commit --no-edit -a &&\n+\n+\t{\n+\t\tcat <<-EOF\n+\t\tSquashed commit of the following:\n+\n+\t\t$(git show -s c7)\n+\n+\t\t# Conflicts:\n+\t\t#\tfile\n+\t\tEOF\n+\t} >expect &&\n+\tgit cat-file commit HEAD | sed -e '1,/^$/d' >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_debug 'git log --graph --decorate --oneline --all'\n \n test_expect_success 'merge c1 with c2 and c3' '\n-- \n2.8.0-rc2-154-gd146b22\n"},{"id":"281396","messageId":"56F075D4.2020002@cs-ware.de","threadId":"41621","inReplyTo":"xmqq4mc8ak3n.fsf@gitster.mtv.corp.google.com","subject":"[PATCH] commit: do not lose SQUASH_MSG contents","fromName":"Sven Strickroth","fromEmail":"sven@cs-ware.de","sentAt":"2016-03-21T22:29:40Z","receivedAt":"2016-03-21T22:29:40Z","isPatch":true,"sender":{"key":"sven@cs-ware.de","avatar":null},"body":"When concluding a conflicted \"git merge --squash\", the command\nfailed to read SQUASH_MSG that was prepared by \"git merge\", and\nshowed only the \"# Conflicts:\" list of conflicted paths.\n\nPlace the contents from SQUASH_MSG at the beginning, just like we\nshow the commit log skeleton first when concluding a normal merge,\nand then show the \"# Conflicts:\" list, to help the user write the\nlog message for the resulting commit.\n\nTest by Junio C Hamano <gitster@pobox.com>.\n\nSigned-off-by: Sven Strickroth <sven@cs-ware.de>\n---\n builtin/commit.c | 11 ++++++++++-\n t/t7600-merge.sh | 28 ++++++++++++++++++++++++++++\n 2 files changed, 38 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex d054f84..d40b788 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -726,9 +726,18 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\t\t\t      &sb, &ctx);\n \t\thook_arg1 = \"message\";\n \t} else if (!stat(git_path_merge_msg(), &statbuf)) {\n+\t\t/*\n+\t\t * prepend SQUASH_MSG here if it exists and a\n+\t\t * \"merge --squash\" was originally performed\n+\t\t*/\n+\t\tif (!stat(git_path_squash_msg(), &statbuf)) {\n+\t\t\tif (strbuf_read_file(&sb, git_path_squash_msg(), 0) < 0)\n+\t\t\t\tdie_errno(_(\"could not read SQUASH_MSG\"));\n+\t\t\thook_arg1 = \"squash\";\n+\t\t} else\n+\t\t\thook_arg1 = \"merge\";\n \t\tif (strbuf_read_file(&sb, git_path_merge_msg(), 0) < 0)\n \t\t\tdie_errno(_(\"could not read MERGE_MSG\"));\n-\t\thook_arg1 = \"merge\";\n \t} else if (!stat(git_path_squash_msg(), &statbuf)) {\n \t\tif (strbuf_read_file(&sb, git_path_squash_msg(), 0) < 0)\n \t\t\tdie_errno(_(\"could not read SQUASH_MSG\"));\ndiff --git a/t/t7600-merge.sh b/t/t7600-merge.sh\nindex 302e238..ba35e00 100755\n--- a/t/t7600-merge.sh\n+++ b/t/t7600-merge.sh\n@@ -33,9 +33,11 @@ printf '%s\\n' 1 2 3 4 5 6 7 8 9 >file\n printf '%s\\n' '1 X' 2 3 4 5 6 7 8 9 >file.1\n printf '%s\\n' 1 2 3 4 '5 X' 6 7 8 9 >file.5\n printf '%s\\n' 1 2 3 4 5 6 7 8 '9 X' >file.9\n+printf '%s\\n' 1 2 3 4 5 6 7 8 '9 Y' >file.9y\n printf '%s\\n' '1 X' 2 3 4 5 6 7 8 9 >result.1\n printf '%s\\n' '1 X' 2 3 4 '5 X' 6 7 8 9 >result.1-5\n printf '%s\\n' '1 X' 2 3 4 '5 X' 6 7 8 '9 X' >result.1-5-9\n+printf '%s\\n' 1 2 3 4 5 6 7 8 '9 Z' >result.9z\n >empty\n \n create_merge_msgs () {\n@@ -128,6 +130,12 @@ test_expect_success 'setup' '\n \tgit tag c2 &&\n \tc2=$(git rev-parse HEAD) &&\n \tgit reset --hard \"$c0\" &&\n+\tcp file.9y file &&\n+\tgit add file &&\n+\ttest_tick &&\n+\tgit commit -m \"commit 7\" &&\n+\tgit tag c7 &&\n+\tgit reset --hard \"$c0\" &&\n \tcp file.9 file &&\n \tgit add file &&\n \ttest_tick &&\n@@ -218,6 +226,26 @@ test_expect_success 'merge c1 with c2' '\n \tverify_parents $c1 $c2\n '\n \n+test_expect_success 'merge --squash c3 with c7' '\n+\tgit reset --hard c3 &&\n+\ttest_must_fail git merge --squash c7 &&\n+\tcat result.9z >file &&\n+\tgit commit --no-edit -a &&\n+\n+\t{\n+\t\tcat <<-EOF\n+\t\tSquashed commit of the following:\n+\n+\t\t$(git show -s c7)\n+\n+\t\t# Conflicts:\n+\t\t#\tfile\n+\t\tEOF\n+\t} >expect &&\n+\tgit cat-file commit HEAD | sed -e '1,/^$/d' >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_debug 'git log --graph --decorate --oneline --all'\n \n test_expect_success 'merge c1 with c2 and c3' '\n-- \n2.7.4.windows.1\n"},{"id":"281397","messageId":"xmqqpounjw9m.fsf@gitster.mtv.corp.google.com","threadId":"41621","inReplyTo":"56F075D4.2020002@cs-ware.de","subject":"Re: [PATCH] commit: do not lose SQUASH_MSG contents","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-21T22:34:13Z","receivedAt":"2016-03-21T22:34:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sven Strickroth <sven@cs-ware.de> writes:\n\n> When concluding a conflicted \"git merge --squash\", the command\n> failed to read SQUASH_MSG that was prepared by \"git merge\", and\n> showed only the \"# Conflicts:\" list of conflicted paths.\n>\n> Place the contents from SQUASH_MSG at the beginning, just like we\n> show the commit log skeleton first when concluding a normal merge,\n> and then show the \"# Conflicts:\" list, to help the user write the\n> log message for the resulting commit.\n>\n> Test by Junio C Hamano <gitster@pobox.com>.\n>\n> Signed-off-by: Sven Strickroth <sven@cs-ware.de>\n> ---\n\nYou must somehow read my mind, as I was about to send a friendly\nping to you saying \"unless you have a reroll, I'll squash the test\nin\" ;-)\n\nWill replace those two commits with this one (after fixing one nit).\n\nThanks.\n\n>  builtin/commit.c | 11 ++++++++++-\n>  t/t7600-merge.sh | 28 ++++++++++++++++++++++++++++\n>  2 files changed, 38 insertions(+), 1 deletion(-)\n>\n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index d054f84..d40b788 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -726,9 +726,18 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n>  \t\t\t\t      &sb, &ctx);\n>  \t\thook_arg1 = \"message\";\n>  \t} else if (!stat(git_path_merge_msg(), &statbuf)) {\n> +\t\t/*\n> +\t\t * prepend SQUASH_MSG here if it exists and a\n> +\t\t * \"merge --squash\" was originally performed\n> +\t\t*/\n\nHere is a nit (\"*/\" needs one more space indent to align).\n"}]}