Hi Ayush
On 26/06/2025 23:16, Ayush Chandekar wrote:
When core.commentChar is set to "auto", Git selects a comment character
by scanning the commit message contents and avoiding any character
already present in the message.
If the message still contains old conflict comments (starting with a
comment character), Git assumes that character is in use and chooses a
different one. As a result, those existing comment lines are no longer
recognized as comments and end up being included in the final commit
message.
To avoid this, skip scanning the trailing comment block when selecting
the comment character. This allows Git to safely reuse the original
character when appropriate, keeping the commit message clean and free of
leftover conflict information.
This is a good explanation of the problem. Maybe this is another reason
to consider removing support for commentChar=auto [1]
[1] https://lore.kernel.org/git/xmqqa59i45wc.fsf@gitster.g/
Background:
The "auto" value for core.commentchar was introduced in the commit
84c9dc2c5a (commit: allow core.commentChar=auto for character auto
selection, 2014-05-17) but did not exhibt this issue at that time.
The bug was introduced in commit a6c2654f83 (rebase -m: fix --signoff
with conflicts, 2024-04-18) where Git started writing conflict comments
to the file at 'rebase_path_message()'.
Mentored-by: Christian Couder <christian.couder@xxxxxxxxx>
Mentored-by: Ghanshyam Thakkar <shyamthakkar001@xxxxxxxxx>
Signed-off-by: Ayush Chandekar <ayu.chandekar@xxxxxxxxx>
---
Thanks to Christian for mentoring, and to Kristopher and Junio for their reviews!
builtin/commit.c | 6 +++++-
t/t3418-rebase-continue.sh | 14 ++++++++++++++
2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index fba0dded64..63e7158e98 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -688,6 +688,10 @@ static void adjust_comment_line_char(const struct strbuf *sb)
char candidates[] = "#;@!$%^&|:";
char *candidate;
const char *p;
+ size_t cutoff;
+
+ /* Ignore comment chars in trailing comments (e.g., Conflicts:) */
+ cutoff = sb->len - ignored_log_message_bytes(sb->buf, sb->len);
This finds the "Conflicts:" line. I was surprised to see that the string
it looks for is hard coded and not translated, however the sequencer
(also surprisingly) does not translate that message either so it should
work.
if (!memchr(sb->buf, candidates[0], sb->len)) {
free(comment_line_str_to_free);
@@ -700,7 +704,7 @@ static void adjust_comment_line_char(const struct strbuf *sb)
candidate = strchr(candidates, *p);
if (candidate)
*candidate = ' ';
- for (p = sb->buf; *p; p++) {
+ for (p = sb->buf; p + 1 < sb->buf + cutoff; p++) {
if ((p[0] == '\n' || p[0] == '\r') && p[1]) {
candidate = strchr(candidates, p[1]);
if (candidate)
diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh
index 127216f722..ccfe77af6c 100755
--- a/t/t3418-rebase-continue.sh
+++ b/t/t3418-rebase-continue.sh
@@ -328,6 +328,20 @@ test_expect_success 'there is no --no-reschedule-failed-exec in an ongoing rebas
test_expect_code 129 git rebase --edit-todo --no-reschedule-failed-exec
'
+test_expect_success 'no change in comment character due to conflicts markers with core.commentChar=auto' '
+ test_commit base file &&
If you used an existing file (F1 or F2) like most of the rest of the
tests in this file we could avoid creating this commit and save
ourselves a couple of processes.
+ git checkout -b branch-a &&
+ test_commit A file &&
+ git checkout -b branch-b base &&
+ test_commit B file &&
+ test_must_fail git rebase branch-a &&
+ printf "B\nA\n" >file &&
+ git add file &&
+ GIT_EDITOR="cat >actual" git -c core.commentChar=auto rebase --continue &&
+ # Check that "#" is still the comment character.
+ test_grep "^# Changes to be committed:$" actual
I agree that it is a good idea to anchor the start of the message, but
I'm not sure it is helpful to anchor the end of the message as we don't
want the test to fail just because an unrelated change adds some
whitespace to the end of this line. I'd be tempted to drop the ':' for
the same reason.
Thanks for fixing this
Phillip
+'
+
test_orig_head_helper () {
test_when_finished 'git rebase --abort &&
git checkout topic &&
