Skip to content

Commit c7477ed

Browse files
mtsgrdByron
authored andcommitted
Move conflicted commit detection from headers to commit message markers
The `gitbutler-conflicted` extra header is stripped when commits are rebased outside of GitButler, silently losing conflict state. Replace it with commit message markers that survive rebasing and are visible to developers in `git log`: - A `[conflict] ` prefix on the subject line for fast visual detection - A `GitButler-Conflict:` multi-line git trailer whose value explains the conflict tree layout Using a standard git trailer means the description is embedded as the trailer value with indented continuation lines, so `git interpret-trailers` and other standard tools can parse and manipulate it. The resulting commit message looks like: [conflict] <original subject> <original body> <existing trailers, if any> GitButler-Conflict: 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 ... To manually resolve, check out this commit, remove the directories listed above, resolve the conflicts, and amend the commit. The trailer is appended after any existing trailers in the message, preserving Change-Id, Signed-off-by, and similar lines. When stripping conflict markers (on resolution, display, or commit matching), the prefix, trailer, and all continuation lines are removed to restore the original message. 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. The CONFLICT-README.txt tree blob is removed since the trailer now serves the same explanatory purpose with better visibility. The conflicted file count (`Option<u64>`) stored in the old header was only ever used as a boolean, so the new approach uses a simple presence check with no count.
1 parent 6e8de80 commit c7477ed

20 files changed

Lines changed: 687 additions & 149 deletions

File tree

crates/but-core/src/commit.rs

Lines changed: 474 additions & 1 deletion
Large diffs are not rendered by default.

crates/but-core/tests/fixtures/scenario/multiple-remotes-with-tracking-branches.sh

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,9 @@ git remote add origin normal-remote
2929
git remote add nested/remote nested-remote
3030
git remote add nested/remote-b nested-remote-b
3131

32+
# NOTE: `git fetch` automatically creates remote HEAD refs (e.g.
33+
# refs/remotes/origin/HEAD) since git 2.48. Tests relying on these
34+
# refs require at least that version.
3235
git fetch origin
3336
git fetch nested/remote
3437
git fetch nested/remote-b

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
let headers = Headers::from_config(&repo.config_snapshot());
185189
new_commit
@@ -204,10 +208,6 @@ pub(crate) mod function {
204208
treat_as_unresolved: gix::merge::tree::TreatAsUnresolved,
205209
) -> anyhow::Result<gix::Id<'repo>> {
206210
let repo = resolved_tree_id.repo;
207-
// in case someone checks this out with vanilla Git, we should warn why it looks like this
208-
let readme_content =
209-
b"You have checked out a GitButler Conflicted commit. You probably didn't mean to do this.";
210-
let readme_blob = repo.write_blob(readme_content)?;
211211

212212
let conflicted_files =
213213
extract_conflicted_files(resolved_tree_id, cherry_pick, treat_as_unresolved)?;
@@ -242,15 +242,20 @@ pub(crate) mod function {
242242
resolved_tree_id,
243243
)?;
244244
tree.upsert(".conflict-files", EntryKind::Blob, conflicted_files_blob)?;
245-
tree.upsert("CONFLICT-README.txt", EntryKind::Blob, readme_blob)?;
246245

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

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

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
let headers = Headers::from_config(&repo.config_snapshot());
322325
new_commit
@@ -347,10 +350,6 @@ fn commit_from_conflicted_tree<'repo>(
347350
sign_commit: SignCommit,
348351
) -> anyhow::Result<gix::Id<'repo>> {
349352
let repo = resolved_tree_id.repo;
350-
// in case someone checks this out with vanilla Git, we should warn why it looks like this
351-
let readme_content =
352-
b"You have checked out a GitButler Conflicted commit. You probably didn't mean to do this.";
353-
let readme_blob = repo.write_blob(readme_content)?;
354353

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

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

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

crates/but-rebase/src/lib.rs

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,11 @@ fn rebase(
286286
None if commit.parents.is_empty() => {
287287
let mut new_commit = commit;
288288
if let Some(new_message) = new_message {
289-
new_commit.message = new_message;
289+
new_commit.message =
290+
but_core::commit::rewrite_conflict_markers_on_message_change(
291+
new_commit.message.as_ref(),
292+
new_message,
293+
);
290294
}
291295
cursor = Some(commit::create(
292296
repo,
@@ -325,7 +329,11 @@ fn rebase(
325329
let mut new_commit = repo.find_commit(new_commit)?.decode()?.to_owned()?;
326330
new_commit.parents = base_commit.parent_ids().map(|id| id.detach()).collect();
327331
if let Some(new_message) = new_message {
328-
new_commit.message = new_message;
332+
new_commit.message =
333+
but_core::commit::rewrite_conflict_markers_on_message_change(
334+
new_commit.message.as_ref(),
335+
new_message,
336+
);
329337
}
330338
*cursor = commit::create(
331339
repo,
@@ -371,7 +379,10 @@ fn reword_commit(
371379
new_message: BString,
372380
) -> Result<gix::ObjectId> {
373381
let mut new_commit = repo.find_commit(oid)?.decode()?.to_owned()?;
374-
new_commit.message = new_message;
382+
new_commit.message = but_core::commit::rewrite_conflict_markers_on_message_change(
383+
new_commit.message.as_ref(),
384+
new_message,
385+
);
375386
Ok(commit::create(
376387
repo,
377388
new_commit,

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

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ fn basic_cherry_pick_cp_conflicts() -> Result<()> {
6969

7070
insta::assert_debug_snapshot!(result, @"
7171
ConflictedCommit(
72-
Sha1(e9ee7b59aff786fc970c30f6965d1de1913c7ec4),
72+
Sha1(9661555c611ff64e8c597f7f5a0575f211eb2e12),
7373
)
7474
");
7575

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

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

185184
insta::assert_debug_snapshot!(result, @"
186185
ConflictedCommit(
187-
Sha1(0fcbe01202743fa55f1a1e07342ad26f2e7a0abe),
186+
Sha1(c8779c405b92941176effe9eb7a72c0dd410c3fd),
188187
)
189188
");
190189

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

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

312310
insta::assert_debug_snapshot!(result, @"
313311
ConflictedCommit(
314-
Sha1(28fa7c91af8652f4e69c1e3184f92569a3468a34),
312+
Sha1(2d4dcd916020924f2412642b842a791dfea74571),
315313
)
316314
");
317315

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

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

442439
insta::assert_debug_snapshot!(result, @"
443440
ConflictedCommit(
444-
Sha1(2e6cb06fe98780bb8c7a301a522edd98805d1499),
441+
Sha1(bf66deb17cc4e84faa063a3253566004aabaf7fe),
445442
)
446443
");
447444

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

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

689685
insta::assert_debug_snapshot!(result, @"
690686
ConflictedCommit(
691-
Sha1(28f862257bff139659b763a2c873b8d3f0f780b0),
687+
Sha1(2ee47b5cbe2705d5d81f4b4067ef4753d87b3b02),
692688
)
693689
");
694690

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

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

739734
insta::assert_debug_snapshot!(result, @"
740735
ConflictedCommit(
741-
Sha1(2e6cb06fe98780bb8c7a301a522edd98805d1499),
736+
Sha1(bf66deb17cc4e84faa063a3253566004aabaf7fe),
742737
)
743738
");
744739

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

759754
insta::assert_debug_snapshot!(result, @"
760755
Commit(
761-
Sha1(3d7dfa09a071658d3b84eb1ee195ea0ebfeb601f),
756+
Sha1(2a307db18bf263e3a802ac71282c5d8016ea75a1),
762757
)
763758
");
764759

crates/but-rebase/tests/rebase/graph_rebase/conflictable_restriction.rs

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ fn by_default_conflicts_are_allowed() -> Result<()> {
3636
insta::assert_snapshot!(overlayed, @"
3737
3838
└── 👉►:0[0]:main[🌳]
39-
├── ·3411540 (⌂|1) ►c
39+
├── ·103b227 (⌂|1) ►c
4040
└── ·5e0ba46 (⌂|1) ►a, ►b
4141
└── ►:1[1]:base
4242
└── ·6155f21 (⌂|1)
@@ -45,17 +45,26 @@ fn by_default_conflicts_are_allowed() -> Result<()> {
4545
assert_eq!(overlayed, graph_tree(&outcome.workspace.graph).to_string());
4646

4747
// We expect to see conflicted headers
48-
insta::assert_snapshot!(cat_commit(&repo, "c")?, @"
49-
tree c199bde16a034b8588f7c896ec3640c40ad017dd
48+
insta::assert_snapshot!(cat_commit(&repo, "c")?, @r#"
49+
tree d935c009623a61ebde94512baef22c76c5ccbaef
5050
parent 5e0ba4636be91de6216903697b269915d3db6c53
5151
author author <author@example.com> 946684800 +0000
5252
committer Committer (Memory Override) <committer@example.com> 946771200 +0000
5353
gitbutler-headers-version 2
5454
change-id 1
55-
gitbutler-conflicted 1
5655
57-
c
58-
");
56+
[conflict] c
57+
58+
GitButler-Conflict: This is a GitButler-managed conflicted commit. Files are auto-resolved
59+
using the "ours" side. The commit tree contains additional directories:
60+
.conflict-side-0 — our tree
61+
.conflict-side-1 — their tree
62+
.conflict-base-0 — the merge base tree
63+
.auto-resolution — the auto-resolved tree
64+
.conflict-files — metadata about conflicted files
65+
To manually resolve, check out this commit, remove the directories
66+
listed above, resolve the conflicts, and amend the commit.
67+
"#);
5968

6069
Ok(())
6170
}

0 commit comments

Comments
 (0)