Skip to content

Commit 632c2c1

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 632c2c1

18 files changed

Lines changed: 280 additions & 136 deletions

File tree

crates/but-core/src/commit.rs

Lines changed: 84 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,82 @@ 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+
This is a GitButler-managed conflicted commit. Files are auto-resolved
644+
using the \"ours\" side. The commit tree contains additional directories:
645+
.conflict-side-0 — our tree
646+
.conflict-side-1 — their tree
647+
.conflict-base-0 — the merge base tree
648+
.auto-resolution — the auto-resolved tree
649+
.conflict-files — metadata about conflicted files
650+
To manually resolve, check out this commit, remove the directories
651+
listed above, resolve the conflicts, and amend the commit.
652+
GitButler-Conflict: true";
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 Some(without_footer) = bytes.strip_suffix(footer.as_bytes()) else {
690+
// Footer not present — return unchanged to avoid stripping a `[conflict] `
691+
// prefix from a non-GitButler commit that happens to start with it.
692+
return BString::from(bytes);
693+
};
694+
695+
let without_prefix = without_footer
696+
.strip_prefix(CONFLICT_MESSAGE_PREFIX.as_bytes())
697+
.unwrap_or(without_footer);
698+
699+
BString::from(without_prefix)
700+
}
701+
702+
/// Check if a commit message contains GitButler conflict markers.
703+
///
704+
/// Requires both the `[conflict] ` prefix and the exact footer block appended
705+
/// by [`add_conflict_markers`] as a suffix, so arbitrary commits with
706+
/// `[conflict]` in their subject or `GitButler-Conflict: true` in the body
707+
/// are not misidentified.
708+
pub fn message_is_conflicted(message: &BStr) -> bool {
709+
let bytes = message.as_bytes();
710+
let footer = conflict_footer_block();
711+
bytes.starts_with(CONFLICT_MESSAGE_PREFIX.as_bytes()) && bytes.ends_with(footer.as_bytes())
712+
}

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: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,16 @@ 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+
// but avoid duplicating markers if the new message already has them.
333+
new_commit.message =
334+
if but_core::commit::message_is_conflicted(new_commit.message.as_ref())
335+
&& !but_core::commit::message_is_conflicted(new_message.as_ref())
336+
{
337+
but_core::commit::add_conflict_markers(new_message.as_ref())
338+
} else {
339+
new_message
340+
};
332341
}
333342
*cursor = commit::create(
334343
repo,
@@ -374,7 +383,15 @@ fn reword_commit(
374383
new_message: BString,
375384
) -> Result<gix::ObjectId> {
376385
let mut new_commit = repo.find_commit(oid)?.decode()?.to_owned()?;
377-
new_commit.message = new_message;
386+
// Preserve conflict markers if the commit was conflicted,
387+
// but avoid duplicating markers if the new message already has them.
388+
let was_conflicted = but_core::commit::message_is_conflicted(new_commit.message.as_ref());
389+
new_commit.message =
390+
if was_conflicted && !but_core::commit::message_is_conflicted(new_message.as_ref()) {
391+
but_core::commit::add_conflict_markers(new_message.as_ref())
392+
} else {
393+
new_message
394+
};
378395
Ok(commit::create(
379396
repo,
380397
new_commit,

0 commit comments

Comments
 (0)