{"thread":{"id":"56277","subject":"[PATCH 0/4] rebase -r: some merge related fixes","startedAt":"2021-08-13T13:07:37Z","lastAt":"2021-08-24T13:34:05Z","messageCount":15,"participants":["Phillip Wood via GitGitGadget","Johannes Schindelin","SZEDER Gábor","Phillip Wood"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"432670","messageId":"pull.1015.git.1628860053.gitgitgadget@gmail.com","threadId":"56277","inReplyTo":null,"subject":"[PATCH 0/4] rebase -r: some merge related fixes","fromName":"Phillip Wood via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-08-13T13:07:29Z","receivedAt":"2021-08-13T13:07:37Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"This is a collection of merge related fixes for rebase -r\n\n * Make merge -c behave like reword.\n * When fast-forwarding a merge don't leave .git/MERGE_MSG around (reported\n   by Gábor)\n * Make merge -c work when with --strategy\n\nPhillip Wood (4):\n  rebase -r: make 'merge -c' behave like reword\n  rebase -i: Add another reword test\n  rebase -r: don't write .git/MERGE_MSG when fast-forwarding\n  rebase -r: fix merge -c with a merge strategy\n\n sequencer.c                   | 106 ++++++++++++++++++----------------\n t/lib-rebase.sh               |  56 ++++++++++++++++++\n t/t3404-rebase-interactive.sh |  13 +++++\n t/t3430-rebase-merges.sh      |  38 +++++++++---\n 4 files changed, 155 insertions(+), 58 deletions(-)\n\n\nbase-commit: 66262451ec94d30ac4b80eb3123549cf7a788afd\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1015%2Fphillipwood%2Fwip%2Fsequencer-merge-c-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1015/phillipwood/wip/sequencer-merge-c-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1015\n-- \ngitgitgadget\n"},{"id":"432671","messageId":"b514dbdf9287128b8e0fab1d1d5b5107de15764e.1628860053.git.gitgitgadget@gmail.com","threadId":"56277","inReplyTo":"pull.1015.git.1628860053.gitgitgadget@gmail.com","subject":"[PATCH 1/4] rebase -r: make 'merge -c' behave like reword","fromName":"Phillip Wood via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-08-13T13:07:30Z","receivedAt":"2021-08-13T13:07:38Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood <phillip.wood@dunelm.org.uk>\n\nIf the user runs git log while rewording a commit it is confusing if\nsometimes we're amending the commit that's being reworded and at other\ntimes we're creating a new commit depending on whether we could\nfast-forward or not[1]. For this reason the reword command ensures\nthat there are no uncommitted changes when rewording. The reword\ncommand also allows the user to edit the todo list while the rebase is\npaused. As 'merge -c' also rewords commits make it behave like reword\nand add a test.\n\n[1] https://lore.kernel.org/git/xmqqlfvu4be3.fsf@gitster-ct.c.googlers.com/T/#m133009cb91cf0917bcf667300f061178be56680a\n\nSigned-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n---\n sequencer.c              | 24 +++++++++++--------\n t/lib-rebase.sh          | 50 ++++++++++++++++++++++++++++++++++++++++\n t/t3430-rebase-merges.sh | 20 ++++++++--------\n 3 files changed, 75 insertions(+), 19 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 7f07cd00f3f..cc8a361cceb 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -3739,10 +3739,9 @@ static struct commit *lookup_label(const char *label, int len,\n static int do_merge(struct repository *r,\n \t\t    struct commit *commit,\n \t\t    const char *arg, int arg_len,\n-\t\t    int flags, struct replay_opts *opts)\n+\t\t    int flags, int *check_todo, struct replay_opts *opts)\n {\n-\tint run_commit_flags = (flags & TODO_EDIT_MERGE_MSG) ?\n-\t\tEDIT_MSG | VERIFY_MSG : 0;\n+\tint run_commit_flags = 0;\n \tstruct strbuf ref_name = STRBUF_INIT;\n \tstruct commit *head_commit, *merge_commit, *i;\n \tstruct commit_list *bases, *j, *reversed = NULL;\n@@ -3898,10 +3897,9 @@ static int do_merge(struct repository *r,\n \t\trollback_lock_file(&lock);\n \t\tret = fast_forward_to(r, &commit->object.oid,\n \t\t\t\t      &head_commit->object.oid, 0, opts);\n-\t\tif (flags & TODO_EDIT_MERGE_MSG) {\n-\t\t\trun_commit_flags |= AMEND_MSG;\n+\t\tif (flags & TODO_EDIT_MERGE_MSG)\n \t\t\tgoto fast_forward_edit;\n-\t\t}\n+\n \t\tgoto leave_merge;\n \t}\n \n@@ -4035,10 +4033,17 @@ static int do_merge(struct repository *r,\n \t\t * value (a negative one would indicate that the `merge`\n \t\t * command needs to be rescheduled).\n \t\t */\n-\tfast_forward_edit:\n \t\tret = !!run_git_commit(git_path_merge_msg(r), opts,\n \t\t\t\t       run_commit_flags);\n \n+\tif (!ret && flags & TODO_EDIT_MERGE_MSG) {\n+\tfast_forward_edit:\n+\t\t*check_todo = 1;\n+\t\trun_commit_flags |= AMEND_MSG | EDIT_MSG | VERIFY_MSG;\n+\t\tret = !!run_git_commit(NULL, opts, run_commit_flags);\n+\t}\n+\n+\n leave_merge:\n \tstrbuf_release(&ref_name);\n \trollback_lock_file(&lock);\n@@ -4405,9 +4410,8 @@ static int pick_commits(struct repository *r,\n \t\t\tif ((res = do_reset(r, arg, item->arg_len, opts)))\n \t\t\t\treschedule = 1;\n \t\t} else if (item->command == TODO_MERGE) {\n-\t\t\tif ((res = do_merge(r, item->commit,\n-\t\t\t\t\t    arg, item->arg_len,\n-\t\t\t\t\t    item->flags, opts)) < 0)\n+\t\t\tif ((res = do_merge(r, item->commit, arg, item->arg_len,\n+\t\t\t\t\t    item->flags, &check_todo, opts)) < 0)\n \t\t\t\treschedule = 1;\n \t\t\telse if (item->commit)\n \t\t\t\trecord_in_rewritten(&item->commit->object.oid,\ndiff --git a/t/lib-rebase.sh b/t/lib-rebase.sh\nindex dc75b834518..99d9e7efd2d 100644\n--- a/t/lib-rebase.sh\n+++ b/t/lib-rebase.sh\n@@ -151,3 +151,53 @@ test_editor_unchanged () {\n \tEOF\n \ttest_cmp expect actual\n }\n+\n+# Set up an editor for testing reword commands\n+# Checks that there are no uncommitted changes when rewording and that the\n+# todo-list is reread after each\n+set_reword_editor () {\n+\t>reword-actual &&\n+\t>reword-oid &&\n+\n+\t# Check rewording keeps the original authorship\n+\tGIT_AUTHOR_NAME=\"Reword Author\"\n+\tGIT_AUTHOR_EMAIL=\"reword.author@example.com\"\n+\tGIT_AUTHOR_DATE=@123456\n+\n+\twrite_script reword-sequence-editor.sh <<-\\EOF &&\n+\ttodo=\"$(cat \"$1\")\" &&\n+\techo \"exec git log -1 --pretty=format:'%an <%ae> %at%n%B%n' \\\n+\t\t>>reword-actual\" >\"$1\" &&\n+\tprintf \"%s\\n\" \"$todo\" >>\"$1\"\n+\tEOF\n+\n+\twrite_script reword-editor.sh <<-EOF &&\n+\t# Save the oid of the first reworded commit so we can check rebase\n+\t# fast-forwards to it\n+\tif ! test -s reword-oid\n+\tthen\n+\t\tgit rev-parse HEAD >reword-oid\n+\tfi &&\n+\t# There should be no uncommited changes\n+\tgit diff --exit-code HEAD &&\n+\t# The todo-list should be re-read after a reword\n+\tGIT_SEQUENCE_EDITOR=\"\\\"$PWD/reword-sequence-editor.sh\\\"\" \\\n+\t\tgit rebase --edit-todo &&\n+\techo edited >>\"\\$1\"\n+\tEOF\n+\n+\ttest_set_editor \"$PWD/reword-editor.sh\"\n+}\n+\n+# Check the results of a rebase after calling set_reword_editor\n+# Pass the commits that were reworded in the order that they were picked\n+# Expects the first pick to be a fast-forward\n+check_reworded_commits () {\n+\ttest_cmp_rev \"$(cat reword-oid)\" \"$1^{commit}\" &&\n+\tgit log --format=\"%an <%ae> %at%n%B%nedited%n\" --no-walk=unsorted \"$@\" \\\n+\t\t>reword-expected &&\n+\ttest_cmp reword-expected reword-actual &&\n+\tgit log --format=\"%an <%ae> %at%n%B\" -n $# --first-parent --reverse \\\n+\t\t>reword-log &&\n+\ttest_cmp reword-expected reword-log\n+}\ndiff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh\nindex 6748070df52..183c3a23838 100755\n--- a/t/t3430-rebase-merges.sh\n+++ b/t/t3430-rebase-merges.sh\n@@ -172,17 +172,19 @@ test_expect_success 'failed `merge <branch>` does not crash' '\n \tgrep \"^Merge branch ${SQ}G${SQ}$\" .git/rebase-merge/message\n '\n \n-test_expect_success 'fast-forward merge -c still rewords' '\n-\tgit checkout -b fast-forward-merge-c H &&\n+test_expect_success 'merge -c commits before rewording and reloads todo-list' '\n+\tcat >script-from-scratch <<-\\EOF &&\n+\tmerge -c E B\n+\tmerge -c H G\n+\tEOF\n+\n+\tgit checkout -b merge-c H &&\n \t(\n-\t\tset_fake_editor &&\n-\t\tFAKE_COMMIT_MESSAGE=edited \\\n-\t\t\tGIT_SEQUENCE_EDITOR=\"echo merge -c H G >\" \\\n-\t\t\tgit rebase -ir @^\n+\t\tset_reword_editor &&\n+\t\tGIT_SEQUENCE_EDITOR=\"\\\"$PWD/replace-editor.sh\\\"\" \\\n+\t\t\tgit rebase -i -r D\n \t) &&\n-\techo edited >expected &&\n-\tgit log --pretty=format:%B -1 >actual &&\n-\ttest_cmp expected actual\n+\tcheck_reworded_commits E H\n '\n \n test_expect_success 'with a branch tip that was cherry-picked already' '\n-- \ngitgitgadget\n\n"},{"id":"432672","messageId":"511cf9204ad5a785ffb2e2eebc757a4e58579826.1628860053.git.gitgitgadget@gmail.com","threadId":"56277","inReplyTo":"pull.1015.git.1628860053.gitgitgadget@gmail.com","subject":"[PATCH 2/4] rebase -i: Add another reword test","fromName":"Phillip Wood via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-08-13T13:07:31Z","receivedAt":"2021-08-13T13:07:40Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood <phillip.wood@dunelm.org.uk>\n\nNone of the existing reword tests check that there are no uncommitted\nchanges when the editor is opened. Reuse the editor script from the\nlast commit to fix this omission.\n\nSigned-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n---\n t/t3404-rebase-interactive.sh | 13 +++++++++++++\n 1 file changed, 13 insertions(+)\n\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 66bcbbf9528..d877872e8f4 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -839,6 +839,19 @@ test_expect_success 'reword' '\n \tgit show HEAD~2 | grep \"C changed\"\n '\n \n+test_expect_success 'no uncommited changes when rewording the todo list is reloaded' '\n+\tgit checkout E &&\n+\ttest_when_finished \"git checkout @{-1}\" &&\n+\t(\n+\t\tset_fake_editor &&\n+\t\tGIT_SEQUENCE_EDITOR=\"\\\"$PWD/fake-editor.sh\\\"\" &&\n+\t\texport GIT_SEQUENCE_EDITOR &&\n+\t\tset_reword_editor &&\n+\t\tFAKE_LINES=\"reword 1 reword 2\" git rebase -i C\n+\t) &&\n+\tcheck_reworded_commits D E\n+'\n+\n test_expect_success 'rebase -i can copy notes' '\n \tgit config notes.rewrite.rebase true &&\n \tgit config notes.rewriteRef \"refs/notes/*\" &&\n-- \ngitgitgadget\n\n"},{"id":"432673","messageId":"01d5ed4cba04d69e484d2c040635632df914bfa6.1628860053.git.gitgitgadget@gmail.com","threadId":"56277","inReplyTo":"pull.1015.git.1628860053.gitgitgadget@gmail.com","subject":"[PATCH 3/4] rebase -r: don't write .git/MERGE_MSG when fast-forwarding","fromName":"Phillip Wood via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-08-13T13:07:32Z","receivedAt":"2021-08-13T13:07:43Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood <phillip.wood@dunelm.org.uk>\n\nWhen fast-forwarding we do not create a new commit so .git/MERGE_MSG\nis not removed and can end up seeding the message of a commit made\nafter the rebase has finished. Avoid writing .git/MERGE_MSG when we\nare fast-forwarding by writing the file after the fast-forward\nchecks.\n\nNote that the way this change is implemented means we no longer write\nthe author script when fast-forwarding either. I believe this is safe\nfor the reasons below but it is a departure from what we do when\nfast-forwarding a non-merge commit. If we reword the merge then 'git\ncommit --amend' will keep the authorship of the commit we're rewording\nas it ignores GIT_AUTHOR_* unless --reset-author is passed. It will\nalso export the correct GIT_AUTHOR_* variables to any hooks and we\nalready test the authorship of the reworded commit. If we are not\nrewording then we no longer call spilt_ident() which means we are no\nlonger checking the commit author header looks sane. However this is\nwhat we already do when fast-forwarding non-merge commits in\nskip_unnecessary_picks() so I don't think we're breaking any promises\nby not checking the author here.\n\nReported-by: SZEDER Gábor <szeder.dev@gmail.com>\nSigned-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n---\n sequencer.c     | 81 +++++++++++++++++++++++++------------------------\n t/lib-rebase.sh | 10 ++++--\n 2 files changed, 49 insertions(+), 42 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex cc8a361cceb..c2cba5ed4b1 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -983,7 +983,8 @@ static int run_git_commit(const char *defmsg,\n \n \tcmd.git_cmd = 1;\n \n-\tif (is_rebase_i(opts) && read_env_script(&cmd.env_array)) {\n+\tif (is_rebase_i(opts) && !(!defmsg && (flags & AMEND_MSG)) &&\n+\t    read_env_script(&cmd.env_array)) {\n \t\tconst char *gpg_opt = gpg_sign_opt_quoted(opts);\n \n \t\treturn error(_(staged_changes_advice),\n@@ -3815,6 +3816,45 @@ static int do_merge(struct repository *r,\n \t\tgoto leave_merge;\n \t}\n \n+\t/*\n+\t * If HEAD is not identical to the first parent of the original merge\n+\t * commit, we cannot fast-forward.\n+\t */\n+\tcan_fast_forward = opts->allow_ff && commit && commit->parents &&\n+\t\toideq(&commit->parents->item->object.oid,\n+\t\t      &head_commit->object.oid);\n+\n+\t/*\n+\t * If any merge head is different from the original one, we cannot\n+\t * fast-forward.\n+\t */\n+\tif (can_fast_forward) {\n+\t\tstruct commit_list *p = commit->parents->next;\n+\n+\t\tfor (j = to_merge; j && p; j = j->next, p = p->next)\n+\t\t\tif (!oideq(&j->item->object.oid,\n+\t\t\t\t   &p->item->object.oid)) {\n+\t\t\t\tcan_fast_forward = 0;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t/*\n+\t\t * If the number of merge heads differs from the original merge\n+\t\t * commit, we cannot fast-forward.\n+\t\t */\n+\t\tif (j || p)\n+\t\t\tcan_fast_forward = 0;\n+\t}\n+\n+\tif (can_fast_forward) {\n+\t\trollback_lock_file(&lock);\n+\t\tret = fast_forward_to(r, &commit->object.oid,\n+\t\t\t\t      &head_commit->object.oid, 0, opts);\n+\t\tif (flags & TODO_EDIT_MERGE_MSG)\n+\t\t\tgoto fast_forward_edit;\n+\n+\t\tgoto leave_merge;\n+\t}\n+\n \tif (commit) {\n \t\tconst char *encoding = get_commit_output_encoding();\n \t\tconst char *message = logmsg_reencode(commit, NULL, encoding);\n@@ -3864,45 +3904,6 @@ static int do_merge(struct repository *r,\n \t\t}\n \t}\n \n-\t/*\n-\t * If HEAD is not identical to the first parent of the original merge\n-\t * commit, we cannot fast-forward.\n-\t */\n-\tcan_fast_forward = opts->allow_ff && commit && commit->parents &&\n-\t\toideq(&commit->parents->item->object.oid,\n-\t\t      &head_commit->object.oid);\n-\n-\t/*\n-\t * If any merge head is different from the original one, we cannot\n-\t * fast-forward.\n-\t */\n-\tif (can_fast_forward) {\n-\t\tstruct commit_list *p = commit->parents->next;\n-\n-\t\tfor (j = to_merge; j && p; j = j->next, p = p->next)\n-\t\t\tif (!oideq(&j->item->object.oid,\n-\t\t\t\t   &p->item->object.oid)) {\n-\t\t\t\tcan_fast_forward = 0;\n-\t\t\t\tbreak;\n-\t\t\t}\n-\t\t/*\n-\t\t * If the number of merge heads differs from the original merge\n-\t\t * commit, we cannot fast-forward.\n-\t\t */\n-\t\tif (j || p)\n-\t\t\tcan_fast_forward = 0;\n-\t}\n-\n-\tif (can_fast_forward) {\n-\t\trollback_lock_file(&lock);\n-\t\tret = fast_forward_to(r, &commit->object.oid,\n-\t\t\t\t      &head_commit->object.oid, 0, opts);\n-\t\tif (flags & TODO_EDIT_MERGE_MSG)\n-\t\t\tgoto fast_forward_edit;\n-\n-\t\tgoto leave_merge;\n-\t}\n-\n \tif (strategy || to_merge->next) {\n \t\t/* Octopus merge */\n \t\tstruct child_process cmd = CHILD_PROCESS_INIT;\ndiff --git a/t/lib-rebase.sh b/t/lib-rebase.sh\nindex 99d9e7efd2d..ec6b9b107da 100644\n--- a/t/lib-rebase.sh\n+++ b/t/lib-rebase.sh\n@@ -173,10 +173,16 @@ set_reword_editor () {\n \n \twrite_script reword-editor.sh <<-EOF &&\n \t# Save the oid of the first reworded commit so we can check rebase\n-\t# fast-forwards to it\n+\t# fast-forwards to it. Also check that we do not write .git/MERGE_MSG\n+\t# when fast-forwarding\n \tif ! test -s reword-oid\n \tthen\n-\t\tgit rev-parse HEAD >reword-oid\n+\t\tgit rev-parse HEAD >reword-oid &&\n+\t\tif test -f .git/MERGE_MSG\n+\t\tthen\n+\t\t\techo 1>&2 \"error: .git/MERGE_MSG exists\"\n+\t\t\texit 1\n+\t\tfi\n \tfi &&\n \t# There should be no uncommited changes\n \tgit diff --exit-code HEAD &&\n-- \ngitgitgadget\n\n"},{"id":"432674","messageId":"f2a2e3531a181f68cee43f832045633aee519fb8.1628860053.git.gitgitgadget@gmail.com","threadId":"56277","inReplyTo":"pull.1015.git.1628860053.gitgitgadget@gmail.com","subject":"[PATCH 4/4] rebase -r: fix merge -c with a merge strategy","fromName":"Phillip Wood via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-08-13T13:07:33Z","receivedAt":"2021-08-13T13:07:44Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood <phillip.wood@dunelm.org.uk>\n\nIf a rebase is started with a --strategy option other than \"ort\" or\n\"recursive\" then \"merge -c\" does not allow the user to reword the\ncommit message.\n\nSigned-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n---\n sequencer.c              |  5 ++++-\n t/t3430-rebase-merges.sh | 18 ++++++++++++++++++\n 2 files changed, 22 insertions(+), 1 deletion(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex c2cba5ed4b1..a19980f62d9 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -3934,7 +3934,10 @@ static int do_merge(struct repository *r,\n \t\t\t\tstrvec_pushf(&cmd.args,\n \t\t\t\t\t     \"-X%s\", opts->xopts[k]);\n \t\t}\n-\t\tstrvec_push(&cmd.args, \"--no-edit\");\n+\t\tif (!(flags & TODO_EDIT_MERGE_MSG))\n+\t\t\tstrvec_push(&cmd.args, \"--no-edit\");\n+\t\telse\n+\t\t\tstrvec_push(&cmd.args, \"--edit\");\n \t\tstrvec_push(&cmd.args, \"--no-ff\");\n \t\tstrvec_push(&cmd.args, \"--no-log\");\n \t\tstrvec_push(&cmd.args, \"--no-stat\");\ndiff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh\nindex 183c3a23838..43c82d9a33b 100755\n--- a/t/t3430-rebase-merges.sh\n+++ b/t/t3430-rebase-merges.sh\n@@ -187,6 +187,24 @@ test_expect_success 'merge -c commits before rewording and reloads todo-list' '\n \tcheck_reworded_commits E H\n '\n \n+test_expect_success 'merge -c rewords when a strategy is given' '\n+\tgit checkout -b merge-c-with-strategy H &&\n+\twrite_script git-merge-override <<-\\EOF &&\n+\techo overridden$1 >G.t\n+\tgit add G.t\n+\tEOF\n+\n+\tPATH=\"$PWD:$PATH\" \\\n+\tGIT_SEQUENCE_EDITOR=\"echo merge -c H G >\" \\\n+\tGIT_EDITOR=\"echo edited >>\" \\\n+\t\tgit rebase --no-ff -ir -s override -Xxopt E &&\n+\ttest_write_lines overridden--xopt >expect &&\n+\ttest_cmp expect G.t &&\n+\ttest_write_lines H \"\" edited \"\" >expect &&\n+\tgit log --format=%B -1 >actual &&\n+\ttest_cmp expect actual\n+\n+'\n test_expect_success 'with a branch tip that was cherry-picked already' '\n \tgit checkout -b already-upstream main &&\n \tbase=\"$(git rev-parse --verify HEAD)\" &&\n-- \ngitgitgadget\n"},{"id":"432755","messageId":"nycvar.QRO.7.76.6.2108150042250.59@tvgsbejvaqbjf.bet","threadId":"56277","inReplyTo":"pull.1015.git.1628860053.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/4] rebase -r: some merge related fixes","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-08-14T22:43:19Z","receivedAt":"2021-08-14T22:43:30Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Phillip,\n\nOn Fri, 13 Aug 2021, Phillip Wood via GitGitGadget wrote:\n\n> This is a collection of merge related fixes for rebase -r\n>\n>  * Make merge -c behave like reword.\n>  * When fast-forwarding a merge don't leave .git/MERGE_MSG around (reported\n>    by Gábor)\n>  * Make merge -c work when with --strategy\n>\n> Phillip Wood (4):\n>   rebase -r: make 'merge -c' behave like reword\n>   rebase -i: Add another reword test\n>   rebase -r: don't write .git/MERGE_MSG when fast-forwarding\n>   rebase -r: fix merge -c with a merge strategy\n\nI reviewed all four patches (the first one took the most time, obviously)\nand it was quite the pleasant read. I am in favor of integrating them\nas-are.\n\nThank you,\nDscho\n\n>\n>  sequencer.c                   | 106 ++++++++++++++++++----------------\n>  t/lib-rebase.sh               |  56 ++++++++++++++++++\n>  t/t3404-rebase-interactive.sh |  13 +++++\n>  t/t3430-rebase-merges.sh      |  38 +++++++++---\n>  4 files changed, 155 insertions(+), 58 deletions(-)\n>\n>\n> base-commit: 66262451ec94d30ac4b80eb3123549cf7a788afd\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1015%2Fphillipwood%2Fwip%2Fsequencer-merge-c-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1015/phillipwood/wip/sequencer-merge-c-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1015\n> --\n> gitgitgadget\n>\n"},{"id":"433004","messageId":"20210817172609.GB2257957@szeder.dev","threadId":"56277","inReplyTo":"01d5ed4cba04d69e484d2c040635632df914bfa6.1628860053.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 3/4] rebase -r: don't write .git/MERGE_MSG when fast-forwarding","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2021-08-17T17:26:09Z","receivedAt":"2021-08-17T17:26:15Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Fri, Aug 13, 2021 at 01:07:32PM +0000, Phillip Wood via GitGitGadget wrote:\n> From: Phillip Wood <phillip.wood@dunelm.org.uk>\n> \n> When fast-forwarding we do not create a new commit so .git/MERGE_MSG\n> is not removed and can end up seeding the message of a commit made\n> after the rebase has finished. Avoid writing .git/MERGE_MSG when we\n> are fast-forwarding by writing the file after the fast-forward\n> checks.\n> \n> Note that the way this change is implemented means we no longer write\n> the author script when fast-forwarding either. I believe this is safe\n> for the reasons below but it is a departure from what we do when\n> fast-forwarding a non-merge commit. If we reword the merge then 'git\n> commit --amend' will keep the authorship of the commit we're rewording\n> as it ignores GIT_AUTHOR_* unless --reset-author is passed. It will\n> also export the correct GIT_AUTHOR_* variables to any hooks and we\n> already test the authorship of the reworded commit. If we are not\n> rewording then we no longer call spilt_ident() which means we are no\n> longer checking the commit author header looks sane. However this is\n> what we already do when fast-forwarding non-merge commits in\n> skip_unnecessary_picks() so I don't think we're breaking any promises\n> by not checking the author here.\n\nThanks you for fixing this bug.\n\nFWIW (not that much, I'm afraid), I think your reasoning about the\nharmlessness of the behavior change concerning the author script makes\nsense.\n\nMy only nit is that the movement of a ~40 lines block of code makes\nout the bulk of the patch; perhaps it would be worth mentioning it\nexplicitly in the commit message, so future readers of this commit\nwon't look for changes in those hunks.\n\n> Reported-by: SZEDER Gábor <szeder.dev@gmail.com>\n> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n> ---\n>  sequencer.c     | 81 +++++++++++++++++++++++++------------------------\n>  t/lib-rebase.sh | 10 ++++--\n>  2 files changed, 49 insertions(+), 42 deletions(-)\n> \n> diff --git a/sequencer.c b/sequencer.c\n> index cc8a361cceb..c2cba5ed4b1 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -983,7 +983,8 @@ static int run_git_commit(const char *defmsg,\n>  \n>  \tcmd.git_cmd = 1;\n>  \n> -\tif (is_rebase_i(opts) && read_env_script(&cmd.env_array)) {\n> +\tif (is_rebase_i(opts) && !(!defmsg && (flags & AMEND_MSG)) &&\n> +\t    read_env_script(&cmd.env_array)) {\n>  \t\tconst char *gpg_opt = gpg_sign_opt_quoted(opts);\n>  \n>  \t\treturn error(_(staged_changes_advice),\n> @@ -3815,6 +3816,45 @@ static int do_merge(struct repository *r,\n>  \t\tgoto leave_merge;\n>  \t}\n>  \n> +\t/*\n> +\t * If HEAD is not identical to the first parent of the original merge\n> +\t * commit, we cannot fast-forward.\n> +\t */\n> +\tcan_fast_forward = opts->allow_ff && commit && commit->parents &&\n> +\t\toideq(&commit->parents->item->object.oid,\n> +\t\t      &head_commit->object.oid);\n> +\n> +\t/*\n> +\t * If any merge head is different from the original one, we cannot\n> +\t * fast-forward.\n> +\t */\n> +\tif (can_fast_forward) {\n> +\t\tstruct commit_list *p = commit->parents->next;\n> +\n> +\t\tfor (j = to_merge; j && p; j = j->next, p = p->next)\n> +\t\t\tif (!oideq(&j->item->object.oid,\n> +\t\t\t\t   &p->item->object.oid)) {\n> +\t\t\t\tcan_fast_forward = 0;\n> +\t\t\t\tbreak;\n> +\t\t\t}\n> +\t\t/*\n> +\t\t * If the number of merge heads differs from the original merge\n> +\t\t * commit, we cannot fast-forward.\n> +\t\t */\n> +\t\tif (j || p)\n> +\t\t\tcan_fast_forward = 0;\n> +\t}\n> +\n> +\tif (can_fast_forward) {\n> +\t\trollback_lock_file(&lock);\n> +\t\tret = fast_forward_to(r, &commit->object.oid,\n> +\t\t\t\t      &head_commit->object.oid, 0, opts);\n> +\t\tif (flags & TODO_EDIT_MERGE_MSG)\n> +\t\t\tgoto fast_forward_edit;\n> +\n> +\t\tgoto leave_merge;\n> +\t}\n> +\n>  \tif (commit) {\n>  \t\tconst char *encoding = get_commit_output_encoding();\n>  \t\tconst char *message = logmsg_reencode(commit, NULL, encoding);\n> @@ -3864,45 +3904,6 @@ static int do_merge(struct repository *r,\n>  \t\t}\n>  \t}\n>  \n> -\t/*\n> -\t * If HEAD is not identical to the first parent of the original merge\n> -\t * commit, we cannot fast-forward.\n> -\t */\n> -\tcan_fast_forward = opts->allow_ff && commit && commit->parents &&\n> -\t\toideq(&commit->parents->item->object.oid,\n> -\t\t      &head_commit->object.oid);\n> -\n> -\t/*\n> -\t * If any merge head is different from the original one, we cannot\n> -\t * fast-forward.\n> -\t */\n> -\tif (can_fast_forward) {\n> -\t\tstruct commit_list *p = commit->parents->next;\n> -\n> -\t\tfor (j = to_merge; j && p; j = j->next, p = p->next)\n> -\t\t\tif (!oideq(&j->item->object.oid,\n> -\t\t\t\t   &p->item->object.oid)) {\n> -\t\t\t\tcan_fast_forward = 0;\n> -\t\t\t\tbreak;\n> -\t\t\t}\n> -\t\t/*\n> -\t\t * If the number of merge heads differs from the original merge\n> -\t\t * commit, we cannot fast-forward.\n> -\t\t */\n> -\t\tif (j || p)\n> -\t\t\tcan_fast_forward = 0;\n> -\t}\n> -\n> -\tif (can_fast_forward) {\n> -\t\trollback_lock_file(&lock);\n> -\t\tret = fast_forward_to(r, &commit->object.oid,\n> -\t\t\t\t      &head_commit->object.oid, 0, opts);\n> -\t\tif (flags & TODO_EDIT_MERGE_MSG)\n> -\t\t\tgoto fast_forward_edit;\n> -\n> -\t\tgoto leave_merge;\n> -\t}\n> -\n>  \tif (strategy || to_merge->next) {\n>  \t\t/* Octopus merge */\n>  \t\tstruct child_process cmd = CHILD_PROCESS_INIT;\n> diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh\n> index 99d9e7efd2d..ec6b9b107da 100644\n> --- a/t/lib-rebase.sh\n> +++ b/t/lib-rebase.sh\n> @@ -173,10 +173,16 @@ set_reword_editor () {\n>  \n>  \twrite_script reword-editor.sh <<-EOF &&\n>  \t# Save the oid of the first reworded commit so we can check rebase\n> -\t# fast-forwards to it\n> +\t# fast-forwards to it. Also check that we do not write .git/MERGE_MSG\n> +\t# when fast-forwarding\n>  \tif ! test -s reword-oid\n>  \tthen\n> -\t\tgit rev-parse HEAD >reword-oid\n> +\t\tgit rev-parse HEAD >reword-oid &&\n> +\t\tif test -f .git/MERGE_MSG\n> +\t\tthen\n> +\t\t\techo 1>&2 \"error: .git/MERGE_MSG exists\"\n> +\t\t\texit 1\n> +\t\tfi\n>  \tfi &&\n>  \t# There should be no uncommited changes\n>  \tgit diff --exit-code HEAD &&\n> -- \n> gitgitgadget\n> \n"},{"id":"433126","messageId":"a6454b5f-dbf5-56e4-eb91-1db876914a78@gmail.com","threadId":"56277","inReplyTo":"nycvar.QRO.7.76.6.2108150042250.59@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 0/4] rebase -r: some merge related fixes","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2021-08-19T10:07:52Z","receivedAt":"2021-08-19T10:08:01Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Dscho\n\nOn 14/08/2021 23:43, Johannes Schindelin wrote:\n> Hi Phillip,\n> \n> On Fri, 13 Aug 2021, Phillip Wood via GitGitGadget wrote:\n> \n>> This is a collection of merge related fixes for rebase -r\n>>\n>>   * Make merge -c behave like reword.\n>>   * When fast-forwarding a merge don't leave .git/MERGE_MSG around (reported\n>>     by Gábor)\n>>   * Make merge -c work when with --strategy\n>>\n>> Phillip Wood (4):\n>>    rebase -r: make 'merge -c' behave like reword\n>>    rebase -i: Add another reword test\n>>    rebase -r: don't write .git/MERGE_MSG when fast-forwarding\n>>    rebase -r: fix merge -c with a merge strategy\n> \n> I reviewed all four patches (the first one took the most time, obviously)\n> and it was quite the pleasant read. I am in favor of integrating them\n> as-are.\n\nThanks for reviewing them\n\nBest Wishes\n\nPhillip\n\n> Thank you,\n> Dscho\n> \n>>\n>>   sequencer.c                   | 106 ++++++++++++++++++----------------\n>>   t/lib-rebase.sh               |  56 ++++++++++++++++++\n>>   t/t3404-rebase-interactive.sh |  13 +++++\n>>   t/t3430-rebase-merges.sh      |  38 +++++++++---\n>>   4 files changed, 155 insertions(+), 58 deletions(-)\n>>\n>>\n>> base-commit: 66262451ec94d30ac4b80eb3123549cf7a788afd\n>> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1015%2Fphillipwood%2Fwip%2Fsequencer-merge-c-v1\n>> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1015/phillipwood/wip/sequencer-merge-c-v1\n>> Pull-Request: https://github.com/gitgitgadget/git/pull/1015\n>> --\n>> gitgitgadget\n>>\n\n"},{"id":"433127","messageId":"49aba79b-14ed-cee0-4e4c-1f98306f1b45@gmail.com","threadId":"56277","inReplyTo":"20210817172609.GB2257957@szeder.dev","subject":"Re: [PATCH 3/4] rebase -r: don't write .git/MERGE_MSG when fast-forwarding","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2021-08-19T10:09:26Z","receivedAt":"2021-08-19T10:09:30Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 17/08/2021 18:26, SZEDER Gábor wrote:\n> On Fri, Aug 13, 2021 at 01:07:32PM +0000, Phillip Wood via GitGitGadget wrote:\n>> From: Phillip Wood <phillip.wood@dunelm.org.uk>\n>>\n>> When fast-forwarding we do not create a new commit so .git/MERGE_MSG\n>> is not removed and can end up seeding the message of a commit made\n>> after the rebase has finished. Avoid writing .git/MERGE_MSG when we\n>> are fast-forwarding by writing the file after the fast-forward\n>> checks.\n>>\n>> Note that the way this change is implemented means we no longer write\n>> the author script when fast-forwarding either. I believe this is safe\n>> for the reasons below but it is a departure from what we do when\n>> fast-forwarding a non-merge commit. If we reword the merge then 'git\n>> commit --amend' will keep the authorship of the commit we're rewording\n>> as it ignores GIT_AUTHOR_* unless --reset-author is passed. It will\n>> also export the correct GIT_AUTHOR_* variables to any hooks and we\n>> already test the authorship of the reworded commit. If we are not\n>> rewording then we no longer call spilt_ident() which means we are no\n>> longer checking the commit author header looks sane. However this is\n>> what we already do when fast-forwarding non-merge commits in\n>> skip_unnecessary_picks() so I don't think we're breaking any promises\n>> by not checking the author here.\n> \n> Thanks you for fixing this bug.\n> \n> FWIW (not that much, I'm afraid), I think your reasoning about the\n> harmlessness of the behavior change concerning the author script makes\n> sense.\n> \n> My only nit is that the movement of a ~40 lines block of code makes\n> out the bulk of the patch; perhaps it would be worth mentioning it\n> explicitly in the commit message, so future readers of this commit\n> won't look for changes in those hunks.\n\nThanks for reading through the patch, I take your point about the code \nmovement and will add a comment (I have --color-moved turned on by \ndefault and tend to forget others may not)\n\nBest Wishes\n\nPhillip\n\n>> Reported-by: SZEDER Gábor <szeder.dev@gmail.com>\n>> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n>> ---\n>>   sequencer.c     | 81 +++++++++++++++++++++++++------------------------\n>>   t/lib-rebase.sh | 10 ++++--\n>>   2 files changed, 49 insertions(+), 42 deletions(-)\n>>\n>> diff --git a/sequencer.c b/sequencer.c\n>> index cc8a361cceb..c2cba5ed4b1 100644\n>> --- a/sequencer.c\n>> +++ b/sequencer.c\n>> @@ -983,7 +983,8 @@ static int run_git_commit(const char *defmsg,\n>>   \n>>   \tcmd.git_cmd = 1;\n>>   \n>> -\tif (is_rebase_i(opts) && read_env_script(&cmd.env_array)) {\n>> +\tif (is_rebase_i(opts) && !(!defmsg && (flags & AMEND_MSG)) &&\n>> +\t    read_env_script(&cmd.env_array)) {\n>>   \t\tconst char *gpg_opt = gpg_sign_opt_quoted(opts);\n>>   \n>>   \t\treturn error(_(staged_changes_advice),\n>> @@ -3815,6 +3816,45 @@ static int do_merge(struct repository *r,\n>>   \t\tgoto leave_merge;\n>>   \t}\n>>   \n>> +\t/*\n>> +\t * If HEAD is not identical to the first parent of the original merge\n>> +\t * commit, we cannot fast-forward.\n>> +\t */\n>> +\tcan_fast_forward = opts->allow_ff && commit && commit->parents &&\n>> +\t\toideq(&commit->parents->item->object.oid,\n>> +\t\t      &head_commit->object.oid);\n>> +\n>> +\t/*\n>> +\t * If any merge head is different from the original one, we cannot\n>> +\t * fast-forward.\n>> +\t */\n>> +\tif (can_fast_forward) {\n>> +\t\tstruct commit_list *p = commit->parents->next;\n>> +\n>> +\t\tfor (j = to_merge; j && p; j = j->next, p = p->next)\n>> +\t\t\tif (!oideq(&j->item->object.oid,\n>> +\t\t\t\t   &p->item->object.oid)) {\n>> +\t\t\t\tcan_fast_forward = 0;\n>> +\t\t\t\tbreak;\n>> +\t\t\t}\n>> +\t\t/*\n>> +\t\t * If the number of merge heads differs from the original merge\n>> +\t\t * commit, we cannot fast-forward.\n>> +\t\t */\n>> +\t\tif (j || p)\n>> +\t\t\tcan_fast_forward = 0;\n>> +\t}\n>> +\n>> +\tif (can_fast_forward) {\n>> +\t\trollback_lock_file(&lock);\n>> +\t\tret = fast_forward_to(r, &commit->object.oid,\n>> +\t\t\t\t      &head_commit->object.oid, 0, opts);\n>> +\t\tif (flags & TODO_EDIT_MERGE_MSG)\n>> +\t\t\tgoto fast_forward_edit;\n>> +\n>> +\t\tgoto leave_merge;\n>> +\t}\n>> +\n>>   \tif (commit) {\n>>   \t\tconst char *encoding = get_commit_output_encoding();\n>>   \t\tconst char *message = logmsg_reencode(commit, NULL, encoding);\n>> @@ -3864,45 +3904,6 @@ static int do_merge(struct repository *r,\n>>   \t\t}\n>>   \t}\n>>   \n>> -\t/*\n>> -\t * If HEAD is not identical to the first parent of the original merge\n>> -\t * commit, we cannot fast-forward.\n>> -\t */\n>> -\tcan_fast_forward = opts->allow_ff && commit && commit->parents &&\n>> -\t\toideq(&commit->parents->item->object.oid,\n>> -\t\t      &head_commit->object.oid);\n>> -\n>> -\t/*\n>> -\t * If any merge head is different from the original one, we cannot\n>> -\t * fast-forward.\n>> -\t */\n>> -\tif (can_fast_forward) {\n>> -\t\tstruct commit_list *p = commit->parents->next;\n>> -\n>> -\t\tfor (j = to_merge; j && p; j = j->next, p = p->next)\n>> -\t\t\tif (!oideq(&j->item->object.oid,\n>> -\t\t\t\t   &p->item->object.oid)) {\n>> -\t\t\t\tcan_fast_forward = 0;\n>> -\t\t\t\tbreak;\n>> -\t\t\t}\n>> -\t\t/*\n>> -\t\t * If the number of merge heads differs from the original merge\n>> -\t\t * commit, we cannot fast-forward.\n>> -\t\t */\n>> -\t\tif (j || p)\n>> -\t\t\tcan_fast_forward = 0;\n>> -\t}\n>> -\n>> -\tif (can_fast_forward) {\n>> -\t\trollback_lock_file(&lock);\n>> -\t\tret = fast_forward_to(r, &commit->object.oid,\n>> -\t\t\t\t      &head_commit->object.oid, 0, opts);\n>> -\t\tif (flags & TODO_EDIT_MERGE_MSG)\n>> -\t\t\tgoto fast_forward_edit;\n>> -\n>> -\t\tgoto leave_merge;\n>> -\t}\n>> -\n>>   \tif (strategy || to_merge->next) {\n>>   \t\t/* Octopus merge */\n>>   \t\tstruct child_process cmd = CHILD_PROCESS_INIT;\n>> diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh\n>> index 99d9e7efd2d..ec6b9b107da 100644\n>> --- a/t/lib-rebase.sh\n>> +++ b/t/lib-rebase.sh\n>> @@ -173,10 +173,16 @@ set_reword_editor () {\n>>   \n>>   \twrite_script reword-editor.sh <<-EOF &&\n>>   \t# Save the oid of the first reworded commit so we can check rebase\n>> -\t# fast-forwards to it\n>> +\t# fast-forwards to it. Also check that we do not write .git/MERGE_MSG\n>> +\t# when fast-forwarding\n>>   \tif ! test -s reword-oid\n>>   \tthen\n>> -\t\tgit rev-parse HEAD >reword-oid\n>> +\t\tgit rev-parse HEAD >reword-oid &&\n>> +\t\tif test -f .git/MERGE_MSG\n>> +\t\tthen\n>> +\t\t\techo 1>&2 \"error: .git/MERGE_MSG exists\"\n>> +\t\t\texit 1\n>> +\t\tfi\n>>   \tfi &&\n>>   \t# There should be no uncommited changes\n>>   \tgit diff --exit-code HEAD &&\n>> -- \n>> gitgitgadget\n>>\n\n"},{"id":"433232","messageId":"pull.1015.v2.git.1629474038.gitgitgadget@gmail.com","threadId":"56277","inReplyTo":"pull.1015.git.1628860053.gitgitgadget@gmail.com","subject":"[PATCH v2 0/4] rebase -r: some merge related fixes","fromName":"Phillip Wood via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-08-20T15:40:34Z","receivedAt":"2021-08-20T15:40:43Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Thanks for the feedback on V1. I have added a sentence to the commit message\nof the third patch to make it clear that the fast-forward code is just\nmoved, not changed\n\nCover letter from V1: This is a collection of merge related fixes for rebase\n-r\n\n * Make merge -c behave like reword.\n * When fast-forwarding a merge don't leave .git/MERGE_MSG around (reported\n   by Gábor)\n * Make merge -c work when with --strategy\n\nPhillip Wood (4):\n  rebase -r: make 'merge -c' behave like reword\n  rebase -i: Add another reword test\n  rebase -r: don't write .git/MERGE_MSG when fast-forwarding\n  rebase -r: fix merge -c with a merge strategy\n\n sequencer.c                   | 106 ++++++++++++++++++----------------\n t/lib-rebase.sh               |  56 ++++++++++++++++++\n t/t3404-rebase-interactive.sh |  13 +++++\n t/t3430-rebase-merges.sh      |  38 +++++++++---\n 4 files changed, 155 insertions(+), 58 deletions(-)\n\n\nbase-commit: 66262451ec94d30ac4b80eb3123549cf7a788afd\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1015%2Fphillipwood%2Fwip%2Fsequencer-merge-c-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1015/phillipwood/wip/sequencer-merge-c-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1015\n\nRange-diff vs v1:\n\n 1:  b514dbdf928 = 1:  b514dbdf928 rebase -r: make 'merge -c' behave like reword\n 2:  511cf9204ad = 2:  511cf9204ad rebase -i: Add another reword test\n 3:  01d5ed4cba0 ! 3:  080e580e11c rebase -r: don't write .git/MERGE_MSG when fast-forwarding\n     @@ Commit message\n          is not removed and can end up seeding the message of a commit made\n          after the rebase has finished. Avoid writing .git/MERGE_MSG when we\n          are fast-forwarding by writing the file after the fast-forward\n     -    checks.\n     +    checks. Note that there are no changes to the fast-forward code, it is\n     +    simply moved.\n      \n          Note that the way this change is implemented means we no longer write\n          the author script when fast-forwarding either. I believe this is safe\n 4:  f2a2e3531a1 = 4:  b6981ea5439 rebase -r: fix merge -c with a merge strategy\n\n-- \ngitgitgadget\n"},{"id":"433233","messageId":"b514dbdf9287128b8e0fab1d1d5b5107de15764e.1629474038.git.gitgitgadget@gmail.com","threadId":"56277","inReplyTo":"pull.1015.v2.git.1629474038.gitgitgadget@gmail.com","subject":"[PATCH v2 1/4] rebase -r: make 'merge -c' behave like reword","fromName":"Phillip Wood via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-08-20T15:40:35Z","receivedAt":"2021-08-20T15:40:45Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood <phillip.wood@dunelm.org.uk>\n\nIf the user runs git log while rewording a commit it is confusing if\nsometimes we're amending the commit that's being reworded and at other\ntimes we're creating a new commit depending on whether we could\nfast-forward or not[1]. For this reason the reword command ensures\nthat there are no uncommitted changes when rewording. The reword\ncommand also allows the user to edit the todo list while the rebase is\npaused. As 'merge -c' also rewords commits make it behave like reword\nand add a test.\n\n[1] https://lore.kernel.org/git/xmqqlfvu4be3.fsf@gitster-ct.c.googlers.com/T/#m133009cb91cf0917bcf667300f061178be56680a\n\nSigned-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n---\n sequencer.c              | 24 +++++++++++--------\n t/lib-rebase.sh          | 50 ++++++++++++++++++++++++++++++++++++++++\n t/t3430-rebase-merges.sh | 20 ++++++++--------\n 3 files changed, 75 insertions(+), 19 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 7f07cd00f3f..cc8a361cceb 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -3739,10 +3739,9 @@ static struct commit *lookup_label(const char *label, int len,\n static int do_merge(struct repository *r,\n \t\t    struct commit *commit,\n \t\t    const char *arg, int arg_len,\n-\t\t    int flags, struct replay_opts *opts)\n+\t\t    int flags, int *check_todo, struct replay_opts *opts)\n {\n-\tint run_commit_flags = (flags & TODO_EDIT_MERGE_MSG) ?\n-\t\tEDIT_MSG | VERIFY_MSG : 0;\n+\tint run_commit_flags = 0;\n \tstruct strbuf ref_name = STRBUF_INIT;\n \tstruct commit *head_commit, *merge_commit, *i;\n \tstruct commit_list *bases, *j, *reversed = NULL;\n@@ -3898,10 +3897,9 @@ static int do_merge(struct repository *r,\n \t\trollback_lock_file(&lock);\n \t\tret = fast_forward_to(r, &commit->object.oid,\n \t\t\t\t      &head_commit->object.oid, 0, opts);\n-\t\tif (flags & TODO_EDIT_MERGE_MSG) {\n-\t\t\trun_commit_flags |= AMEND_MSG;\n+\t\tif (flags & TODO_EDIT_MERGE_MSG)\n \t\t\tgoto fast_forward_edit;\n-\t\t}\n+\n \t\tgoto leave_merge;\n \t}\n \n@@ -4035,10 +4033,17 @@ static int do_merge(struct repository *r,\n \t\t * value (a negative one would indicate that the `merge`\n \t\t * command needs to be rescheduled).\n \t\t */\n-\tfast_forward_edit:\n \t\tret = !!run_git_commit(git_path_merge_msg(r), opts,\n \t\t\t\t       run_commit_flags);\n \n+\tif (!ret && flags & TODO_EDIT_MERGE_MSG) {\n+\tfast_forward_edit:\n+\t\t*check_todo = 1;\n+\t\trun_commit_flags |= AMEND_MSG | EDIT_MSG | VERIFY_MSG;\n+\t\tret = !!run_git_commit(NULL, opts, run_commit_flags);\n+\t}\n+\n+\n leave_merge:\n \tstrbuf_release(&ref_name);\n \trollback_lock_file(&lock);\n@@ -4405,9 +4410,8 @@ static int pick_commits(struct repository *r,\n \t\t\tif ((res = do_reset(r, arg, item->arg_len, opts)))\n \t\t\t\treschedule = 1;\n \t\t} else if (item->command == TODO_MERGE) {\n-\t\t\tif ((res = do_merge(r, item->commit,\n-\t\t\t\t\t    arg, item->arg_len,\n-\t\t\t\t\t    item->flags, opts)) < 0)\n+\t\t\tif ((res = do_merge(r, item->commit, arg, item->arg_len,\n+\t\t\t\t\t    item->flags, &check_todo, opts)) < 0)\n \t\t\t\treschedule = 1;\n \t\t\telse if (item->commit)\n \t\t\t\trecord_in_rewritten(&item->commit->object.oid,\ndiff --git a/t/lib-rebase.sh b/t/lib-rebase.sh\nindex dc75b834518..99d9e7efd2d 100644\n--- a/t/lib-rebase.sh\n+++ b/t/lib-rebase.sh\n@@ -151,3 +151,53 @@ test_editor_unchanged () {\n \tEOF\n \ttest_cmp expect actual\n }\n+\n+# Set up an editor for testing reword commands\n+# Checks that there are no uncommitted changes when rewording and that the\n+# todo-list is reread after each\n+set_reword_editor () {\n+\t>reword-actual &&\n+\t>reword-oid &&\n+\n+\t# Check rewording keeps the original authorship\n+\tGIT_AUTHOR_NAME=\"Reword Author\"\n+\tGIT_AUTHOR_EMAIL=\"reword.author@example.com\"\n+\tGIT_AUTHOR_DATE=@123456\n+\n+\twrite_script reword-sequence-editor.sh <<-\\EOF &&\n+\ttodo=\"$(cat \"$1\")\" &&\n+\techo \"exec git log -1 --pretty=format:'%an <%ae> %at%n%B%n' \\\n+\t\t>>reword-actual\" >\"$1\" &&\n+\tprintf \"%s\\n\" \"$todo\" >>\"$1\"\n+\tEOF\n+\n+\twrite_script reword-editor.sh <<-EOF &&\n+\t# Save the oid of the first reworded commit so we can check rebase\n+\t# fast-forwards to it\n+\tif ! test -s reword-oid\n+\tthen\n+\t\tgit rev-parse HEAD >reword-oid\n+\tfi &&\n+\t# There should be no uncommited changes\n+\tgit diff --exit-code HEAD &&\n+\t# The todo-list should be re-read after a reword\n+\tGIT_SEQUENCE_EDITOR=\"\\\"$PWD/reword-sequence-editor.sh\\\"\" \\\n+\t\tgit rebase --edit-todo &&\n+\techo edited >>\"\\$1\"\n+\tEOF\n+\n+\ttest_set_editor \"$PWD/reword-editor.sh\"\n+}\n+\n+# Check the results of a rebase after calling set_reword_editor\n+# Pass the commits that were reworded in the order that they were picked\n+# Expects the first pick to be a fast-forward\n+check_reworded_commits () {\n+\ttest_cmp_rev \"$(cat reword-oid)\" \"$1^{commit}\" &&\n+\tgit log --format=\"%an <%ae> %at%n%B%nedited%n\" --no-walk=unsorted \"$@\" \\\n+\t\t>reword-expected &&\n+\ttest_cmp reword-expected reword-actual &&\n+\tgit log --format=\"%an <%ae> %at%n%B\" -n $# --first-parent --reverse \\\n+\t\t>reword-log &&\n+\ttest_cmp reword-expected reword-log\n+}\ndiff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh\nindex 6748070df52..183c3a23838 100755\n--- a/t/t3430-rebase-merges.sh\n+++ b/t/t3430-rebase-merges.sh\n@@ -172,17 +172,19 @@ test_expect_success 'failed `merge <branch>` does not crash' '\n \tgrep \"^Merge branch ${SQ}G${SQ}$\" .git/rebase-merge/message\n '\n \n-test_expect_success 'fast-forward merge -c still rewords' '\n-\tgit checkout -b fast-forward-merge-c H &&\n+test_expect_success 'merge -c commits before rewording and reloads todo-list' '\n+\tcat >script-from-scratch <<-\\EOF &&\n+\tmerge -c E B\n+\tmerge -c H G\n+\tEOF\n+\n+\tgit checkout -b merge-c H &&\n \t(\n-\t\tset_fake_editor &&\n-\t\tFAKE_COMMIT_MESSAGE=edited \\\n-\t\t\tGIT_SEQUENCE_EDITOR=\"echo merge -c H G >\" \\\n-\t\t\tgit rebase -ir @^\n+\t\tset_reword_editor &&\n+\t\tGIT_SEQUENCE_EDITOR=\"\\\"$PWD/replace-editor.sh\\\"\" \\\n+\t\t\tgit rebase -i -r D\n \t) &&\n-\techo edited >expected &&\n-\tgit log --pretty=format:%B -1 >actual &&\n-\ttest_cmp expected actual\n+\tcheck_reworded_commits E H\n '\n \n test_expect_success 'with a branch tip that was cherry-picked already' '\n-- \ngitgitgadget\n\n"},{"id":"433234","messageId":"511cf9204ad5a785ffb2e2eebc757a4e58579826.1629474038.git.gitgitgadget@gmail.com","threadId":"56277","inReplyTo":"pull.1015.v2.git.1629474038.gitgitgadget@gmail.com","subject":"[PATCH v2 2/4] rebase -i: Add another reword test","fromName":"Phillip Wood via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-08-20T15:40:36Z","receivedAt":"2021-08-20T15:40:49Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood <phillip.wood@dunelm.org.uk>\n\nNone of the existing reword tests check that there are no uncommitted\nchanges when the editor is opened. Reuse the editor script from the\nlast commit to fix this omission.\n\nSigned-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n---\n t/t3404-rebase-interactive.sh | 13 +++++++++++++\n 1 file changed, 13 insertions(+)\n\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 66bcbbf9528..d877872e8f4 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -839,6 +839,19 @@ test_expect_success 'reword' '\n \tgit show HEAD~2 | grep \"C changed\"\n '\n \n+test_expect_success 'no uncommited changes when rewording the todo list is reloaded' '\n+\tgit checkout E &&\n+\ttest_when_finished \"git checkout @{-1}\" &&\n+\t(\n+\t\tset_fake_editor &&\n+\t\tGIT_SEQUENCE_EDITOR=\"\\\"$PWD/fake-editor.sh\\\"\" &&\n+\t\texport GIT_SEQUENCE_EDITOR &&\n+\t\tset_reword_editor &&\n+\t\tFAKE_LINES=\"reword 1 reword 2\" git rebase -i C\n+\t) &&\n+\tcheck_reworded_commits D E\n+'\n+\n test_expect_success 'rebase -i can copy notes' '\n \tgit config notes.rewrite.rebase true &&\n \tgit config notes.rewriteRef \"refs/notes/*\" &&\n-- \ngitgitgadget\n\n"},{"id":"433235","messageId":"080e580e11c10526ae828caec242851c7823c2ce.1629474038.git.gitgitgadget@gmail.com","threadId":"56277","inReplyTo":"pull.1015.v2.git.1629474038.gitgitgadget@gmail.com","subject":"[PATCH v2 3/4] rebase -r: don't write .git/MERGE_MSG when fast-forwarding","fromName":"Phillip Wood via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-08-20T15:40:37Z","receivedAt":"2021-08-20T15:40:50Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood <phillip.wood@dunelm.org.uk>\n\nWhen fast-forwarding we do not create a new commit so .git/MERGE_MSG\nis not removed and can end up seeding the message of a commit made\nafter the rebase has finished. Avoid writing .git/MERGE_MSG when we\nare fast-forwarding by writing the file after the fast-forward\nchecks. Note that there are no changes to the fast-forward code, it is\nsimply moved.\n\nNote that the way this change is implemented means we no longer write\nthe author script when fast-forwarding either. I believe this is safe\nfor the reasons below but it is a departure from what we do when\nfast-forwarding a non-merge commit. If we reword the merge then 'git\ncommit --amend' will keep the authorship of the commit we're rewording\nas it ignores GIT_AUTHOR_* unless --reset-author is passed. It will\nalso export the correct GIT_AUTHOR_* variables to any hooks and we\nalready test the authorship of the reworded commit. If we are not\nrewording then we no longer call spilt_ident() which means we are no\nlonger checking the commit author header looks sane. However this is\nwhat we already do when fast-forwarding non-merge commits in\nskip_unnecessary_picks() so I don't think we're breaking any promises\nby not checking the author here.\n\nReported-by: SZEDER Gábor <szeder.dev@gmail.com>\nSigned-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n---\n sequencer.c     | 81 +++++++++++++++++++++++++------------------------\n t/lib-rebase.sh | 10 ++++--\n 2 files changed, 49 insertions(+), 42 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex cc8a361cceb..c2cba5ed4b1 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -983,7 +983,8 @@ static int run_git_commit(const char *defmsg,\n \n \tcmd.git_cmd = 1;\n \n-\tif (is_rebase_i(opts) && read_env_script(&cmd.env_array)) {\n+\tif (is_rebase_i(opts) && !(!defmsg && (flags & AMEND_MSG)) &&\n+\t    read_env_script(&cmd.env_array)) {\n \t\tconst char *gpg_opt = gpg_sign_opt_quoted(opts);\n \n \t\treturn error(_(staged_changes_advice),\n@@ -3815,6 +3816,45 @@ static int do_merge(struct repository *r,\n \t\tgoto leave_merge;\n \t}\n \n+\t/*\n+\t * If HEAD is not identical to the first parent of the original merge\n+\t * commit, we cannot fast-forward.\n+\t */\n+\tcan_fast_forward = opts->allow_ff && commit && commit->parents &&\n+\t\toideq(&commit->parents->item->object.oid,\n+\t\t      &head_commit->object.oid);\n+\n+\t/*\n+\t * If any merge head is different from the original one, we cannot\n+\t * fast-forward.\n+\t */\n+\tif (can_fast_forward) {\n+\t\tstruct commit_list *p = commit->parents->next;\n+\n+\t\tfor (j = to_merge; j && p; j = j->next, p = p->next)\n+\t\t\tif (!oideq(&j->item->object.oid,\n+\t\t\t\t   &p->item->object.oid)) {\n+\t\t\t\tcan_fast_forward = 0;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t/*\n+\t\t * If the number of merge heads differs from the original merge\n+\t\t * commit, we cannot fast-forward.\n+\t\t */\n+\t\tif (j || p)\n+\t\t\tcan_fast_forward = 0;\n+\t}\n+\n+\tif (can_fast_forward) {\n+\t\trollback_lock_file(&lock);\n+\t\tret = fast_forward_to(r, &commit->object.oid,\n+\t\t\t\t      &head_commit->object.oid, 0, opts);\n+\t\tif (flags & TODO_EDIT_MERGE_MSG)\n+\t\t\tgoto fast_forward_edit;\n+\n+\t\tgoto leave_merge;\n+\t}\n+\n \tif (commit) {\n \t\tconst char *encoding = get_commit_output_encoding();\n \t\tconst char *message = logmsg_reencode(commit, NULL, encoding);\n@@ -3864,45 +3904,6 @@ static int do_merge(struct repository *r,\n \t\t}\n \t}\n \n-\t/*\n-\t * If HEAD is not identical to the first parent of the original merge\n-\t * commit, we cannot fast-forward.\n-\t */\n-\tcan_fast_forward = opts->allow_ff && commit && commit->parents &&\n-\t\toideq(&commit->parents->item->object.oid,\n-\t\t      &head_commit->object.oid);\n-\n-\t/*\n-\t * If any merge head is different from the original one, we cannot\n-\t * fast-forward.\n-\t */\n-\tif (can_fast_forward) {\n-\t\tstruct commit_list *p = commit->parents->next;\n-\n-\t\tfor (j = to_merge; j && p; j = j->next, p = p->next)\n-\t\t\tif (!oideq(&j->item->object.oid,\n-\t\t\t\t   &p->item->object.oid)) {\n-\t\t\t\tcan_fast_forward = 0;\n-\t\t\t\tbreak;\n-\t\t\t}\n-\t\t/*\n-\t\t * If the number of merge heads differs from the original merge\n-\t\t * commit, we cannot fast-forward.\n-\t\t */\n-\t\tif (j || p)\n-\t\t\tcan_fast_forward = 0;\n-\t}\n-\n-\tif (can_fast_forward) {\n-\t\trollback_lock_file(&lock);\n-\t\tret = fast_forward_to(r, &commit->object.oid,\n-\t\t\t\t      &head_commit->object.oid, 0, opts);\n-\t\tif (flags & TODO_EDIT_MERGE_MSG)\n-\t\t\tgoto fast_forward_edit;\n-\n-\t\tgoto leave_merge;\n-\t}\n-\n \tif (strategy || to_merge->next) {\n \t\t/* Octopus merge */\n \t\tstruct child_process cmd = CHILD_PROCESS_INIT;\ndiff --git a/t/lib-rebase.sh b/t/lib-rebase.sh\nindex 99d9e7efd2d..ec6b9b107da 100644\n--- a/t/lib-rebase.sh\n+++ b/t/lib-rebase.sh\n@@ -173,10 +173,16 @@ set_reword_editor () {\n \n \twrite_script reword-editor.sh <<-EOF &&\n \t# Save the oid of the first reworded commit so we can check rebase\n-\t# fast-forwards to it\n+\t# fast-forwards to it. Also check that we do not write .git/MERGE_MSG\n+\t# when fast-forwarding\n \tif ! test -s reword-oid\n \tthen\n-\t\tgit rev-parse HEAD >reword-oid\n+\t\tgit rev-parse HEAD >reword-oid &&\n+\t\tif test -f .git/MERGE_MSG\n+\t\tthen\n+\t\t\techo 1>&2 \"error: .git/MERGE_MSG exists\"\n+\t\t\texit 1\n+\t\tfi\n \tfi &&\n \t# There should be no uncommited changes\n \tgit diff --exit-code HEAD &&\n-- \ngitgitgadget\n\n"},{"id":"433236","messageId":"b6981ea5439416f57e97575f89bbe8d2f3f38527.1629474038.git.gitgitgadget@gmail.com","threadId":"56277","inReplyTo":"pull.1015.v2.git.1629474038.gitgitgadget@gmail.com","subject":"[PATCH v2 4/4] rebase -r: fix merge -c with a merge strategy","fromName":"Phillip Wood via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-08-20T15:40:38Z","receivedAt":"2021-08-20T15:40:50Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood <phillip.wood@dunelm.org.uk>\n\nIf a rebase is started with a --strategy option other than \"ort\" or\n\"recursive\" then \"merge -c\" does not allow the user to reword the\ncommit message.\n\nSigned-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n---\n sequencer.c              |  5 ++++-\n t/t3430-rebase-merges.sh | 18 ++++++++++++++++++\n 2 files changed, 22 insertions(+), 1 deletion(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex c2cba5ed4b1..a19980f62d9 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -3934,7 +3934,10 @@ static int do_merge(struct repository *r,\n \t\t\t\tstrvec_pushf(&cmd.args,\n \t\t\t\t\t     \"-X%s\", opts->xopts[k]);\n \t\t}\n-\t\tstrvec_push(&cmd.args, \"--no-edit\");\n+\t\tif (!(flags & TODO_EDIT_MERGE_MSG))\n+\t\t\tstrvec_push(&cmd.args, \"--no-edit\");\n+\t\telse\n+\t\t\tstrvec_push(&cmd.args, \"--edit\");\n \t\tstrvec_push(&cmd.args, \"--no-ff\");\n \t\tstrvec_push(&cmd.args, \"--no-log\");\n \t\tstrvec_push(&cmd.args, \"--no-stat\");\ndiff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh\nindex 183c3a23838..43c82d9a33b 100755\n--- a/t/t3430-rebase-merges.sh\n+++ b/t/t3430-rebase-merges.sh\n@@ -187,6 +187,24 @@ test_expect_success 'merge -c commits before rewording and reloads todo-list' '\n \tcheck_reworded_commits E H\n '\n \n+test_expect_success 'merge -c rewords when a strategy is given' '\n+\tgit checkout -b merge-c-with-strategy H &&\n+\twrite_script git-merge-override <<-\\EOF &&\n+\techo overridden$1 >G.t\n+\tgit add G.t\n+\tEOF\n+\n+\tPATH=\"$PWD:$PATH\" \\\n+\tGIT_SEQUENCE_EDITOR=\"echo merge -c H G >\" \\\n+\tGIT_EDITOR=\"echo edited >>\" \\\n+\t\tgit rebase --no-ff -ir -s override -Xxopt E &&\n+\ttest_write_lines overridden--xopt >expect &&\n+\ttest_cmp expect G.t &&\n+\ttest_write_lines H \"\" edited \"\" >expect &&\n+\tgit log --format=%B -1 >actual &&\n+\ttest_cmp expect actual\n+\n+'\n test_expect_success 'with a branch tip that was cherry-picked already' '\n \tgit checkout -b already-upstream main &&\n \tbase=\"$(git rev-parse --verify HEAD)\" &&\n-- \ngitgitgadget\n"},{"id":"433548","messageId":"nycvar.QRO.7.76.6.2108241533270.55@tvgsbejvaqbjf.bet","threadId":"56277","inReplyTo":"pull.1015.v2.git.1629474038.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 0/4] rebase -r: some merge related fixes","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-08-24T13:33:53Z","receivedAt":"2021-08-24T13:34:05Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Phillip,\n\nOn Fri, 20 Aug 2021, Phillip Wood via GitGitGadget wrote:\n\n> Thanks for the feedback on V1. I have added a sentence to the commit message\n> of the third patch to make it clear that the fast-forward code is just\n> moved, not changed\n>\n> Cover letter from V1: This is a collection of merge related fixes for rebase\n> -r\n>\n>  * Make merge -c behave like reword.\n>  * When fast-forwarding a merge don't leave .git/MERGE_MSG around (reported\n>    by Gábor)\n>  * Make merge -c work when with --strategy\n>\n> Phillip Wood (4):\n>   rebase -r: make 'merge -c' behave like reword\n>   rebase -i: Add another reword test\n>   rebase -r: don't write .git/MERGE_MSG when fast-forwarding\n>   rebase -r: fix merge -c with a merge strategy\n>\n>  sequencer.c                   | 106 ++++++++++++++++++----------------\n>  t/lib-rebase.sh               |  56 ++++++++++++++++++\n>  t/t3404-rebase-interactive.sh |  13 +++++\n>  t/t3430-rebase-merges.sh      |  38 +++++++++---\n>  4 files changed, 155 insertions(+), 58 deletions(-)\n>\n>\n> base-commit: 66262451ec94d30ac4b80eb3123549cf7a788afd\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1015%2Fphillipwood%2Fwip%2Fsequencer-merge-c-v2\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1015/phillipwood/wip/sequencer-merge-c-v2\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1015\n>\n> Range-diff vs v1:\n>\n>  1:  b514dbdf928 = 1:  b514dbdf928 rebase -r: make 'merge -c' behave like reword\n>  2:  511cf9204ad = 2:  511cf9204ad rebase -i: Add another reword test\n>  3:  01d5ed4cba0 ! 3:  080e580e11c rebase -r: don't write .git/MERGE_MSG when fast-forwarding\n>      @@ Commit message\n>           is not removed and can end up seeding the message of a commit made\n>           after the rebase has finished. Avoid writing .git/MERGE_MSG when we\n>           are fast-forwarding by writing the file after the fast-forward\n>      -    checks.\n>      +    checks. Note that there are no changes to the fast-forward code, it is\n>      +    simply moved.\n\nThe result still looks fine to me.\n\nThank you,\nDscho\n\n>\n>           Note that the way this change is implemented means we no longer write\n>           the author script when fast-forwarding either. I believe this is safe\n>  4:  f2a2e3531a1 = 4:  b6981ea5439 rebase -r: fix merge -c with a merge strategy\n>\n> --\n> gitgitgadget\n>\n"}]}