git/sequencer.c, branch v2.45.4

Merge branch 'pw/rebase-i-error-message' into maint-2.45

2024-07-02T16:27:56Z

When the user adds to "git rebase -i" instruction to "pick" a merge commit, the error experience is not pleasant. Such an error is now caught earlier in the process that parses the todo list. * pw/rebase-i-error-message: rebase -i: improve error message when picking merge rebase -i: pass struct replay_opts to parse_insn_line()

Merge branch 'pw/rebase-m-signoff-fix' into maint-2.45

2024-06-28T22:53:06Z

"git rebase --signoff" used to forget that it needs to add a sign-off to the resulting commit when told to continue after a conflict stops its operation. * pw/rebase-m-signoff-fix: rebase -m: fix --signoff with conflicts sequencer: store commit message in private context sequencer: move current fixups to private context sequencer: start removing private fields from public API sequencer: always free "struct replay_opts"

rebase -i: improve error message when picking merge

2024-05-30T17:02:58Z

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 and printing some advice on how to fix it when the todo list is parsed rather than waiting for the "pick" command to fail. The advice recommends "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 Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano

rebase -i: pass struct replay_opts to parse_insn_line()

2024-05-30T17:02:56Z

This new parameter will be used in the next commit. As adding the parameter requires quite a few changes to plumb it through the call chain these are separated into their own commit to avoid cluttering up the next commit with incidental changes. Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano

rebase -m: fix --signoff with conflicts

2024-04-18T20:33:41Z

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 Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano

sequencer: store commit message in private context

2024-04-18T20:33:41Z

Add an strbuf to "struct replay_ctx" to hold the current commit message. This does not change the behavior but it will allow us to fix a bug with "git rebase --signoff" in the next commit. A future patch series will use the changes here to avoid writing the commit message to disc unless there are conflicts or the commit is being reworded. The changes in do_pick_commit() are a mechanical replacement of "msgbuf" with "ctx->message". In do_merge() the code to write commit message to disc is factored out of the conditional now that both branches store the message in the same buffer. Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano

sequencer: move current fixups to private context

2024-04-18T20:33:41Z

The list of current fixups is an implementation detail of the sequencer and so it should not be stored in the public options struct. Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano

sequencer: start removing private fields from public API

2024-04-18T20:33:41Z

"struct replay_opts" has a number of fields that are for internal use. While they are marked as private having them in a public struct is a distraction for callers and means that every time the internal details are changed we have to recompile all the files that include sequencer.h even though the public API is unchanged. This commit starts the process of removing the private fields by adding an opaque pointer to a "struct replay_ctx" to "struct replay_opts" and moving the "reflog_message" member to the new private struct. The sequencer currently updates the state files on disc each time it processes a command in the todo list. This is an artifact of the scripted implementation and makes the code hard to reason about as it is not possible to get a complete view of the state in memory. In the future we will add new members to "struct replay_ctx" to remedy this and avoid writing state to disc unless the sequencer stops for user interaction. Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano

sequencer: always free "struct replay_opts"

2024-04-18T20:33:41Z

sequencer_post_commit_cleanup() initializes an instance of "struct replay_opts" but does not call replay_opts_release(). Currently this does not leak memory because the code paths called don't allocate any of the struct members. That will change in the next commit so add call to replay_opts_release() to prevent a memory leak in "git commit" that breaks all of the leak free tests. Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano

Merge branch 'jk/core-comment-string'

2024-04-05T17:49:49Z

core.commentChar used to be limited to a single byte, but has been updated to allow an arbitrary multi-byte sequence. * jk/core-comment-string: config: add core.commentString config: allow multi-byte core.commentChar environment: drop comment_line_char compatibility macro wt-status: drop custom comment-char stringification sequencer: handle multi-byte comment characters when writing todo list find multi-byte comment chars in unterminated buffers find multi-byte comment chars in NUL-terminated strings prefer comment_line_str to comment_line_char for printing strbuf: accept a comment string for strbuf_add_commented_lines() strbuf: accept a comment string for strbuf_commented_addf() strbuf: accept a comment string for strbuf_stripspace() environment: store comment_line_char as a string strbuf: avoid shadowing global comment_line_char name commit: refactor base-case of adjust_comment_line_char() strbuf: avoid static variables in strbuf_add_commented_lines() strbuf: simplify comment-handling in add_lines() helper config: forbid newline as core.commentChar