{"thread":{"id":"51497","subject":"[PATCH v3] builtin/merge: support --squash --commit","startedAt":"2019-07-19T05:40:10Z","lastAt":"2019-07-29T14:06:10Z","messageCount":2,"participants":["Edmundo Carmona Antoranz"],"isPatch":true,"patchVersion":3,"patchTotal":null},"messages":[{"id":"379094","messageId":"20190719053952.13516-1-eantoranz@gmail.com","threadId":"51497","inReplyTo":null,"subject":"[PATCH v3] builtin/merge: support --squash --commit","fromName":"Edmundo Carmona Antoranz","fromEmail":"eantoranz@gmail.com","sentAt":"2019-07-19T05:39:52Z","receivedAt":"2019-07-19T05:40:10Z","isPatch":true,"sender":{"key":"eantoranz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1491018?v=4"},"body":"Using --squash made git stop regardless of conflicts so that the\nuser could finish the operation with a later call to git-commit.\n\nNow --squash --commit allows for the operation to finish with the\nnew revision if there are no conflicts. If the user does not use\n--commit, then --no-commit is used as default so that it doesn't\nbreak previous git behavior.\n\nFunction squash_message() now saves the value in merge_msg so that\nthe message with the squashed revisions is readily available when\ncalling finish_automerge() to create new revision object.\n\nFunction finish() used to skip execution paths if using --squash\nbecause there would be no new revision object created. Also, it\nnow makes sure to skip reflog update if using --squash _without_\n--commit.\n\nFunction finish_automerge() allows to create a new revision object\nfor an squashed merge (sets parent to current revision only,\nmerge_msg will be set to squashed revisions by calling\nsquash_message()), sets the reflog message to specify that it was\na merge-squash operation and removes the $GIT_DIR/SQUASH_MSG file\nif needed.\n\nSigned-off-by: Edmundo Carmona Antoranz <eantoranz@gmail.com>\n---\n builtin/merge.c  | 93 +++++++++++++++++++++++++++++-------------------\n t/t7600-merge.sh | 86 ++++++++++++++++++++++++++++++++++++++++----\n 2 files changed, 136 insertions(+), 43 deletions(-)\n\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex aad5a9504c..ad9c6e900a 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -390,11 +390,13 @@ static void finish_up_to_date(const char *msg)\n static void squash_message(struct commit *commit, struct commit_list *remoteheads)\n {\n \tstruct rev_info rev;\n-\tstruct strbuf out = STRBUF_INIT;\n \tstruct commit_list *j;\n \tstruct pretty_print_context ctx = {0};\n \n-\tprintf(_(\"Squash commit -- not updating HEAD\\n\"));\n+\tstrbuf_release(&merge_msg);\n+\n+\tif (!option_commit)\n+\t\tprintf(_(\"Squash commit -- not updating HEAD\\n\"));\n \n \trepo_init_revisions(the_repository, &rev, NULL);\n \trev.ignore_merges = 1;\n@@ -414,15 +416,14 @@ static void squash_message(struct commit *commit, struct commit_list *remotehead\n \tctx.date_mode = rev.date_mode;\n \tctx.fmt = rev.commit_format;\n \n-\tstrbuf_addstr(&out, \"Squashed commit of the following:\\n\");\n+\tstrbuf_addstr(&merge_msg, \"Squashed commit of the following:\\n\");\n \twhile ((commit = get_revision(&rev)) != NULL) {\n-\t\tstrbuf_addch(&out, '\\n');\n-\t\tstrbuf_addf(&out, \"commit %s\\n\",\n+\t\tstrbuf_addch(&merge_msg, '\\n');\n+\t\tstrbuf_addf(&merge_msg, \"commit %s\\n\",\n \t\t\toid_to_hex(&commit->object.oid));\n-\t\tpretty_print_commit(&ctx, commit, &out);\n+\t\tpretty_print_commit(&ctx, commit, &merge_msg);\n \t}\n-\twrite_file_buf(git_path_squash_msg(the_repository), out.buf, out.len);\n-\tstrbuf_release(&out);\n+\twrite_file_buf(git_path_squash_msg(the_repository), merge_msg.buf, merge_msg.len);\n }\n \n static void finish(struct commit *head_commit,\n@@ -440,22 +441,22 @@ static void finish(struct commit *head_commit,\n \t\tstrbuf_addf(&reflog_message, \"%s: %s\",\n \t\t\tgetenv(\"GIT_REFLOG_ACTION\"), msg);\n \t}\n-\tif (squash) {\n+\tif (squash)\n \t\tsquash_message(head_commit, remoteheads);\n-\t} else {\n-\t\tif (verbosity >= 0 && !merge_msg.len)\n-\t\t\tprintf(_(\"No merge message -- not updating HEAD\\n\"));\n-\t\telse {\n-\t\t\tconst char *argv_gc_auto[] = { \"gc\", \"--auto\", NULL };\n-\t\t\tupdate_ref(reflog_message.buf, \"HEAD\", new_head, head,\n-\t\t\t\t   0, UPDATE_REFS_DIE_ON_ERR);\n-\t\t\t/*\n-\t\t\t * We ignore errors in 'gc --auto', since the\n-\t\t\t * user should see them.\n-\t\t\t */\n-\t\t\tclose_object_store(the_repository->objects);\n-\t\t\trun_command_v_opt(argv_gc_auto, RUN_GIT_CMD);\n-\t\t}\n+\tif (verbosity >= 0 && !merge_msg.len)\n+\t\tprintf(_(\"No merge message -- not updating HEAD\\n\"));\n+\telse if (squash && !option_commit)\n+\t\t; /* avoid calling update_ref */\n+\telse {\n+\t\tconst char *argv_gc_auto[] = { \"gc\", \"--auto\", NULL };\n+\t\tupdate_ref(reflog_message.buf, \"HEAD\", new_head, head,\n+\t\t\t   0, UPDATE_REFS_DIE_ON_ERR);\n+\t\t/*\n+\t\t * We ignore errors in 'gc --auto', since the\n+\t\t * user should see them.\n+\t\t */\n+\t\tclose_object_store(the_repository->objects);\n+\t\trun_command_v_opt(argv_gc_auto, RUN_GIT_CMD);\n \t}\n \tif (new_head && show_diffstat) {\n \t\tstruct diff_options opts;\n@@ -893,17 +894,26 @@ static int finish_automerge(struct commit *head,\n \tstruct object_id result_commit;\n \n \tfree_commit_list(common);\n-\tparents = remoteheads;\n-\tif (!head_subsumed || fast_forward == FF_NO)\n-\t\tcommit_list_insert(head, &parents);\n-\tprepare_to_commit(remoteheads);\n+\tif (squash) {\n+\t\tsquash_message(head, remoteheads);\n+\t\tparents = commit_list_insert(head, &parents);\n+\t} else {\n+\t\tparents = remoteheads;\n+\t\tif (!head_subsumed || fast_forward == FF_NO)\n+\t\t\tcommit_list_insert(head, &parents);\n+\t\tprepare_to_commit(remoteheads);\n+\t}\n \tif (commit_tree(merge_msg.buf, merge_msg.len, result_tree, parents,\n \t\t\t&result_commit, NULL, sign_commit))\n-\t\tdie(_(\"failed to write commit object\"));\n-\tstrbuf_addf(&buf, \"Merge made by the '%s' strategy.\", wt_strategy);\n+\t\tdie(squash ? _(\"failed to write commit object on squash\") :\n+\t\t\t_(\"failed to write commit object\"));\n+\tstrbuf_addf(&buf, \"Merge made by the '%s' strategy\", wt_strategy);\n+\tstrbuf_addstr(&buf, squash ? \" (squashed).\" : \".\");\n \tfinish(head, remoteheads, &result_commit, buf.buf);\n \tstrbuf_release(&buf);\n \tremove_merge_branch_state(the_repository);\n+\tif (squash && option_commit)\n+\t\tunlink(git_path_squash_msg(the_repository));\n \treturn 0;\n }\n \n@@ -1345,14 +1355,13 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \tif (squash) {\n \t\tif (fast_forward == FF_NO)\n \t\t\tdie(_(\"You cannot combine --squash with --no-ff.\"));\n-\t\tif (option_commit > 0)\n-\t\t\tdie(_(\"You cannot combine --squash with --commit.\"));\n+\n \t\t/*\n-\t\t * squash can now silently disable option_commit - this is not\n-\t\t * a problem as it is only overriding the default, not a user\n-\t\t * supplied option.\n+\t\t * In order to not break current behavior for --squash, if the user\n+\t\t * does not specify --commit, we assume it's --no-commit\n \t\t */\n-\t\toption_commit = 0;\n+\t\tif (option_commit < 0)\n+\t\t\toption_commit = 0;\n \t}\n \n \tif (option_commit < 0)\n@@ -1510,6 +1519,13 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \t\t\tgoto done;\n \t\t}\n \n+\t\tif (squash && option_commit) {\n+\t\t\tret = finish_automerge(head_commit, 1, common,\n+\t\t\t\t\t       remoteheads, get_commit_tree_oid(commit),\n+\t\t\t\t\t       \"Fast-forward\");\n+\t\t\tgoto done;\n+\t\t}\n+\n \t\tfinish(head_commit, remoteheads, &commit->object.oid, msg.buf);\n \t\tremove_merge_branch_state(the_repository);\n \t\tgoto done;\n@@ -1682,8 +1698,11 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \t\twrite_merge_state(remoteheads);\n \n \tif (merge_was_ok)\n-\t\tfprintf(stderr, _(\"Automatic merge went well; \"\n-\t\t\t\"stopped before committing as requested\\n\"));\n+\t\tif (!option_commit)\n+\t\t\tfprintf(stderr, _(\"Automatic merge went well; \"\n+\t\t\t\t\"stopped before committing as requested\\n\"));\n+\t\telse\n+\t\t\t;\n \telse\n \t\tret = suggest_conflicts();\n \ndiff --git a/t/t7600-merge.sh b/t/t7600-merge.sh\nindex 132608879a..c3d824247f 100755\n--- a/t/t7600-merge.sh\n+++ b/t/t7600-merge.sh\n@@ -107,6 +107,10 @@ verify_no_mergehead () {\n \t! test -e .git/MERGE_HEAD\n }\n \n+verify_no_squash_msg () {\n+\t! test -e .git/SQUASH_MSG\n+}\n+\n test_expect_success 'setup' '\n \tgit add file &&\n \ttest_tick &&\n@@ -246,6 +250,25 @@ test_expect_success 'merge --squash c3 with c7' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'merge --squash --commit c3 with c7' '\n+\tgit reset --hard c3 &&\n+\ttest_must_fail git merge --squash --commit c7 &&\n+\tcat result.9z >file &&\n+\tgit commit --no-edit -a &&\n+\n+\tcat >expect <<-EOF &&\n+\tSquashed commit of the following:\n+\n+\t$(git show -s c7)\n+\n+\t# Conflicts:\n+\t#\tfile\n+\tEOF\n+\tgit cat-file commit HEAD >raw &&\n+\tsed -e '1,/^$/d' raw >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'merge c3 with c7 with commit.cleanup = scissors' '\n \tgit config commit.cleanup scissors &&\n \tgit reset --hard c3 &&\n@@ -294,6 +317,32 @@ test_expect_success 'merge c3 with c7 with --squash commit.cleanup = scissors' '\n \n test_debug 'git log --graph --decorate --oneline --all'\n \n+test_expect_success 'merge c3 with c7 with --squash --commit commit.cleanup = scissors' '\n+\tgit config commit.cleanup scissors &&\n+\tgit reset --hard c3 &&\n+\ttest_must_fail git merge --squash --commit c7 &&\n+\tcat result.9z >file &&\n+\tgit commit --no-edit -a &&\n+\n+\tcat >expect <<-EOF &&\n+\tSquashed commit of the following:\n+\n+\t$(git show -s c7)\n+\n+\t# ------------------------ >8 ------------------------\n+\t# Do not modify or remove the line above.\n+\t# Everything below it will be ignored.\n+\t#\n+\t# Conflicts:\n+\t#\tfile\n+\tEOF\n+\tgit cat-file commit HEAD >raw &&\n+\tsed -e '1,/^$/d' raw >actual &&\n+\ttest_i18ncmp 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 \tgit reset --hard c1 &&\n \ttest_tick &&\n@@ -367,6 +416,26 @@ test_expect_success 'merge c0 with c1 (squash)' '\n \n test_debug 'git log --graph --decorate --oneline --all'\n \n+test_expect_success 'merge c0 with c1 (squash --commit)' '\n+\tgit reset --hard c0 &&\n+\tgit merge --squash --commit c1 &&\n+\tverify_merge file result.1 &&\n+\tverify_parents $c0 &&\n+\tverify_no_mergehead &&\n+\tverify_no_squash_msg &&\n+\n+\tcat >expect <<-EOF &&\n+\tSquashed commit of the following:\n+\n+\t$(git show -s c1)\n+\tEOF\n+\tgit cat-file commit HEAD >raw &&\n+\tsed -e '1,/^$/d' raw >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_debug 'git log --graph --decorate --oneline --all'\n+\n test_expect_success 'merge c0 with c1 (squash, ff-only)' '\n \tgit reset --hard c0 &&\n \tgit merge --squash --ff-only c1 &&\n@@ -389,6 +458,17 @@ test_expect_success 'merge c1 with c2 (squash)' '\n \n test_debug 'git log --graph --decorate --oneline --all'\n \n+test_expect_success 'merge c1 with c2 (squash --commit)' '\n+\tgit reset --hard c1 &&\n+\tgit merge --squash --commit c2 &&\n+\tverify_merge file result.1-5 &&\n+\tverify_parents $c1 &&\n+\tverify_no_mergehead &&\n+\tverify_no_squash_msg\n+'\n+\n+test_debug 'git log --graph --decorate --oneline --all'\n+\n test_expect_success 'unsuccessful merge of c1 with c2 (squash, ff-only)' '\n \tgit reset --hard c1 &&\n \ttest_must_fail git merge --squash --ff-only c2\n@@ -570,12 +650,6 @@ test_expect_success 'combining --squash and --no-ff is refused' '\n \ttest_must_fail git merge --no-ff --squash c1\n '\n \n-test_expect_success 'combining --squash and --commit is refused' '\n-\tgit reset --hard c0 &&\n-\ttest_must_fail git merge --squash --commit c1 &&\n-\ttest_must_fail git merge --commit --squash c1\n-'\n-\n test_expect_success 'option --ff-only overwrites --no-ff' '\n \tgit merge --no-ff --ff-only c1 &&\n \ttest_must_fail git merge --no-ff --ff-only c2\n-- \n2.20.1\n\n"},{"id":"379498","messageId":"CAOc6etZaNG9gU89S491emSr7PHj9a+p+_0gNYT+BWcydadM+NA@mail.gmail.com","threadId":"51497","inReplyTo":"20190719053952.13516-1-eantoranz@gmail.com","subject":"Re: [PATCH v3] builtin/merge: support --squash --commit","fromName":"Edmundo Carmona Antoranz","fromEmail":"eantoranz@gmail.com","sentAt":"2019-07-29T14:05:57Z","receivedAt":"2019-07-29T14:06:10Z","isPatch":true,"sender":{"key":"eantoranz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1491018?v=4"},"body":"On Thu, Jul 18, 2019 at 11:40 PM Edmundo Carmona Antoranz\n<eantoranz@gmail.com> wrote:\n>\n> Using --squash made git stop regardless of conflicts so that the\n> user could finish the operation with a later call to git-commit.\n>\n> Now --squash --commit allows for the operation to finish with the\n> new revision if there are no conflicts. If the user does not use\n> --commit, then --no-commit is used as default so that it doesn't\n> break previous git behavior.\n>\n\nWhat should I do to get this patch to move forward? Either get\ncomments (as the previous versions did... thanks, Junio!) or be\naccepted? Given that I didn't get a feedback I thought that it had\nmade it (always the optimistic) but I see that it's not in Junio's\n'what's cooking' mail from friday so I think it's fair to assume that\nthis version of the patch is not gonna fly.\n\nThanks in advance!\n"}]}