Skip to content

Commit dc0c262

Browse files
authored
Merge pull request #13155 from gitbutlerapp/refactor-git2-to-gix
Fix workspace merge failure when stacks have adjacent edits
2 parents 8fc4bdb + d954a50 commit dc0c262

5 files changed

Lines changed: 102 additions & 21 deletions

File tree

crates/but-worktrees/src/integrate.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -244,20 +244,21 @@ fn worktree_integration_inner(
244244
let wd_tree = repo.create_wd_tree(0)?;
245245

246246
let working_dir_conflicts = {
247+
let gix_repo = ctx.clone_repo_for_merging()?;
247248
let before_heads = stacks
248249
.iter()
249250
.map(|s| s.head_oid(ctx))
250251
.collect::<Result<Vec<_>>>()?;
251252
let before = WorkspaceState::create_from_heads(ctx, perm, &before_heads)?;
252-
let before = merge_workspace(&*ctx.git2_repo.get()?, before)?;
253+
let before = merge_workspace(&gix_repo, &before)?.to_git2();
253254
let mut after_heads = stacks
254255
.iter()
255256
.filter(|s| s.id != stack.id)
256257
.map(|s| s.head_oid(ctx))
257258
.collect::<Result<Vec<_>>>()?;
258259
after_heads.push(output.top_commit);
259260
let after = WorkspaceState::create_from_heads(ctx, perm, &after_heads)?;
260-
let after = merge_workspace(&*ctx.git2_repo.get()?, after)?;
261+
let after = merge_workspace(&gix_repo, &after)?.to_git2();
261262
let index = move_tree(&*ctx.git2_repo.get()?, wd_tree.to_git2(), before, after)?;
262263

263264
index.has_conflicts()

crates/gitbutler-branch-actions/tests/branch-actions/driverless.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,18 @@ fn seed_fixture(repo: &gix::Repository, script_name: &str, repo_name: &str) -> R
163163
in_workspace: true,
164164
},
165165
],
166+
("workspace-commit.sh", "adjacent-stacks") => vec![
167+
StackSpec {
168+
id: 1,
169+
branches_base_to_top: &["stack_a"],
170+
in_workspace: true,
171+
},
172+
StackSpec {
173+
id: 2,
174+
branches_base_to_top: &["stack_b"],
175+
in_workspace: true,
176+
},
177+
],
166178
unsupported => anyhow::bail!("unsupported driverless fixture {unsupported:?}"),
167179
};
168180

crates/gitbutler-branch-actions/tests/branch-actions/workspace_commit.rs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,3 +71,35 @@ fn conflicting_stacks_evicted_from_workspace_commit_parents() -> Result<()> {
7171

7272
Ok(())
7373
}
74+
75+
/// When two applied stacks modify adjacent but non-overlapping sections of the same
76+
/// file, `merge_workspace` must produce a clean merge.
77+
///
78+
/// Stack A owns lines 1–5 and 11–15; Stack B owns lines 6–10.
79+
/// A's top hunk immediately precedes B's hunk (adjacency from above) and B's hunk
80+
/// immediately precedes A's bottom hunk (adjacency from below).
81+
///
82+
/// Before the fix, `merge_workspace` used git2's Myers diff which incorrectly flagged
83+
/// these adjacent hunks as conflicting (`MergeConflict (-24)`), breaking every workspace
84+
/// mutation (squash, reorder, etc.) that recomputed the workspace tree.
85+
#[test]
86+
fn merge_workspace_succeeds_with_adjacent_hunks_from_both_sides() -> Result<()> {
87+
let (ctx, _temp_dir) = command_ctx("adjacent-stacks")?;
88+
89+
// Build the workspace commit so both stacks are properly registered.
90+
gitbutler_branch_actions::update_workspace_commit(&ctx, false)?;
91+
92+
let vb_state = VirtualBranchesHandle::new(ctx.project_data_dir());
93+
let stacks = vb_state.list_stacks_in_workspace()?;
94+
assert_eq!(stacks.len(), 2, "both stacks should be in workspace");
95+
96+
// Build a WorkspaceState from both stacks and call merge_workspace directly.
97+
// This is the exact function that was fixed from git2 to gix.
98+
let guard = ctx.shared_worktree_access();
99+
let workspace =
100+
gitbutler_workspace::branch_trees::WorkspaceState::create(&ctx, guard.read_permission())?;
101+
let gix_repo = ctx.clone_repo_for_merging()?;
102+
gitbutler_workspace::branch_trees::merge_workspace(&gix_repo, &workspace)?;
103+
104+
Ok(())
105+
}

crates/gitbutler-branch-actions/tests/fixtures/workspace-commit.sh

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ git init --initial-branch=main remote
4242
git config user.name "Author"
4343
git config user.email "author@example.com"
4444
echo "base content" > shared.txt
45+
seq 15 > file
4546
git add . && git commit -m "init"
4647
)
4748

@@ -68,3 +69,31 @@ git clone remote conflicting-stacks
6869
# at main. The seed + update_workspace_commit call in the test will
6970
# rebuild it properly.
7071
)
72+
73+
# Two stacks each modifying adjacent (non-overlapping) sections of the same file,
74+
# with zero lines of buffer between the changed regions.
75+
# Stack A owns lines 1-5 and lines 11-15; Stack B owns lines 6-10.
76+
# A's top hunk immediately precedes B's hunk (adjacency from above), and
77+
# B's hunk immediately precedes A's bottom hunk (adjacency from below).
78+
# This exercises the gix fix for the git2 bug where adjacent hunks in an
79+
# octopus workspace merge were incorrectly flagged as conflicting.
80+
git clone remote adjacent-stacks
81+
(cd adjacent-stacks
82+
git config user.name "Author"
83+
git config user.email "author@example.com"
84+
85+
git checkout -b stack_a main
86+
87+
# Change lines 1-5 (top) and lines 11-15 (bottom); lines 6-10 untouched.
88+
printf 'a1\na2\na3\na4\na5\n6\n7\n8\n9\n10\na11\na12\na13\na14\na15\n' > file
89+
commit_with_tick "stack_a: change top and bottom sections"
90+
91+
git checkout -b stack_b main
92+
# Change only lines 6-10 (middle); lines 1-5 and 11-15 untouched from base.
93+
printf '1\n2\n3\n4\n5\nb6\nb7\nb8\nb9\nb10\n11\n12\n13\n14\n15\n' > file
94+
commit_with_tick "stack_b: change middle section"
95+
96+
# Point workspace at main; update_workspace_commit in the test rebuilds it properly.
97+
git checkout -b gitbutler/workspace main
98+
git commit --allow-empty -m "GitButler Workspace Commit"
99+
)

crates/gitbutler-workspace/src/branch_trees.rs

Lines changed: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -99,10 +99,11 @@ pub fn update_uncommitted_changes_with_tree(
9999
always_checkout: Option<bool>,
100100
_perm: &mut RepoExclusive,
101101
) -> Result<()> {
102+
let gix_repo = ctx.clone_repo_for_merging()?;
102103
let repo = &*ctx.git2_repo.get()?;
103104
if let Some(worktree_id) = old_uncommitted_changes {
104105
let mut new_uncommitted_changes =
105-
move_tree_between_workspaces(repo, worktree_id, old, new)?;
106+
move_tree_between_workspaces(repo, &gix_repo, worktree_id, old, new)?;
106107

107108
// If the new tree and old tree are the same, then we don't need to do anything
108109
if !new_uncommitted_changes.has_conflicts() && !always_checkout.unwrap_or(false) {
@@ -122,9 +123,8 @@ pub fn update_uncommitted_changes_with_tree(
122123
),
123124
)?;
124125
} else {
125-
let old_tree_id = merge_workspace(repo, old)?.to_gix();
126-
let new_tree_id = merge_workspace(repo, new)?.to_gix();
127-
let gix_repo = ctx.clone_repo_for_merging()?;
126+
let old_tree_id = merge_workspace(&gix_repo, &old)?;
127+
let new_tree_id = merge_workspace(&gix_repo, &new)?;
128128
but_core::worktree::safe_checkout(
129129
old_tree_id,
130130
new_tree_id,
@@ -139,12 +139,13 @@ pub fn update_uncommitted_changes_with_tree(
139139
/// like if they were on top of the new workspace.
140140
fn move_tree_between_workspaces(
141141
repo: &git2::Repository,
142+
gix_repo: &gix::Repository,
142143
tree: git2::Oid,
143144
old: WorkspaceState,
144145
new: WorkspaceState,
145146
) -> Result<git2::Index> {
146-
let old_workspace = merge_workspace(repo, old)?;
147-
let new_workspace = merge_workspace(repo, new)?;
147+
let old_workspace = merge_workspace(gix_repo, &old)?.to_git2();
148+
let new_workspace = merge_workspace(gix_repo, &new)?.to_git2();
148149
move_tree(repo, tree, old_workspace, new_workspace)
149150
}
150151

@@ -167,26 +168,32 @@ pub fn move_tree(
167168
Ok(merge)
168169
}
169170

170-
/// Octopus merge
171-
/// What: Takes N trees and a base tree and all the heads together with respect
172-
/// to the given base.
171+
/// Octopus merge using gix, which correctly resolves adjacent-hunk changes that git2 treats as conflicts.
172+
/// Takes N trees and a base tree and merges all the heads together with respect to the given base.
173173
///
174174
/// If there are no heads provided, the base will be returned.
175-
pub fn merge_workspace(repo: &git2::Repository, workspace: WorkspaceState) -> Result<git2::Oid> {
176-
let mut output = workspace.base;
175+
pub fn merge_workspace(
176+
repo: &gix::Repository,
177+
workspace: &WorkspaceState,
178+
) -> Result<gix::ObjectId> {
179+
let mut output = workspace.base.to_gix();
180+
let base = workspace.base.to_gix();
177181

178-
for head in workspace.heads {
179-
let mut merge_options = git2::MergeOptions::new();
180-
merge_options.fail_on_conflict(true);
182+
let (merge_options, conflict_kind) = repo.merge_options_fail_fast()?;
181183

184+
for head in &workspace.heads {
182185
let mut merge = repo.merge_trees(
183-
&repo.find_tree(workspace.base)?,
184-
&repo.find_tree(output)?,
185-
&repo.find_tree(head)?,
186-
Some(&merge_options),
186+
base,
187+
output,
188+
head.to_gix(),
189+
repo.default_merge_labels(),
190+
merge_options.clone(),
187191
)?;
188192

189-
output = merge.write_tree_to(repo)?;
193+
if merge.has_unresolved_conflicts(conflict_kind) {
194+
anyhow::bail!("merge conflict when computing workspace tree");
195+
}
196+
output = merge.tree.write()?.detach();
190197
}
191198

192199
Ok(output)

0 commit comments

Comments
 (0)