diff options
author | Phillip Wood <phillip.wood@dunelm.org.uk> | 2024-04-18 15:14:09 +0200 |
---|---|---|
committer | Junio C Hamano <gitster@pobox.com> | 2024-04-18 22:33:41 +0200 |
commit | a6c2654f839d508c9c912372a9cad18340d7b59c (patch) | |
tree | 6da66beee760f60b34818ce3e564dd11b9f727b5 /sequencer.c | |
parent | sequencer: store commit message in private context (diff) | |
download | git-a6c2654f839d508c9c912372a9cad18340d7b59c.tar.xz git-a6c2654f839d508c9c912372a9cad18340d7b59c.zip |
rebase -m: fix --signoff with conflicts
When rebasing with "--signoff" the commit created by "rebase --continue"
after resolving conflicts or editing a commit fails to add the
"Signed-off-by:" trailer. This happens because the message from the
original commit is reused instead of the one that would have been used
if the sequencer had not stopped for the user interaction. The correct
message is stored in ctx->message and so with a couple of exceptions
this is written to rebase_path_message() when stopping for user
interaction instead. The exceptions are (i) "fixup" and "squash"
commands where the file is written by error_failed_squash() and (ii)
"edit" commands that are fast-forwarded where the original message is
still reused. The latter is safe because "--signoff" will never
fast-forward.
Note this introduces a change in behavior as the message file now
contains conflict comments. This is safe because commit_staged_changes()
passes an explicit cleanup flag when not editing the message and when
the message is being edited it will be cleaned up automatically. This
means user now sees the same message comments in editor with "rebase
--continue" as they would if they ran "git commit" themselves before
continuing the rebase. It also matches the behavior of "git
cherry-pick", "git merge" etc. which all list the files with merge
conflicts.
The tests are extended to check that all commits made after continuing a
rebase have a "Signed-off-by:" trailer. Sadly there are a couple of
leaks in apply.c which I've not been able to track down that mean this
test file is no-longer leak free when testing "git rebase --apply
--signoff" with conflicts.
Reported-by: David Bimmler <david.bimmler@isovalent.com>
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Diffstat (limited to 'sequencer.c')
-rw-r--r-- | sequencer.c | 23 |
1 files changed, 17 insertions, 6 deletions
diff --git a/sequencer.c b/sequencer.c index 31e4d3fdec..e29a01944b 100644 --- a/sequencer.c +++ b/sequencer.c @@ -3636,13 +3636,24 @@ static int error_with_patch(struct repository *r, struct replay_opts *opts, int exit_code, int to_amend) { - if (commit) { - if (make_patch(r, commit, opts)) + struct replay_ctx *ctx = opts->ctx; + + /* + * Write the commit message to be used by "git rebase + * --continue". If a "fixup" or "squash" command has conflicts + * then we will have already written rebase_path_message() in + * error_failed_squash(). If an "edit" command was + * fast-forwarded then we don't have a message in ctx->message + * and rely on make_patch() to write rebase_path_message() + * instead. + */ + if (ctx->have_message && !file_exists(rebase_path_message()) && + write_message(ctx->message.buf, ctx->message.len, + rebase_path_message(), 0)) + return error(_("could not write commit message file")); + + if (commit && make_patch(r, commit, opts)) return -1; - } else if (copy_file(rebase_path_message(), - git_path_merge_msg(r), 0666)) - return error(_("unable to copy '%s' to '%s'"), - git_path_merge_msg(r), rebase_path_message()); if (to_amend) { if (intend_to_amend()) |