diff --git a/sequencer.c b/sequencer.c index 87cee5c6fe..291d1da4ea 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1946,6 +1946,13 @@ static int seen_squash(struct replay_ctx *ctx) strstr(ctx->current_fixups.buf, "\nsquash"); } +/* Does the current fixup chain contain a "fixup -c" command? */ +static int seen_fixup_edit_msg(struct replay_ctx *ctx) +{ + return starts_with(ctx->current_fixups.buf, "fixup -c") || + strstr(ctx->current_fixups.buf, "\nfixup -c"); +} + static void update_comment_bufs(struct strbuf *buf1, struct strbuf *buf2, int n) { strbuf_setlen(buf1, strlen(comment_line_str) + 1); @@ -2156,9 +2163,14 @@ static int update_squash_messages(struct repository *r, strbuf_release(&buf); if (!res) { - strbuf_addf(&ctx->current_fixups, "%s%s %s", + const char *fixup_flag = ""; + + if (is_fixup_flag(command, flag) && (flag & TODO_EDIT_FIXUP_MSG)) + fixup_flag = " -c"; + + strbuf_addf(&ctx->current_fixups, "%s%s%s %s", ctx->current_fixups.len ? "\n" : "", - command_to_string(command), + command_to_string(command), fixup_flag, oid_to_hex(&commit->object.oid)); res = write_message(ctx->current_fixups.buf, ctx->current_fixups.len, @@ -3319,7 +3331,13 @@ static int read_populate_opts(struct replay_opts *opts) const char *p = ctx->current_fixups.buf; ctx->current_fixup_count = 1; while ((p = strchr(p, '\n'))) { - ctx->current_fixup_count++; + /* + * Older versions of git accidentally + * inserted blank lines when a fixup + * was skipped. + */ + if (p[1] != '\n') + ctx->current_fixup_count++; p++; } } @@ -5414,6 +5432,9 @@ static int commit_staged_changes(struct repository *r, BUG("Incorrect current_fixups:\n%s", p); while (len && p[len - 1] != '\n') len--; + /* Remove trailing newline */ + if (len) + len--; strbuf_setlen(&ctx->current_fixups, len); if (write_message(p, len, rebase_path_current_fixups(), 0) < 0) { @@ -5442,8 +5463,8 @@ static int commit_staged_changes(struct repository *r, * message, no need to bother the user with * opening the commit message in the editor. */ - if (!starts_with(p, "squash ") && - !strstr(p, "\nsquash ")) + if (!seen_squash(ctx) && + !seen_fixup_edit_msg(ctx)) flags = (flags & ~EDIT_MSG) | CLEANUP_MSG; } else if (is_fixup(peek_command(todo_list, 0))) { /* diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh index 03e0714864..cb5c3a1cb5 100755 --- a/t/t3418-rebase-continue.sh +++ b/t/t3418-rebase-continue.sh @@ -134,6 +134,7 @@ test_expect_success '--skip after failed fixup cleans commit message' ' EOF : skip and continue && + test_config commit.status false && echo "cp \"\$1\" .git/copy.txt" | write_script copy-editor.sh && (test_set_editor "$PWD/copy-editor.sh" && git rebase --skip) && @@ -145,7 +146,8 @@ test_expect_success '--skip after failed fixup cleans commit message' ' : now, let us ensure that "squash" is handled correctly && git reset --hard wants-fixup-3 && - test_must_fail env FAKE_LINES="1 squash 2 squash 1 squash 3 squash 1" \ + test_must_fail env \ + FAKE_LINES="1 squash 2 squash 1 squash 3 squash 1 squash 4 squash 1" \ git rebase -i HEAD~4 && : the second squash failed, but there are two more in the chain && @@ -171,6 +173,32 @@ test_expect_success '--skip after failed fixup cleans commit message' ' fixup 2 EOF + (test_set_editor "$PWD/copy-editor.sh" && + test_must_fail git rebase --skip) && + : not the final squash, no need to edit the commit message && + test_path_is_missing .git/copy.txt && + + : The first, third and fifth squashes succeeded, therefore: && + cat >expect <<-\EOF && + # This is a combination of 4 commits. + # This is the 1st commit message: + + wants-fixup + + # This is the commit message #2: + + fixup 1 + + # This is the commit message #3: + + fixup 2 + + # This is the commit message #4: + + fixup 3 + EOF + test_commit_message HEAD expect && + (test_set_editor "$PWD/copy-editor.sh" && git rebase --skip) && test_commit_message HEAD <<-\EOF && wants-fixup @@ -178,12 +206,12 @@ test_expect_success '--skip after failed fixup cleans commit message' ' fixup 1 fixup 2 + + fixup 3 EOF : Final squash failed, but there was still a squash && - head -n1 .git/copy.txt >first-line && - test_grep "# This is a combination of 3 commits" first-line && - test_grep "# This is the commit message #3:" .git/copy.txt + test_cmp expect .git/copy.txt ' test_expect_success 'setup rerere database' ' diff --git a/t/t3437-rebase-fixup-options.sh b/t/t3437-rebase-fixup-options.sh index 5d306a4769..a4b2a63165 100755 --- a/t/t3437-rebase-fixup-options.sh +++ b/t/t3437-rebase-fixup-options.sh @@ -186,6 +186,53 @@ test_expect_success 'multiple fixup -c opens editor once' ' test_commit_message HEAD expected-message ' +test_expect_success 'fixup -c is remembered after skipping final fixup' ' + test_when_finished "test_might_fail git rebase --abort" && + cat >todo <<-\EOF && + pick B + fixup -c A1 + fixup A3 + EOF + ( + set_fake_editor && + set_replace_editor todo && + test_must_fail git rebase -i A A && + git show && cat .git/rebase-merge/message-squash && + FAKE_COMMIT_AMEND=edited git rebase --skip + ) && + test_commit_message HEAD <<-\EOF + new subject + + new + body + + edited + EOF +' +test_expect_success 'fixup -c is remembered after skipping later fixup' ' + test_when_finished "test_might_fail git rebase --abort" && + cat >todo <<-\EOF && + pick B + fixup -c A1 + fixup A3 + fixup A2 + EOF + ( + set_fake_editor && + set_replace_editor todo && + test_must_fail git rebase -i A A && + FAKE_COMMIT_AMEND=edited git rebase --skip + ) && + test_commit_message HEAD <<-\EOF + new subject + + new + body + + edited + EOF +' + test_expect_success 'sequence squash, fixup & fixup -c gives combined message' ' test_when_finished "test_might_fail git rebase --abort" && git checkout --detach A3 &&