Skip to content

Commit fce7789

Browse files
committed
Move conflicted commit detection from headers to commit message markers
The `gitbutler-conflicted` header is stripped when commits are rebased outside of GitButler, silently losing conflict state. Replace it with commit message markers — a `[conflict]` prefix for fast detection and a `GitButler-Conflict: true` footer for verification — which survive rebasing and are visible to developers in `git log`. New commits only write message markers (no header). Reading checks the message first, then falls back to the header for backward compatibility with existing conflicted commits for a release or two. Conflicted commit message format: [conflict] <original subject> <original body> GitButler-Conflict: true This is a GitButler-managed conflicted commit. Files are auto-resolved using the "ours" side. The commit tree contains additional directories: .conflict-side-0 — our tree .conflict-side-1 — their tree .conflict-base-0 — the merge base tree .auto-resolution — the auto-resolved tree .conflict-files — metadata about conflicted files To manually resolve, check out this commit, remove the directories listed above, resolve the conflicts, and amend the commit. Markers are added when creating conflicted commits and stripped when conflicts are resolved (either through the edit-mode flow or when a previously-conflicted commit rebases cleanly). Reword and squash operations preserve the markers on still-conflicted commits. The UI layer strips markers before displaying commit messages, since it already has a dedicated `has_conflicts` flag. The CONFLICT-README.txt tree blob is removed since the footer now serves the same purpose with better visibility. The conflicted file count (`Option<u64>`) stored in the old header was only ever used as a boolean — no code path read the actual count — so the new approach uses a simple presence check with no count.
1 parent d307086 commit fce7789

17 files changed

Lines changed: 250 additions & 108 deletions

File tree

crates/but-core/src/commit.rs

Lines changed: 80 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -505,8 +505,12 @@ impl<'repo> Commit<'repo> {
505505
}
506506

507507
/// Return `true` if this commit contains a tree that is conflicted.
508+
///
509+
/// Checks the commit message for conflict markers first (new style),
510+
/// then falls back to the `gitbutler-conflicted` header (legacy).
508511
pub fn is_conflicted(&self) -> bool {
509-
self.headers().is_some_and(|hdr| hdr.is_conflicted())
512+
message_is_conflicted(self.inner.message.as_ref())
513+
|| self.headers().is_some_and(|hdr| hdr.is_conflicted())
510514
}
511515

512516
/// If the commit is conflicted, then it returns the auto-resolution tree,
@@ -627,3 +631,78 @@ impl ConflictEntries {
627631
set.len()
628632
}
629633
}
634+
635+
/// The prefix prepended to the commit subject line to mark a conflicted commit.
636+
pub const CONFLICT_MESSAGE_PREFIX: &str = "[conflict] ";
637+
638+
/// The marker line in the commit message footer that confirms a GitButler-managed conflict.
639+
pub const CONFLICT_FOOTER_MARKER: &str = "GitButler-Conflict: true";
640+
641+
/// The full footer appended to a conflicted commit message, explaining the conflict state.
642+
const CONFLICT_FOOTER: &str = "\
643+
GitButler-Conflict: true
644+
This is a GitButler-managed conflicted commit. Files are auto-resolved
645+
using the \"ours\" side. The commit tree contains additional directories:
646+
.conflict-side-0 — our tree
647+
.conflict-side-1 — their tree
648+
.conflict-base-0 — the merge base tree
649+
.auto-resolution — the auto-resolved tree
650+
.conflict-files — metadata about conflicted files
651+
To manually resolve, check out this commit, remove the directories
652+
listed above, resolve the conflicts, and amend the commit.";
653+
654+
/// Add conflict markers (prefix and footer) to a commit message.
655+
///
656+
/// Uses byte operations to preserve non-UTF8 content in the original message.
657+
pub fn add_conflict_markers(message: &BStr) -> BString {
658+
let trimmed = message
659+
.as_bytes()
660+
.strip_suffix(b"\n")
661+
.unwrap_or(message.as_bytes());
662+
let footer_block = format!("\n\n{CONFLICT_FOOTER}\n");
663+
let mut result = BString::from(Vec::with_capacity(
664+
CONFLICT_MESSAGE_PREFIX.len() + trimmed.len() + footer_block.len(),
665+
));
666+
result.extend_from_slice(CONFLICT_MESSAGE_PREFIX.as_bytes());
667+
result.extend_from_slice(trimmed);
668+
result.extend_from_slice(footer_block.as_bytes());
669+
result
670+
}
671+
672+
/// The exact footer block suffix appended by [`add_conflict_markers`], used for
673+
/// suffix-based matching to avoid false positives from partial matches in the
674+
/// commit body.
675+
fn conflict_footer_block() -> String {
676+
format!("\n\n{CONFLICT_FOOTER}\n")
677+
}
678+
679+
/// Strip conflict markers (prefix and footer) from a commit message, restoring
680+
/// the original message. Returns the message unchanged if no markers are found.
681+
///
682+
/// Only strips when the exact footer block produced by [`add_conflict_markers`]
683+
/// is present as a suffix, so arbitrary occurrences of the marker text inside
684+
/// the commit body cannot cause false stripping.
685+
pub fn strip_conflict_markers(message: &BStr) -> BString {
686+
let bytes = message.as_bytes();
687+
let footer = conflict_footer_block();
688+
689+
let without_footer = bytes.strip_suffix(footer.as_bytes()).unwrap_or(bytes);
690+
691+
let without_prefix = without_footer
692+
.strip_prefix(CONFLICT_MESSAGE_PREFIX.as_bytes())
693+
.unwrap_or(without_footer);
694+
695+
BString::from(without_prefix)
696+
}
697+
698+
/// Check if a commit message contains GitButler conflict markers.
699+
///
700+
/// Requires both the `[conflict] ` prefix and the exact footer block appended
701+
/// by [`add_conflict_markers`] as a suffix, so arbitrary commits with
702+
/// `[conflict]` in their subject or `GitButler-Conflict: true` in the body
703+
/// are not misidentified.
704+
pub fn message_is_conflicted(message: &BStr) -> bool {
705+
let bytes = message.as_bytes();
706+
let footer = conflict_footer_block();
707+
bytes.starts_with(CONFLICT_MESSAGE_PREFIX.as_bytes()) && bytes.ends_with(footer.as_bytes())
708+
}

crates/but-core/tests/core/branch/find_unique_refname.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,9 +74,9 @@ fn registered_remotes_help_deal_with_slashed_remote_names() -> anyhow::Result<()
7474
let repo = read_only_in_memory_scenario("multiple-remotes-with-tracking-branches")?;
7575
insta::assert_snapshot!(visualize_commit_graph_all(&repo)?,
7676
@"
77-
* 14f7926 (nested/remote-b/main, nested/remote-b/in-nested-remote-b, nested/remote-b/HEAD) init-c
78-
* 5efb67f (nested/remote/main, nested/remote/in-nested-remote, nested/remote/HEAD) init-b
79-
* 263500f (origin/normal-remote, origin/main, origin/HEAD) init-a
77+
* 14f7926 (nested/remote-b/main, nested/remote-b/in-nested-remote-b) init-c
78+
* 5efb67f (nested/remote/main, nested/remote/in-nested-remote) init-b
79+
* 263500f (origin/normal-remote, origin/main) init-a
8080
");
8181

8282
let unique = find_unique_refname(&repo, "refs/heads/in-nested-remote".try_into()?)?;

crates/but-core/tests/core/extract_remote_name_and_short_name.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@ fn journey() -> anyhow::Result<()> {
66
let repo = read_only_in_memory_scenario("multiple-remotes-with-tracking-branches")?;
77
insta::assert_snapshot!(visualize_commit_graph_all(&repo)?,
88
@"
9-
* 14f7926 (nested/remote-b/main, nested/remote-b/in-nested-remote-b, nested/remote-b/HEAD) init-c
10-
* 5efb67f (nested/remote/main, nested/remote/in-nested-remote, nested/remote/HEAD) init-b
11-
* 263500f (origin/normal-remote, origin/main, origin/HEAD) init-a
9+
* 14f7926 (nested/remote-b/main, nested/remote-b/in-nested-remote-b) init-c
10+
* 5efb67f (nested/remote/main, nested/remote/in-nested-remote) init-b
11+
* 263500f (origin/normal-remote, origin/main) init-a
1212
");
1313

1414
let remote_names = repo.remote_names();

crates/but-rebase/src/cherry_pick.rs

Lines changed: 14 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ pub enum EmptyCommit {
2323
}
2424

2525
pub(crate) mod function {
26-
use std::{collections::HashSet, path::PathBuf};
26+
use std::path::PathBuf;
2727

2828
use anyhow::{Context as _, bail};
2929
use bstr::BString;
@@ -168,18 +168,22 @@ pub(crate) mod function {
168168
}
169169

170170
let headers = to_rebase.headers();
171-
let to_rebase_is_conflicted = headers.as_ref().is_some_and(|hdr| hdr.is_conflicted());
171+
let was_conflicted = to_rebase.is_conflicted();
172172
let mut new_commit = to_rebase.inner;
173173
new_commit.tree = resolved_tree_id.detach();
174174

175175
// Assure the commit isn't thinking it's conflicted.
176-
if to_rebase_is_conflicted {
176+
if was_conflicted {
177+
// Strip the legacy header if present
177178
if let Some(pos) = new_commit
178179
.extra_headers()
179180
.find_pos(HEADERS_CONFLICTED_FIELD)
180181
{
181182
new_commit.extra_headers.remove(pos);
182183
}
184+
// Strip conflict markers from the commit message
185+
new_commit.message =
186+
but_core::commit::strip_conflict_markers(new_commit.message.as_ref());
183187
} else if headers.is_none() {
184188
new_commit
185189
.extra_headers
@@ -205,10 +209,6 @@ pub(crate) mod function {
205209
treat_as_unresolved: gix::merge::tree::TreatAsUnresolved,
206210
) -> anyhow::Result<gix::Id<'repo>> {
207211
let repo = resolved_tree_id.repo;
208-
// in case someone checks this out with vanilla Git, we should warn why it looks like this
209-
let readme_content =
210-
b"You have checked out a GitButler Conflicted commit. You probably didn't mean to do this.";
211-
let readme_blob = repo.write_blob(readme_content)?;
212212

213213
let conflicted_files =
214214
extract_conflicted_files(resolved_tree_id, cherry_pick, treat_as_unresolved)?;
@@ -243,15 +243,20 @@ pub(crate) mod function {
243243
resolved_tree_id,
244244
)?;
245245
tree.upsert(".conflict-files", EntryKind::Blob, conflicted_files_blob)?;
246-
tree.upsert("CONFLICT-README.txt", EntryKind::Blob, readme_blob)?;
247246

248247
let mut headers = to_rebase
249248
.headers()
250249
.unwrap_or_else(|| Headers::from_config(&repo.config_snapshot()));
251-
headers.conflicted = conflicted_files.conflicted_header_field();
250+
headers.conflicted = None;
252251
to_rebase.tree = tree.write().context("failed to write tree")?.detach();
253252
set_parent(&mut to_rebase, head.id.detach())?;
254253

254+
// Add conflict markers to the commit message
255+
if !but_core::commit::message_is_conflicted(to_rebase.inner.message.as_ref()) {
256+
to_rebase.inner.message =
257+
but_core::commit::add_conflict_markers(to_rebase.inner.message.as_ref());
258+
}
259+
255260
to_rebase.set_headers(&headers);
256261
Ok(crate::commit::create(
257262
repo,
@@ -342,22 +347,5 @@ pub(crate) mod function {
342347
|| !self.our_entries.is_empty()
343348
|| !self.their_entries.is_empty()
344349
}
345-
346-
fn total_entries(&self) -> usize {
347-
let set = self
348-
.ancestor_entries
349-
.iter()
350-
.chain(self.our_entries.iter())
351-
.chain(self.their_entries.iter())
352-
.collect::<HashSet<_>>();
353-
354-
set.len()
355-
}
356-
357-
/// Return the `conflicted` header field value.
358-
pub(crate) fn conflicted_header_field(&self) -> Option<u64> {
359-
let entries = self.total_entries();
360-
Some(if entries > 0 { entries as u64 } else { 1 })
361-
}
362350
}
363351
}

crates/but-rebase/src/graph_rebase/cherry_pick.rs

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -305,18 +305,21 @@ fn commit_from_unconflicted_tree<'repo>(
305305
let repo = to_rebase.id.repo;
306306

307307
let headers = to_rebase.headers();
308-
let to_rebase_is_conflicted = headers.as_ref().is_some_and(|hdr| hdr.is_conflicted());
308+
let was_conflicted = to_rebase.is_conflicted();
309309
let mut new_commit = to_rebase.inner;
310310
new_commit.tree = resolved_tree_id.detach();
311311

312312
// Ensure the commit isn't thinking it's conflicted.
313-
if to_rebase_is_conflicted {
313+
if was_conflicted {
314+
// Strip the legacy header if present
314315
if let Some(pos) = new_commit
315316
.extra_headers()
316317
.find_pos(HEADERS_CONFLICTED_FIELD)
317318
{
318319
new_commit.extra_headers.remove(pos);
319320
}
321+
// Strip conflict markers from the commit message
322+
new_commit.message = but_core::commit::strip_conflict_markers(new_commit.message.as_ref());
320323
} else if headers.is_none() {
321324
new_commit
322325
.extra_headers
@@ -348,10 +351,6 @@ fn commit_from_conflicted_tree<'repo>(
348351
sign_commit: SignCommit,
349352
) -> anyhow::Result<gix::Id<'repo>> {
350353
let repo = resolved_tree_id.repo;
351-
// in case someone checks this out with vanilla Git, we should warn why it looks like this
352-
let readme_content =
353-
b"You have checked out a GitButler Conflicted commit. You probably didn't mean to do this.";
354-
let readme_blob = repo.write_blob(readme_content)?;
355354

356355
let conflicted_files =
357356
extract_conflicted_files(resolved_tree_id, cherry_pick, treat_as_unresolved)?;
@@ -383,15 +382,20 @@ fn commit_from_conflicted_tree<'repo>(
383382
resolved_tree_id,
384383
)?;
385384
tree.upsert(".conflict-files", EntryKind::Blob, conflicted_files_blob)?;
386-
tree.upsert("CONFLICT-README.txt", EntryKind::Blob, readme_blob)?;
387385

388386
let mut headers = to_rebase
389387
.headers()
390388
.unwrap_or_else(|| Headers::from_config(&repo.config_snapshot()));
391-
headers.conflicted = conflicted_files.conflicted_header_field();
389+
headers.conflicted = None;
392390
to_rebase.tree = tree.write().context("failed to write tree")?.detach();
393391
to_rebase.parents = parents.into();
394392

393+
// Add conflict markers to the commit message
394+
if !but_core::commit::message_is_conflicted(to_rebase.inner.message.as_ref()) {
395+
to_rebase.inner.message =
396+
but_core::commit::add_conflict_markers(to_rebase.inner.message.as_ref());
397+
}
398+
395399
to_rebase.set_headers(&headers);
396400
Ok(crate::commit::create(
397401
repo,

crates/but-rebase/src/lib.rs

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,13 @@ fn rebase(
328328
let mut new_commit = repo.find_commit(new_commit)?.decode()?.to_owned()?;
329329
new_commit.parents = base_commit.parent_ids().map(|id| id.detach()).collect();
330330
if let Some(new_message) = new_message {
331-
new_commit.message = new_message;
331+
// Preserve conflict markers if the commit is conflicted
332+
new_commit.message =
333+
if but_core::commit::message_is_conflicted(new_commit.message.as_ref()) {
334+
but_core::commit::add_conflict_markers(new_message.as_ref())
335+
} else {
336+
new_message
337+
};
332338
}
333339
*cursor = commit::create(
334340
repo,
@@ -374,7 +380,13 @@ fn reword_commit(
374380
new_message: BString,
375381
) -> Result<gix::ObjectId> {
376382
let mut new_commit = repo.find_commit(oid)?.decode()?.to_owned()?;
377-
new_commit.message = new_message;
383+
// Preserve conflict markers if the commit was conflicted
384+
let was_conflicted = but_core::commit::message_is_conflicted(new_commit.message.as_ref());
385+
new_commit.message = if was_conflicted {
386+
but_core::commit::add_conflict_markers(new_message.as_ref())
387+
} else {
388+
new_message
389+
};
378390
Ok(commit::create(
379391
repo,
380392
new_commit,

0 commit comments

Comments
 (0)