From: Phillip Wood <phillip.wood@xxxxxxxxxxxxx>
The only todo commands that accept a merge commit are "merge" and
"reset". All the other commands like "pick" or "reword" fail when they
try to pick a a merge commit and print the message
error: commit abc123 is a merge but no -m option was given.
followed by a hint about the command being rescheduled. This message is
designed to help the user when they cherry-pick a merge and forget to
pass "-m". For users who are rebasing the message is confusing as there
is no way for rebase to cherry-pick the merge.
Improve the user experience by detecting the error when the todo list is
parsed rather than waiting for the "pick" command to fail and print a
message recommending the "merge" command instead. We recommend "merge"
rather than "exec git cherry-pick -m ..." on the assumption that
cherry-picking merges is relatively rare and it is more likely that the
user chose "pick" by a mistake.
It would be possible to support cherry-picking merges by allowing the
user to pass "-m" to "pick" commands but that adds complexity to do
something that can already be achieved with
exec git cherry-pick -m1 abc123
Reported-by: Stefan Haller <lists@xxxxxxxxxxxxxxxx>
Signed-off-by: Phillip Wood <phillip.wood@xxxxxxxxxxxxx>
---
sequencer.c | 37 +++++++++++++++++++++++++++++++++--
t/t3404-rebase-interactive.sh | 33 +++++++++++++++++++++++++++++++
2 files changed, 68 insertions(+), 2 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index a3154ba3347..4012c6f88d9 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2573,7 +2573,35 @@ static int check_label_or_ref_arg(enum todo_command command, const char *arg)
return 0;
}
-static int parse_insn_line(struct repository *r, struct replay_opts *opts UNUSED,
+static int error_merge_commit(enum todo_command command)
+{
+ switch(command) {
+ case TODO_PICK:
+ return error(_("'%s' does not accept merge commits, "
+ "please use '%s'"),
+ todo_command_info[command].str, "merge -C");
+
+ case TODO_REWORD:
+ return error(_("'%s' does not accept merge commits, "
+ "please use '%s'"),
+ todo_command_info[command].str, "merge -c");
+
+ case TODO_EDIT:
+ return error(_("'%s' does not accept merge commits, "
+ "please use '%s' followed by '%s'"),
+ todo_command_info[command].str,
+ "merge -C", "break");
+
+ case TODO_FIXUP:
+ case TODO_SQUASH:
+ return error(_("cannot squash merge commit into another commit"));
+
+ default:
+ BUG("unexpected todo_command");
+ }
+}
+
+static int parse_insn_line(struct repository *r, struct replay_opts *opts,
struct todo_item *item, const char *buf,
const char *bol, char *eol)
{
@@ -2679,7 +2707,12 @@ static int parse_insn_line(struct repository *r, struct replay_opts *opts UNUSED
return status;
item->commit = lookup_commit_reference(r, &commit_oid);
- return item->commit ? 0 : -1;
+ if (!item->commit)
+ return -1;
+ if (is_rebase_i(opts) && item->command != TODO_MERGE &&
+ item->commit->parents && item->commit->parents->next)
+ return error_merge_commit(item->command);
+ return 0;
}
int sequencer_get_last_command(struct repository *r UNUSED, enum replay_action *action)
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index d1bead61fad..8565fd4b5ae 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -2215,6 +2215,39 @@ test_expect_success 'bad labels and refs rejected when parsing todo list' '
test_path_is_missing execed
'
+test_expect_success 'non-merge commands reject merge commits' '
+ test_when_finished "test_might_fail git rebase --abort" &&
+ git checkout E &&
+ git merge I &&
+ oid=$(git rev-parse HEAD) &&
+ cat >todo <<-EOF &&
+ pick $oid
+ reword $oid
+ edit $oid
+ fixup $oid
+ squash $oid
+ EOF
+ (
+ set_replace_editor todo &&
+ test_must_fail git rebase -i HEAD 2>actual
+ ) &&
+ cat >expect <<-EOF &&
+ error: ${SQ}pick${SQ} does not accept merge commits, please use ${SQ}merge -C${SQ}
+ error: invalid line 1: pick $oid
+ error: ${SQ}reword${SQ} does not accept merge commits, please use ${SQ}merge -c${SQ}
+ error: invalid line 2: reword $oid
+ error: ${SQ}edit${SQ} does not accept merge commits, please use ${SQ}merge -C${SQ} followed by ${SQ}break${SQ}
+ error: invalid line 3: edit $oid
+ error: cannot squash merge commit into another commit
+ error: invalid line 4: fixup $oid
+ error: cannot squash merge commit into another commit
+ error: invalid line 5: squash $oid
+ You can fix this with ${SQ}git rebase --edit-todo${SQ} and then run ${SQ}git rebase --continue${SQ}.
+ Or you can abort the rebase with ${SQ}git rebase --abort${SQ}.
+ EOF
+ test_cmp expect actual
+'
+
# This must be the last test in this file
test_expect_success '$EDITOR and friends are unchanged' '
test_editor_unchanged
--
gitgitgadget
