diff --git a/crates/gitbutler-branch-actions/src/actions.rs b/crates/gitbutler-branch-actions/src/actions.rs index 0aba22503..9c2f00da9 100644 --- a/crates/gitbutler-branch-actions/src/actions.rs +++ b/crates/gitbutler-branch-actions/src/actions.rs @@ -1,4 +1,5 @@ use super::r#virtual as vbranch; +use crate::reorder_commits; use crate::upstream_integration::{self, BranchStatuses, Resolution, UpstreamIntegrationContext}; use crate::{ base, @@ -372,7 +373,14 @@ pub fn reorder_commit( SnapshotDetails::new(OperationKind::ReorderCommit), guard.write_permission(), ); - vbranch::reorder_commit(&ctx, branch_id, commit_oid, offset).map_err(Into::into) + reorder_commits::reorder_commit( + &ctx, + branch_id, + commit_oid, + offset, + guard.write_permission(), + ) + .map_err(Into::into) } pub fn reset_virtual_branch( diff --git a/crates/gitbutler-branch-actions/src/lib.rs b/crates/gitbutler-branch-actions/src/lib.rs index 2bd683c9f..736e89195 100644 --- a/crates/gitbutler-branch-actions/src/lib.rs +++ b/crates/gitbutler-branch-actions/src/lib.rs @@ -43,6 +43,8 @@ pub use remote::{RemoteBranch, RemoteBranchData, RemoteCommit}; pub mod conflicts; +mod reorder_commits; + mod author; mod status; use gitbutler_branch::VirtualBranchesHandle; diff --git a/crates/gitbutler-branch-actions/src/reorder_commits.rs b/crates/gitbutler-branch-actions/src/reorder_commits.rs new file mode 100644 index 000000000..3c902dced --- /dev/null +++ b/crates/gitbutler-branch-actions/src/reorder_commits.rs @@ -0,0 +1,609 @@ +use anyhow::{bail, Context as _, Result}; +use gitbutler_branch::{signature, BranchId, SignaturePurpose}; +use gitbutler_cherry_pick::RepositoryExt as _; +use gitbutler_command_context::CommandContext; +use gitbutler_commit::commit_ext::CommitExt as _; +use gitbutler_project::access::WorktreeWritePermission; +use gitbutler_repo::{rebase::cherry_rebase_group, LogUntil, RepositoryExt as _}; + +use crate::VirtualBranchesExt as _; + +/// +/// Presume we had the stack: +/// +/// A +/// B +/// C +/// D +/// +/// If C was the subject, and the offset was -1, we would expect: +/// +/// A +/// C +/// B +/// D +/// +/// Presume we had the stack: +/// +/// A +/// B +/// C +/// D +/// +/// If B was the subject, and the offset was 1, we would expect: +/// +/// A +/// C +/// B +/// D +/// + +// move a given commit in a branch up one or down one +// if the offset is positive, move the commit down one +// if the offset is negative, move the commit up one +// rewrites the branch head to the new head commit +pub(crate) fn reorder_commit( + ctx: &CommandContext, + branch_id: BranchId, + subject_commit_oid: git2::Oid, + offset: i32, + _perm: &mut WorktreeWritePermission, +) -> Result<()> { + let repository = ctx.repository(); + let vb_state = ctx.project().virtual_branches(); + let default_target = vb_state.get_default_target()?; + let default_target_commit = repository + .find_reference(&default_target.branch.to_string())? + .peel_to_commit()?; + let merge_base = repository.merge_base(default_target_commit.id(), subject_commit_oid)?; + + let mut branch = vb_state.get_branch_in_workspace(branch_id)?; + + let ReorderResult { tree, head } = inner_reorder_commit( + repository, + merge_base, + subject_commit_oid, + offset, + &repository.l(branch.head, LogUntil::Commit(merge_base))?, + &repository.find_tree(branch.tree)?, + ctx.project().succeeding_rebases, + )?; + + branch.tree = tree; + branch.head = head; + + branch.updated_timestamp_ms = gitbutler_time::time::now_ms(); + vb_state.set_branch(branch.clone())?; + + crate::integration::update_workspace_commit(&vb_state, ctx) + .context("failed to update gitbutler workspace")?; + + Ok(()) +} + +struct ReorderResult { + tree: git2::Oid, + head: git2::Oid, +} + +/// Commit reordering implemented with libgit2 primatives +/// +/// This returns a new head commit and tree for the branch. +/// +/// If the tree ends up in a conflicted state when rebasing the head commit +/// will actually be the commited tree in a conflicted state, and the tree +/// will be the auto-resolved tree. +/// +/// After usage (which should only be in `reorder_commit`), its important to +/// re-merge all the branch trees together and check that out, as otherwise +/// the tree writes will be lost. +/// +/// * `base_commit`: The merge base of the branch head and trunk +/// * `subject_commit`: The commit to be moved +/// * `offset`: The offset to move the subject commit by, where +/// negative is up (towards the branch head) and positive is down +/// * `branch_commits`: The commits all the commits in the branch. +/// * `branch_tree`: The tree of the branch +fn inner_reorder_commit( + repository: &git2::Repository, + base_commit: git2::Oid, + subject_commit: git2::Oid, + offset: i32, + branch_commits: &[git2::Oid], + branch_tree: &git2::Tree, + succeeding_rebases: bool, +) -> Result { + if branch_commits.len() < 2 { + bail!("Cannot re-order less than two commits"); + }; + + if offset == 0 { + return Ok(ReorderResult { + tree: branch_tree.id(), + head: branch_commits[0], + }); + }; + + ensure_offset_within_bounds(subject_commit, offset, branch_commits)?; + + let author = signature(SignaturePurpose::Author)?; + let committer = signature(SignaturePurpose::Committer)?; + let tree_commit = repository.commit( + None, + &author, + &committer, + "Uncommited changes", + branch_tree, + &[&repository.find_commit(branch_commits[0])?], + )?; + + let branch_commits = reorder_commit_list(subject_commit, offset, branch_commits)?; + + // Rebase branch commits + // We are passing all the commits to the cherry_rebase_group funcion, but + // this is not a concern as it will verbaitm copy any commits that have + // not had their parents changed. + let new_head_oid = + cherry_rebase_group(repository, base_commit, &branch_commits, succeeding_rebases)?; + + // Rebase branch tree on top of new stack + let new_tree_commit = + cherry_rebase_group(repository, new_head_oid, &[tree_commit], succeeding_rebases)?; + let new_tree_commit = repository.find_commit(new_tree_commit)?; + + if new_tree_commit.is_conflicted() { + Ok(ReorderResult { + tree: repository + .find_real_tree(&new_tree_commit, Default::default())? + .id(), + head: new_tree_commit.id(), + }) + } else { + Ok(ReorderResult { + tree: new_tree_commit.tree_id(), + head: new_head_oid, + }) + } +} + +fn reorder_commit_list( + subject_commit: git2::Oid, + offset: i32, + branch_commits: &[git2::Oid], +) -> Result> { + let subject_index = branch_commits + .iter() + .position(|c| *c == subject_commit) + .ok_or(anyhow::anyhow!( + "Subject commit not found in branch commits" + ))?; + + let mut output = branch_commits.to_owned(); + + output.remove(subject_index); + output.insert(((subject_index as i32) + offset) as usize, subject_commit); + + Ok(output) +} + +/// Presume we had the stack: +/// +/// A +/// B +/// C // idx 2 // 2 + -1 = 1; 1 < 0 => All good +/// D +/// +/// If C was the subject, and the offset was -1, we would expect: +/// +/// A +/// C +/// B +/// D +/// +/// Presume we had the stack: +/// +/// A +/// B // idx 1 // 1 + 1 = 2; 2 >= 4 => All good +/// C +/// D +/// +/// If B was the subject, and the offset was 1, we would expect: +/// +/// A +/// C +/// B +/// D +fn ensure_offset_within_bounds( + subject_commit: git2::Oid, + offset: i32, + branch_commits: &[git2::Oid], +) -> Result<()> { + let subject_index = branch_commits + .iter() + .position(|c| *c == subject_commit) + .ok_or(anyhow::anyhow!( + "Subject commit not found in branch commits" + ))?; + + if subject_index as i32 + offset < 0 { + bail!("Offset is out of bounds"); + } + + if subject_index as i32 + offset >= branch_commits.len() as i32 { + bail!("Offset is out of bounds"); + } + + Ok(()) +} + +#[cfg(test)] +mod test { + #[cfg(test)] + mod inner_reorder_commit { + use gitbutler_cherry_pick::RepositoryExt as _; + use gitbutler_commit::commit_ext::CommitExt as _; + use gitbutler_repo::LogUntil; + use gitbutler_repo::RepositoryExt as _; + use gitbutler_testsupport::testing_repository::{assert_tree_matches, TestingRepository}; + + #[test] + fn less_than_two_commits_is_an_error() { + let test_repository = TestingRepository::open(); + + let merge_base = test_repository.commit_tree(None, &[("foo.txt", "a")]); + let b = test_repository.commit_tree(Some(&merge_base), &[("foo.txt", "b")]); + + let result = crate::reorder_commits::inner_reorder_commit( + &test_repository.repository, + merge_base.id(), + b.id(), + 0, + &[b.id()], + &b.tree().unwrap(), + true, + ); + + assert!(result.is_err()); + } + + #[test] + fn unconflicting_reorder() { + let test_repository = TestingRepository::open(); + + let merge_base = test_repository.commit_tree(None, &[("foo.txt", "a")]); + // Commit A adds file bar + let a = test_repository + .commit_tree(Some(&merge_base), &[("foo.txt", "a"), ("bar.txt", "a")]); + // Commit B adds file baz + let b = test_repository.commit_tree( + Some(&a), + &[("foo.txt", "a"), ("bar.txt", "a"), ("baz.txt", "a")], + ); + + dbg!(merge_base.tree_id(), a.tree_id(), b.tree_id()); + + let result = crate::reorder_commits::inner_reorder_commit( + &test_repository.repository, + merge_base.id(), + a.id(), + -1, + &[b.id(), a.id()], + &b.tree().unwrap(), + true, + ) + .unwrap(); + + let a_prime: git2::Commit = + test_repository.repository.find_commit(result.head).unwrap(); + assert!(!a_prime.is_conflicted()); + + assert_tree_matches( + &test_repository.repository, + &a_prime, + &[("foo.txt", b"a"), ("bar.txt", b"a"), ("baz.txt", b"a")], + ); + + let b_prime: git2::Commit = a_prime.parent(0).unwrap(); + assert!(!b_prime.is_conflicted()); + + assert_tree_matches( + &test_repository.repository, + &b_prime, + &[("foo.txt", b"a"), ("baz.txt", b"a")], + ); + assert!(b_prime.tree().unwrap().get_name("bar.txt").is_none()); + + // In this case, the tree should be the same as a_prime, as there + // were no uncommited files + assert_eq!(result.tree, a_prime.tree_id()); + } + + #[test] + fn conflicting_reorder() { + let test_repository = TestingRepository::open(); + + let merge_base = test_repository.commit_tree(None, &[("foo.txt", "a")]); + let a = test_repository.commit_tree(Some(&merge_base), &[("foo.txt", "x")]); + let b = test_repository.commit_tree(Some(&a), &[("foo.txt", "y")]); + + // When we re-order a and b, they will conflict. + // + // Setup: : Step 1: : Step 2: : + // B: y : : A': x : + // | : : : + // A: x : B': a : B': a : <- B' is the auto-resolved tree + // | : : : + // MB: a : MB: a : MB: a : + // + // Reorder step 1: + // Cherry pick B on top of Merge Base: + // Merge trees: Bt and MBt, with base At. The theirs and ours sides + // both have changes compared to the base so it conflicts. + // We auto resolve to have the content of MBt: "a" + // + // Reorder step 2: + // Cherry pick A on top of B': + // Merge trees: At and B't, with base MBt. MBt is unchange relative + // to the base, so it merges cleanly to "x". + + let result = crate::reorder_commits::inner_reorder_commit( + &test_repository.repository, + merge_base.id(), + a.id(), + -1, + &[b.id(), a.id()], + &b.tree().unwrap(), + true, + ) + .unwrap(); + + let a_prime: git2::Commit = + test_repository.repository.find_commit(result.head).unwrap(); + assert!(!a_prime.is_conflicted()); + + assert_tree_matches(&test_repository.repository, &a_prime, &[("foo.txt", b"x")]); + + let b_prime: git2::Commit = a_prime.parent(0).unwrap(); + assert!(b_prime.is_conflicted()); + + assert_tree_matches( + &test_repository.repository, + &b_prime, + &[ + (".auto-resolution/foo.txt", b"a"), + (".conflict-base-0/foo.txt", b"x"), + (".conflict-side-0/foo.txt", b"a"), + (".conflict-side-1/foo.txt", b"y"), + ], + ); + + // In this case, the tree should be the same as a_prime, as there + // were no uncommited files + assert_eq!(result.tree, a_prime.tree_id()); + + // We should now be able to re-order the commits back back to their + // original order and the tree should be the same as the original. + + // When we re-order A' and B', they become unconflicted + // + // Setup: : Step 1: : Step 2: : + // A': y : : B'': x : + // | : : : + // B': a : A'': y : A'': y : <- B' is the auto-resolved tree + // | : : : + // MB: a : MB: a : MB: a : + // + // Reorder step 1: + // Cherry pick A'' on top of Merge Base: + // Merge trees: A't and MBt, with base B't (auto-resolution. MBt is + // unchanged relative to the base, so it merges cleanly to "x". + // + // Reorder step 2: + // Cherry pick B' on top of A'': + // Merge trees: A''t and B't (ours == Bt), with base At. + // A''t is y, B't is a, and At is y. This is a clean merge. + + let result = crate::reorder_commits::inner_reorder_commit( + &test_repository.repository, + merge_base.id(), + b_prime.id(), + -1, + &[a_prime.id(), b_prime.id()], + &a_prime.tree().unwrap(), + true, + ) + .unwrap(); + + let b_double_prime: git2::Commit = + test_repository.repository.find_commit(result.head).unwrap(); + + assert!(!b_double_prime.is_conflicted()); + assert_eq!(b_double_prime.tree_id(), b.tree_id()); + + let a_double_prime: git2::Commit = b_double_prime.parent(0).unwrap(); + + assert!(!a_double_prime.is_conflicted()); + assert_eq!(a_double_prime.tree_id(), a.tree_id()); + } + + #[test] + fn conflicting_tree() { + let test_repository = TestingRepository::open(); + + let merge_base = test_repository.commit_tree(None, &[("foo.txt", "a")]); + let a = test_repository.commit_tree(Some(&merge_base), &[("foo.txt", "x")]); + let b = test_repository.commit_tree(Some(&a), &[("foo.txt", "y")]); + let tree = test_repository.commit_tree(Some(&b), &[("foo.txt", "z")]); + + let result = crate::reorder_commits::inner_reorder_commit( + &test_repository.repository, + merge_base.id(), + a.id(), + -1, + &[b.id(), a.id()], + &tree.tree().unwrap(), + true, + ) + .unwrap(); + + let commits: Vec = test_repository + .repository + .log(result.head, LogUntil::Commit(merge_base.id())) + .unwrap(); + + assert_eq!( + commits.len(), + 3, + "The conflicted tree should be come a commit" + ); + + let tree_commit = &commits[0]; + let a_prime = &commits[1]; + let b_prime = &commits[2]; + + assert!(tree_commit.is_conflicted()); + assert!(!a_prime.is_conflicted()); + assert!(b_prime.is_conflicted()); + + assert_eq!( + test_repository + .repository + .find_real_tree(tree_commit, Default::default()) + .unwrap() + .id(), + result.tree, + "The tree should be the auto-resolved tree of the tree commit" + ); + + // Order the commits back to their initial order and all should be + // resolved + let result = crate::reorder_commits::inner_reorder_commit( + &test_repository.repository, + merge_base.id(), + b_prime.id(), + -1, + &[tree_commit.id(), a_prime.id(), b_prime.id()], + &tree.tree().unwrap(), + true, + ) + .unwrap(); + + let commits: Vec = test_repository + .repository + .log(result.head, LogUntil::Commit(merge_base.id())) + .unwrap(); + + assert_eq!(commits.len(), 3); + + let tree_commit = &commits[0]; + let b_double_prime = &commits[1]; + let a_double_prime = &commits[2]; + + assert!(!tree_commit.is_conflicted()); + assert!(!b_double_prime.is_conflicted()); + assert!(!a_double_prime.is_conflicted()); + + assert_eq!(tree_commit.tree_id(), result.tree); + } + } + + #[cfg(test)] + mod reorder_commit_list { + use gitbutler_testsupport::testing_repository::TestingRepository; + + use crate::reorder_commits::reorder_commit_list; + + #[test] + fn move_commit_up() { + let test_repository = TestingRepository::open(); + + let a = test_repository.commit_tree(None, &[("foo.txt", "a")]).id(); + let b = test_repository.commit_tree(None, &[("foo.txt", "b")]).id(); + let c = test_repository.commit_tree(None, &[("foo.txt", "c")]).id(); + let d = test_repository.commit_tree(None, &[("foo.txt", "d")]).id(); + + let branch_commits = reorder_commit_list(c, -1, &[a, b, c, d]).unwrap(); + + assert_eq!(branch_commits, &[a, c, b, d]); + } + + #[test] + fn move_commit_up_to_top() { + let test_repository = TestingRepository::open(); + + let a = test_repository.commit_tree(None, &[("foo.txt", "a")]).id(); + let b = test_repository.commit_tree(None, &[("foo.txt", "b")]).id(); + let c = test_repository.commit_tree(None, &[("foo.txt", "c")]).id(); + let d = test_repository.commit_tree(None, &[("foo.txt", "d")]).id(); + + let branch_commits = reorder_commit_list(c, -2, &[a, b, c, d]).unwrap(); + + assert_eq!(branch_commits, &[c, a, b, d]); + } + + #[test] + fn move_nowhere() { + let test_repository = TestingRepository::open(); + + let a = test_repository.commit_tree(None, &[("foo.txt", "a")]).id(); + let b = test_repository.commit_tree(None, &[("foo.txt", "b")]).id(); + let c = test_repository.commit_tree(None, &[("foo.txt", "c")]).id(); + let d = test_repository.commit_tree(None, &[("foo.txt", "d")]).id(); + + let branch_commits = reorder_commit_list(b, 0, &[a, b, c, d]).unwrap(); + + assert_eq!(branch_commits, &[a, b, c, d]); + } + + #[test] + fn move_commit_down() { + let test_repository = TestingRepository::open(); + + let a = test_repository.commit_tree(None, &[("foo.txt", "a")]).id(); + let b = test_repository.commit_tree(None, &[("foo.txt", "b")]).id(); + let c = test_repository.commit_tree(None, &[("foo.txt", "c")]).id(); + let d = test_repository.commit_tree(None, &[("foo.txt", "d")]).id(); + + let branch_commits = reorder_commit_list(b, 1, &[a, b, c, d]).unwrap(); + + assert_eq!(branch_commits, &[a, c, b, d]); + } + + #[test] + fn move_commit_down_two() { + let test_repository = TestingRepository::open(); + + let a = test_repository.commit_tree(None, &[("foo.txt", "a")]).id(); + let b = test_repository.commit_tree(None, &[("foo.txt", "b")]).id(); + let c = test_repository.commit_tree(None, &[("foo.txt", "c")]).id(); + let d = test_repository.commit_tree(None, &[("foo.txt", "d")]).id(); + + let branch_commits = reorder_commit_list(b, 2, &[a, b, c, d]).unwrap(); + + assert_eq!(branch_commits, &[a, c, d, b]); + } + } + + #[cfg(test)] + mod ensure_offset_within_bounds { + use crate::reorder_commits::ensure_offset_within_bounds; + use gitbutler_testsupport::testing_repository::TestingRepository; + + #[test] + fn test() { + let test_repository = TestingRepository::open(); + + let a = test_repository.commit_tree(None, &[("foo.txt", "a")]).id(); + let b = test_repository.commit_tree(None, &[("foo.txt", "b")]).id(); + let c = test_repository.commit_tree(None, &[("foo.txt", "c")]).id(); + let d = test_repository.commit_tree(None, &[("foo.txt", "d")]).id(); + + assert!(ensure_offset_within_bounds(b, -2, &[a, b, c, d]).is_err()); + assert!(ensure_offset_within_bounds(b, -1, &[a, b, c, d]).is_ok()); + assert!(ensure_offset_within_bounds(b, 0, &[a, b, c, d]).is_ok()); + assert!(ensure_offset_within_bounds(b, 1, &[a, b, c, d]).is_ok()); + assert!(ensure_offset_within_bounds(b, 2, &[a, b, c, d]).is_ok()); + assert!(ensure_offset_within_bounds(b, 3, &[a, b, c, d]).is_err()); + } + } +} diff --git a/crates/gitbutler-branch-actions/src/virtual.rs b/crates/gitbutler-branch-actions/src/virtual.rs index bdc654f92..e06bed322 100644 --- a/crates/gitbutler-branch-actions/src/virtual.rs +++ b/crates/gitbutler-branch-actions/src/virtual.rs @@ -12,8 +12,8 @@ use anyhow::{anyhow, bail, Context, Result}; use bstr::{BString, ByteSlice}; use git2_hooks::HookResult; use gitbutler_branch::{ - dedup, dedup_fmt, reconcile_claims, signature, Branch, BranchId, BranchOwnershipClaims, - BranchUpdateRequest, OwnershipClaim, SignaturePurpose, Target, VirtualBranchesHandle, + dedup, dedup_fmt, reconcile_claims, Branch, BranchId, BranchOwnershipClaims, + BranchUpdateRequest, OwnershipClaim, Target, VirtualBranchesHandle, }; use gitbutler_cherry_pick::RepositoryExt as _; use gitbutler_command_context::CommandContext; @@ -1588,139 +1588,6 @@ pub(crate) fn amend( } } -// move a given commit in a branch up one or down one -// if the offset is positive, move the commit down one -// if the offset is negative, move the commit up one -// rewrites the branch head to the new head commit -pub(crate) fn reorder_commit( - ctx: &CommandContext, - branch_id: BranchId, - commit_oid: git2::Oid, - offset: i32, -) -> Result<()> { - let vb_state = ctx.project().virtual_branches(); - - let default_target = vb_state.get_default_target()?; - - let mut branch = vb_state.get_branch_in_workspace(branch_id)?; - // find the commit to offset from - let commit = ctx - .repository() - .find_commit(commit_oid) - .context("failed to find commit")?; - - let parent = commit.parent(0).context("failed to find parent")?; - let parent_oid = parent.id(); - - let repository = ctx.repository(); - - let tree = repository - .find_tree(branch.tree) - .context("Failed to get branch tree")?; - - let author_signature = - signature(SignaturePurpose::Author).context("Failed to get gitbutler signature")?; - let committer_signature = - signature(SignaturePurpose::Committer).context("Failed to get gitbutler signature")?; - - let head = repository - .find_commit(branch.head) - .context("Failed to find branch head commit")?; - - let tree_commit = repository - .commit( - None, - &author_signature, - &committer_signature, - "Branch commited changes", - &tree, - &[&head], - ) - .context("Failed to commit uncommited changes")?; - - let succeeding_rebases = ctx.project().succeeding_rebases; - - if offset < 0 { - // move commit up - if branch.head == commit_oid { - // can't move the head commit up - return Ok(()); - } - - // get a list of the commits to rebase - let mut ids_to_rebase = ctx - .repository() - .l(branch.head, LogUntil::Commit(commit.id()))?; - - ids_to_rebase.insert( - ids_to_rebase.len() - offset.unsigned_abs() as usize, - commit_oid, - ); - - let new_head = - cherry_rebase_group(repository, parent_oid, &ids_to_rebase, succeeding_rebases) - .context("rebase failed")?; - - branch.head = new_head; - } else { - // move commit down - if default_target.sha == parent_oid { - // can't move the commit down past the target - return Ok(()); - } - - let mut target = parent.clone(); - - for _ in 0..offset { - target = target.parent(0).context("failed to find target")?; - } - - let target_oid = target.id(); - - // get a list of the commits to rebase - let mut ids_to_rebase: Vec = ctx - .repository() - .l(branch.head, LogUntil::Commit(target_oid))? - .iter() - .filter(|id| **id != commit_oid) - .cloned() - .collect(); - - ids_to_rebase.push(commit_oid); - - let new_head = - cherry_rebase_group(repository, target_oid, &ids_to_rebase, succeeding_rebases) - .context("rebase failed")?; - - branch.head = new_head; - } - - let new_tree_commit = - cherry_rebase_group(repository, branch.head, &[tree_commit], succeeding_rebases) - .context("rebase failed")?; - - let new_tree_commit = repository - .find_commit(new_tree_commit) - .context("Failed to find new tree commit")?; - - branch.tree = repository - .find_real_tree(&new_tree_commit, Default::default())? - .id(); - - // Use the conflicted tree commit as the head. - if new_tree_commit.is_conflicted() { - branch.head = new_tree_commit.id(); - } - - branch.updated_timestamp_ms = gitbutler_time::time::now_ms(); - vb_state.set_branch(branch.clone())?; - - crate::integration::update_workspace_commit(&vb_state, ctx) - .context("failed to update gitbutler workspace")?; - - Ok(()) -} - // create and insert a blank commit (no tree change) either above or below a commit // if offset is positive, insert below, if negative, insert above // return the oid of the new head commit of the branch with the inserted blank commit diff --git a/crates/gitbutler-cherry-pick/src/lib.rs b/crates/gitbutler-cherry-pick/src/lib.rs index 9f5905628..692420b7f 100644 --- a/crates/gitbutler-cherry-pick/src/lib.rs +++ b/crates/gitbutler-cherry-pick/src/lib.rs @@ -66,18 +66,18 @@ impl RepositoryExt for git2::Repository { ) -> Result { // we need to do a manual 3-way patch merge // find the base, which is the parent of to_rebase - let base = if to_rebase.is_conflicted() { + let base = dbg!(if to_rebase.is_conflicted() { // Use to_rebase's recorded base self.find_real_tree(to_rebase, ConflictedTreeKey::Base)? } else { let base_commit = to_rebase.parent(0)?; // Use the parent's auto-resolution self.find_real_tree(&base_commit, Default::default())? - }; + }); // Get the auto-resolution - let ours = self.find_real_tree(head, Default::default())?; + let ours = dbg!(self.find_real_tree(head, Default::default())?); // Get the original theirs - let thiers = self.find_real_tree(to_rebase, ConflictedTreeKey::Theirs)?; + let thiers = dbg!(self.find_real_tree(to_rebase, ConflictedTreeKey::Theirs)?); self.merge_trees(&base, &ours, &thiers, merge_options) .context("failed to merge trees for cherry pick") diff --git a/crates/gitbutler-repo/src/rebase.rs b/crates/gitbutler-repo/src/rebase.rs index 3297aae30..d773111d8 100644 --- a/crates/gitbutler-repo/src/rebase.rs +++ b/crates/gitbutler-repo/src/rebase.rs @@ -421,36 +421,11 @@ fn resolve_index( #[cfg(test)] mod test { - use std::path::Path; - - use bstr::ByteSlice; - - fn assert_tree_matches( - repository: &git2::Repository, - commit: &git2::Commit, - files: &[(&str, &[u8])], - ) { - let tree = commit.tree().unwrap(); - - for (path, content) in files { - let blob = tree.get_path(Path::new(path)).unwrap().id(); - let blob: git2::Blob = repository.find_blob(blob).unwrap(); - assert_eq!( - blob.content(), - *content, - "{}: expect {} == {}", - path, - blob.content().to_str_lossy(), - content.to_str_lossy() - ); - } - } - #[cfg(test)] mod cherry_rebase_group { - use crate::{rebase::test::assert_tree_matches, repository_ext::RepositoryExt as _}; + use crate::repository_ext::RepositoryExt as _; use gitbutler_commit::commit_ext::CommitExt; - use gitbutler_testsupport::testing_repository::TestingRepository; + use gitbutler_testsupport::testing_repository::{assert_tree_matches, TestingRepository}; use crate::{rebase::cherry_rebase_group, LogUntil}; @@ -651,9 +626,9 @@ mod test { #[cfg(test)] mod gitbutler_merge_commits { - use crate::rebase::{gitbutler_merge_commits, test::assert_tree_matches}; + use crate::rebase::gitbutler_merge_commits; use gitbutler_commit::commit_ext::CommitExt as _; - use gitbutler_testsupport::testing_repository::TestingRepository; + use gitbutler_testsupport::testing_repository::{assert_tree_matches, TestingRepository}; #[test] fn unconflicting_merge() { diff --git a/crates/gitbutler-testsupport/src/testing_repository.rs b/crates/gitbutler-testsupport/src/testing_repository.rs index b6050d519..c6ad7842d 100644 --- a/crates/gitbutler-testsupport/src/testing_repository.rs +++ b/crates/gitbutler-testsupport/src/testing_repository.rs @@ -1,5 +1,6 @@ -use std::fs; +use std::{fs, path::Path}; +use gix_testtools::bstr::ByteSlice as _; use tempfile::{tempdir, TempDir}; pub struct TestingRepository { @@ -79,3 +80,24 @@ impl TestingRepository { self.repository.find_commit(commit).unwrap() } } + +pub fn assert_tree_matches<'a>( + repository: &'a git2::Repository, + commit: &git2::Commit<'a>, + files: &[(&str, &[u8])], +) { + let tree = commit.tree().unwrap(); + + for (path, content) in files { + let blob = tree.get_path(Path::new(path)).unwrap().id(); + let blob: git2::Blob<'a> = repository.find_blob(blob).unwrap(); + assert_eq!( + blob.content(), + *content, + "{}: expect {} == {}", + path, + blob.content().to_str_lossy(), + content.to_str_lossy() + ); + } +}