diff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt index 0e35ae5240..fa61241756 100644 --- a/Documentation/config/advice.txt +++ b/Documentation/config/advice.txt @@ -96,6 +96,8 @@ advice.*:: `pushNonFFCurrent`, `pushNonFFMatching`, `pushAlreadyExists`, `pushFetchFirst`, `pushNeedsForce`, and `pushRefNeedsUpdate` simultaneously. + rebaseTodoError:: + Shown when there is an error after editing the rebase todo list. refSyntax:: Shown when the user provides an illegal ref name, to tell the user about the ref syntax documentation. diff --git a/advice.c b/advice.c index d19648b7f8..6546231e91 100644 --- a/advice.c +++ b/advice.c @@ -69,6 +69,7 @@ static struct { [ADVICE_PUSH_UNQUALIFIED_REF_NAME] = { "pushUnqualifiedRefName" }, [ADVICE_PUSH_UPDATE_REJECTED] = { "pushUpdateRejected" }, [ADVICE_PUSH_UPDATE_REJECTED_ALIAS] = { "pushNonFastForward" }, /* backwards compatibility */ + [ADVICE_REBASE_TODO_ERROR] = { "rebaseTodoError" }, [ADVICE_REF_SYNTAX] = { "refSyntax" }, [ADVICE_RESET_NO_REFRESH_WARNING] = { "resetNoRefresh" }, [ADVICE_RESOLVE_CONFLICT] = { "resolveConflict" }, diff --git a/advice.h b/advice.h index c8d29f97f3..5105d90129 100644 --- a/advice.h +++ b/advice.h @@ -37,6 +37,7 @@ enum advice_type { ADVICE_PUSH_UNQUALIFIED_REF_NAME, ADVICE_PUSH_UPDATE_REJECTED, ADVICE_PUSH_UPDATE_REJECTED_ALIAS, + ADVICE_REBASE_TODO_ERROR, ADVICE_REF_SYNTAX, ADVICE_RESET_NO_REFRESH_WARNING, ADVICE_RESOLVE_CONFLICT, diff --git a/sequencer.c b/sequencer.c index a3154ba334..0959f335f7 100644 --- a/sequencer.c +++ b/sequencer.c @@ -2573,7 +2573,61 @@ 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 check_merge_commit_insn(enum todo_command command) +{ + switch(command) { + case TODO_PICK: + error(_("'%s' does not accept merge commits"), + todo_command_info[command].str); + advise_if_enabled(ADVICE_REBASE_TODO_ERROR, _( + /* + * TRANSLATORS: 'pick' and 'merge -C' should not be + * translated. + */ + "'pick' does not take a merge commit. If you wanted to\n" + "replay the merge, use 'merge -C' on the commit.")); + return -1; + + case TODO_REWORD: + error(_("'%s' does not accept merge commits"), + todo_command_info[command].str); + advise_if_enabled(ADVICE_REBASE_TODO_ERROR, _( + /* + * TRANSLATORS: 'reword' and 'merge -c' should not be + * translated. + */ + "'reword' does not take a merge commit. If you wanted to\n" + "replay the merge and reword the commit message, use\n" + "'merge -c' on the commit")); + return -1; + + case TODO_EDIT: + error(_("'%s' does not accept merge commits"), + todo_command_info[command].str); + advise_if_enabled(ADVICE_REBASE_TODO_ERROR, _( + /* + * TRANSLATORS: 'edit', 'merge -C' and 'break' should + * not be translated. + */ + "'edit' does not take a merge commit. If you wanted to\n" + "replay the merge, use 'merge -C' on the commit, and then\n" + "'break' to give the control back to you so that you can\n" + "do 'git commit --amend && git rebase --continue'.")); + return -1; + + case TODO_FIXUP: + case TODO_SQUASH: + return error(_("cannot squash merge commit into another commit")); + + case TODO_MERGE: + return 0; + + 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 +2733,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->commit->parents && item->commit->parents->next) + return check_merge_commit_insn(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 d1bead61fa..f92baad138 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -2215,6 +2215,51 @@ 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 + hint: ${SQ}pick${SQ} does not take a merge commit. If you wanted to + hint: replay the merge, use ${SQ}merge -C${SQ} on the commit. + hint: Disable this message with "git config advice.rebaseTodoError false" + error: invalid line 1: pick $oid + error: ${SQ}reword${SQ} does not accept merge commits + hint: ${SQ}reword${SQ} does not take a merge commit. If you wanted to + hint: replay the merge and reword the commit message, use + hint: ${SQ}merge -c${SQ} on the commit + hint: Disable this message with "git config advice.rebaseTodoError false" + error: invalid line 2: reword $oid + error: ${SQ}edit${SQ} does not accept merge commits + hint: ${SQ}edit${SQ} does not take a merge commit. If you wanted to + hint: replay the merge, use ${SQ}merge -C${SQ} on the commit, and then + hint: ${SQ}break${SQ} to give the control back to you so that you can + hint: do ${SQ}git commit --amend && git rebase --continue${SQ}. + hint: Disable this message with "git config advice.rebaseTodoError false" + 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