Skip to content

Move conflicted commit detection from headers to commit message markers #13247

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Byron merged 3 commits into from
Apr 15, 2026
Merged

Move conflicted commit detection from headers to commit message markers #13247

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0d13d77
Move conflicted commit detection from headers to commit message markers
mtsgrd Apr 9, 2026
6b55c60
review
Byron Apr 14, 2026
79ac145
refactor - use `gix` more and avoid line-ending change
codex Apr 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
537 changes: 537 additions & 0 deletions crates/but-core/src/commit/conflict.rs
View file
Open in desktop

Large diffs are not rendered by default.

11 changes: 10 additions & 1 deletion crates/but-core/src/commit.rs → crates/but-core/src/commit/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -555,8 +555,11 @@ impl<'repo> Commit<'repo> {
}

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

/// If the commit is conflicted, then it returns the auto-resolution tree,
Expand Down Expand Up @@ -686,3 +689,9 @@ impl ConflictEntries {
set.len()
}
}

mod conflict;
pub use conflict::{
add_conflict_markers, is_conflicted, message_is_conflicted,
rewrite_conflict_markers_on_message_change, strip_conflict_markers,
};
3 changes: 3 additions & 0 deletions crates/but-core/tests/fixtures/scenario/multiple-remotes-with-tracking-branches.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ git remote add origin normal-remote
git remote add nested/remote nested-remote
git remote add nested/remote-b nested-remote-b

# NOTE: `git fetch` automatically creates remote HEAD refs (e.g.
# refs/remotes/origin/HEAD) since git 2.48. Tests relying on these
# refs require at least that version.
git fetch origin
git fetch nested/remote
git fetch nested/remote-b
44 changes: 12 additions & 32 deletions crates/but-rebase/src/cherry_pick.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ pub enum EmptyCommit {
}

pub(crate) mod function {
use std::{collections::HashSet, path::PathBuf};
use std::path::PathBuf;

use anyhow::{Context as _, bail};
use bstr::BString;
Expand Down Expand Up @@ -168,18 +168,16 @@ pub(crate) mod function {
}

let headers = to_rebase.headers();
let to_rebase_is_conflicted = headers.as_ref().is_some_and(|hdr| hdr.is_conflicted());
let mut new_commit = to_rebase.inner;
new_commit.tree = resolved_tree_id.detach();

// Assure the commit isn't thinking it's conflicted.
if to_rebase_is_conflicted {
if let Some(pos) = new_commit
.extra_headers()
.find_pos(HEADERS_CONFLICTED_FIELD)
{
new_commit.extra_headers.remove(pos);
}
new_commit.message = but_core::commit::strip_conflict_markers(new_commit.message.as_ref());
if let Some(pos) = new_commit
.extra_headers()
.find_pos(HEADERS_CONFLICTED_FIELD)
{
new_commit.extra_headers.remove(pos);
} else if headers.is_none() {
let headers = Headers::from_config(&repo.config_snapshot());
new_commit
Expand All @@ -204,10 +202,6 @@ pub(crate) mod function {
treat_as_unresolved: gix::merge::tree::TreatAsUnresolved,
) -> anyhow::Result<gix::Id<'repo>> {
let repo = resolved_tree_id.repo;
// in case someone checks this out with vanilla Git, we should warn why it looks like this
let readme_content =
b"You have checked out a GitButler Conflicted commit. You probably didn't mean to do this.";
let readme_blob = repo.write_blob(readme_content)?;

let conflicted_files =
extract_conflicted_files(resolved_tree_id, cherry_pick, treat_as_unresolved)?;
Expand Down Expand Up @@ -242,15 +236,18 @@ pub(crate) mod function {
resolved_tree_id,
)?;
tree.upsert(".conflict-files", EntryKind::Blob, conflicted_files_blob)?;
tree.upsert("CONFLICT-README.txt", EntryKind::Blob, readme_blob)?;

let mut headers = to_rebase
.headers()
.unwrap_or_else(|| Headers::from_config(&repo.config_snapshot()));
headers.conflicted = conflicted_files.conflicted_header_field();
headers.conflicted = None;
to_rebase.tree = tree.write().context("failed to write tree")?.detach();
set_parent(&mut to_rebase, head.id.detach())?;

// Add conflict markers to the commit message
to_rebase.inner.message =
but_core::commit::add_conflict_markers(to_rebase.inner.message.as_ref());

to_rebase.set_headers(&headers);
Ok(crate::commit::create(
repo,
Expand Down Expand Up @@ -341,22 +338,5 @@ pub(crate) mod function {
|| !self.our_entries.is_empty()
|| !self.their_entries.is_empty()
}

fn total_entries(&self) -> usize {
let set = self
.ancestor_entries
.iter()
.chain(self.our_entries.iter())
.chain(self.their_entries.iter())
.collect::<HashSet<_>>();

set.len()
}

/// Return the `conflicted` header field value.
pub(crate) fn conflicted_header_field(&self) -> Option<u64> {
let entries = self.total_entries();
Some(if entries > 0 { entries as u64 } else { 1 })
}
}
}
25 changes: 11 additions & 14 deletions crates/but-rebase/src/graph_rebase/cherry_pick.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,18 +305,16 @@ fn commit_from_unconflicted_tree<'repo>(
let repo = to_rebase.id.repo;

let headers = to_rebase.headers();
let to_rebase_is_conflicted = headers.as_ref().is_some_and(|hdr| hdr.is_conflicted());
let mut new_commit = to_rebase.inner;
new_commit.tree = resolved_tree_id.detach();

// Ensure the commit isn't thinking it's conflicted.
if to_rebase_is_conflicted {
if let Some(pos) = new_commit
.extra_headers()
.find_pos(HEADERS_CONFLICTED_FIELD)
{
new_commit.extra_headers.remove(pos);
}
new_commit.message = but_core::commit::strip_conflict_markers(new_commit.message.as_ref());
if let Some(pos) = new_commit
.extra_headers()
.find_pos(HEADERS_CONFLICTED_FIELD)
{
new_commit.extra_headers.remove(pos);
} else if headers.is_none() {
let headers = Headers::from_config(&repo.config_snapshot());
new_commit
Expand Down Expand Up @@ -347,10 +345,6 @@ fn commit_from_conflicted_tree<'repo>(
sign_commit: SignCommit,
) -> anyhow::Result<gix::Id<'repo>> {
let repo = resolved_tree_id.repo;
// in case someone checks this out with vanilla Git, we should warn why it looks like this
let readme_content =
b"You have checked out a GitButler Conflicted commit. You probably didn't mean to do this.";
let readme_blob = repo.write_blob(readme_content)?;

let conflicted_files =
extract_conflicted_files(resolved_tree_id, cherry_pick, treat_as_unresolved)?;
Expand Down Expand Up @@ -382,15 +376,18 @@ fn commit_from_conflicted_tree<'repo>(
resolved_tree_id,
)?;
tree.upsert(".conflict-files", EntryKind::Blob, conflicted_files_blob)?;
tree.upsert("CONFLICT-README.txt", EntryKind::Blob, readme_blob)?;

let mut headers = to_rebase
.headers()
.unwrap_or_else(|| Headers::from_config(&repo.config_snapshot()));
headers.conflicted = conflicted_files.conflicted_header_field();
headers.conflicted = None;
to_rebase.tree = tree.write().context("failed to write tree")?.detach();
to_rebase.parents = parents.into();

// Add conflict markers to the commit message
to_rebase.inner.message =
but_core::commit::add_conflict_markers(to_rebase.inner.message.as_ref());

to_rebase.set_headers(&headers);
Ok(crate::commit::create(
repo,
Expand Down
17 changes: 14 additions & 3 deletions crates/but-rebase/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,11 @@ fn rebase(
None if commit.parents.is_empty() => {
let mut new_commit = commit;
if let Some(new_message) = new_message {
new_commit.message = new_message;
new_commit.message =
but_core::commit::rewrite_conflict_markers_on_message_change(
new_commit.message.as_ref(),
new_message,
);
}
cursor = Some(commit::create(
repo,
Expand Down Expand Up @@ -325,7 +329,11 @@ fn rebase(
let mut new_commit = repo.find_commit(new_commit)?.decode()?.to_owned()?;
new_commit.parents = base_commit.parent_ids().map(|id| id.detach()).collect();
if let Some(new_message) = new_message {
new_commit.message = new_message;
new_commit.message =
but_core::commit::rewrite_conflict_markers_on_message_change(
new_commit.message.as_ref(),
new_message,
);
}
*cursor = commit::create(
repo,
Expand Down Expand Up @@ -371,7 +379,10 @@ fn reword_commit(
new_message: BString,
) -> Result<gix::ObjectId> {
let mut new_commit = repo.find_commit(oid)?.decode()?.to_owned()?;
new_commit.message = new_message;
new_commit.message = but_core::commit::rewrite_conflict_markers_on_message_change(
new_commit.message.as_ref(),
new_message,
);
Ok(commit::create(
repo,
new_commit,
Expand Down
29 changes: 12 additions & 17 deletions crates/but-rebase/tests/rebase/graph_rebase/cherry_pick.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ fn basic_cherry_pick_cp_conflicts() -> Result<()> {

insta::assert_debug_snapshot!(result, @"
ConflictedCommit(
Sha1(e9ee7b59aff786fc970c30f6965d1de1913c7ec4),
Sha1(9661555c611ff64e8c597f7f5a0575f211eb2e12),
)
");

Expand All @@ -80,7 +80,7 @@ fn basic_cherry_pick_cp_conflicts() -> Result<()> {
assert_eq!(&get_parents(&id.attach(&repo))?, &[onto]);

insta::assert_snapshot!(visualize_tree(id.attach(&repo)), @r#"
0367fb7
1090f8a
├── .auto-resolution:aa3d213
│ ├── base-f:100644:7898192 "a\n"
│ └── target-f:100644:eb5a316 "target\n"
Expand All @@ -95,7 +95,6 @@ fn basic_cherry_pick_cp_conflicts() -> Result<()> {
│ ├── base-f:100644:7898192 "a\n"
│ ├── clean-f:100644:8312630 "clean\n"
│ └── target-f:100644:9b1719f "conflict\n"
├── CONFLICT-README.txt:100644:2af04b7 "You have checked out a GitButler Conflicted commit. You probably didn\'t mean to do this."
├── base-f:100644:7898192 "a\n"
└── target-f:100644:eb5a316 "target\n"
"#);
Expand Down Expand Up @@ -184,7 +183,7 @@ fn single_parent_to_multiple_parents_cp_conflicts() -> Result<()> {

insta::assert_debug_snapshot!(result, @"
ConflictedCommit(
Sha1(0fcbe01202743fa55f1a1e07342ad26f2e7a0abe),
Sha1(c8779c405b92941176effe9eb7a72c0dd410c3fd),
)
");

Expand All @@ -195,7 +194,7 @@ fn single_parent_to_multiple_parents_cp_conflicts() -> Result<()> {
assert_eq!(&get_parents(&id.attach(&repo))?, &[onto, onto2]);

insta::assert_snapshot!(visualize_tree(id.attach(&repo)), @r#"
1804f3d
4c6dc70
├── .auto-resolution:744efa9
│ ├── base-f:100644:7898192 "a\n"
│ ├── target-2-f:100644:caac8f9 "target 2\n"
Expand All @@ -212,7 +211,6 @@ fn single_parent_to_multiple_parents_cp_conflicts() -> Result<()> {
│ ├── base-f:100644:7898192 "a\n"
│ ├── clean-f:100644:8312630 "clean\n"
│ └── target-f:100644:9b1719f "conflict\n"
├── CONFLICT-README.txt:100644:2af04b7 "You have checked out a GitButler Conflicted commit. You probably didn\'t mean to do this."
├── base-f:100644:7898192 "a\n"
├── target-2-f:100644:caac8f9 "target 2\n"
└── target-f:100644:eb5a316 "target\n"
Expand Down Expand Up @@ -311,7 +309,7 @@ fn multiple_parents_to_single_parent_cp_conflicts() -> Result<()> {

insta::assert_debug_snapshot!(result, @"
ConflictedCommit(
Sha1(28fa7c91af8652f4e69c1e3184f92569a3468a34),
Sha1(2d4dcd916020924f2412642b842a791dfea74571),
)
");

Expand All @@ -322,7 +320,7 @@ fn multiple_parents_to_single_parent_cp_conflicts() -> Result<()> {
assert_eq!(&get_parents(&id.attach(&repo))?, &[onto]);

insta::assert_snapshot!(visualize_tree(id.attach(&repo)), @r#"
91fe014
8c4acd1
├── .auto-resolution:aa3d213
│ ├── base-f:100644:7898192 "a\n"
│ └── target-f:100644:eb5a316 "target\n"
Expand All @@ -339,7 +337,6 @@ fn multiple_parents_to_single_parent_cp_conflicts() -> Result<()> {
│ ├── clean-2-f:100644:13e9394 "clean 2\n"
│ ├── clean-f:100644:8312630 "clean\n"
│ └── target-f:100644:9b1719f "conflict\n"
├── CONFLICT-README.txt:100644:2af04b7 "You have checked out a GitButler Conflicted commit. You probably didn\'t mean to do this."
├── base-f:100644:7898192 "a\n"
└── target-f:100644:eb5a316 "target\n"
"#);
Expand Down Expand Up @@ -441,7 +438,7 @@ fn multiple_parents_to_multiple_parents_cp_conflicts() -> Result<()> {

insta::assert_debug_snapshot!(result, @"
ConflictedCommit(
Sha1(2e6cb06fe98780bb8c7a301a522edd98805d1499),
Sha1(bf66deb17cc4e84faa063a3253566004aabaf7fe),
)
");

Expand All @@ -452,7 +449,7 @@ fn multiple_parents_to_multiple_parents_cp_conflicts() -> Result<()> {
assert_eq!(&get_parents(&id.attach(&repo))?, &[onto, onto2]);

insta::assert_snapshot!(visualize_tree(id.attach(&repo)), @r#"
0aeaf79
1620d95
├── .auto-resolution:744efa9
│ ├── base-f:100644:7898192 "a\n"
│ ├── target-2-f:100644:caac8f9 "target 2\n"
Expand All @@ -471,7 +468,6 @@ fn multiple_parents_to_multiple_parents_cp_conflicts() -> Result<()> {
│ ├── clean-2-f:100644:13e9394 "clean 2\n"
│ ├── clean-f:100644:8312630 "clean\n"
│ └── target-f:100644:9b1719f "conflict\n"
├── CONFLICT-README.txt:100644:2af04b7 "You have checked out a GitButler Conflicted commit. You probably didn\'t mean to do this."
├── base-f:100644:7898192 "a\n"
├── target-2-f:100644:caac8f9 "target 2\n"
└── target-f:100644:eb5a316 "target\n"
Expand Down Expand Up @@ -688,7 +684,7 @@ fn no_parents_to_single_parent_cp_conflicts() -> Result<()> {

insta::assert_debug_snapshot!(result, @"
ConflictedCommit(
Sha1(28f862257bff139659b763a2c873b8d3f0f780b0),
Sha1(2ee47b5cbe2705d5d81f4b4067ef4753d87b3b02),
)
");

Expand All @@ -699,7 +695,7 @@ fn no_parents_to_single_parent_cp_conflicts() -> Result<()> {
assert_eq!(&get_parents(&id.attach(&repo))?, &[onto]);

insta::assert_snapshot!(visualize_tree(id.attach(&repo)), @r#"
1267a55
38edd44
├── .auto-resolution:aa3d213
│ ├── base-f:100644:7898192 "a\n"
│ └── target-f:100644:eb5a316 "target\n"
Expand All @@ -711,7 +707,6 @@ fn no_parents_to_single_parent_cp_conflicts() -> Result<()> {
├── .conflict-side-1:144e5f5
│ ├── base-f:100644:7898192 "a\n"
│ └── target-f:100644:9b1719f "conflict\n"
├── CONFLICT-README.txt:100644:2af04b7 "You have checked out a GitButler Conflicted commit. You probably didn\'t mean to do this."
├── base-f:100644:7898192 "a\n"
└── target-f:100644:eb5a316 "target\n"
"#);
Expand All @@ -738,7 +733,7 @@ fn cherry_pick_back_to_original_parents_unconflicts() -> Result<()> {

insta::assert_debug_snapshot!(result, @"
ConflictedCommit(
Sha1(2e6cb06fe98780bb8c7a301a522edd98805d1499),
Sha1(bf66deb17cc4e84faa063a3253566004aabaf7fe),
)
");

Expand All @@ -758,7 +753,7 @@ fn cherry_pick_back_to_original_parents_unconflicts() -> Result<()> {

insta::assert_debug_snapshot!(result, @"
Commit(
Sha1(3d7dfa09a071658d3b84eb1ee195ea0ebfeb601f),
Sha1(2a307db18bf263e3a802ac71282c5d8016ea75a1),
)
");

Expand Down
Loading
Loading