From 2b1d808314e9578bc91d78b4bba9fe12bd5d0e39 Mon Sep 17 00:00:00 2001 From: Scott Chacon Date: Mon, 29 Apr 2024 15:03:01 +0200 Subject: [PATCH] add several history manipulation backend functions this adds backend functions in Rust to do the following: * move file hunks between commits (basic) * undo any commit in a stack * insert a blank commit * move a commit within the stack * update a commit message in place --- crates/gitbutler-core/src/snapshots/entry.rs | 3 + .../src/virtual_branches/controller.rs | 152 ++- .../src/virtual_branches/errors.rs | 90 +- .../src/virtual_branches/virtual.rs | 953 ++++++++++++++---- .../tests/suite/virtual_branches/amend.rs | 190 ++-- .../virtual_branches/insert_blank_commit.rs | 145 +++ .../tests/suite/virtual_branches/mod.rs | 4 + .../virtual_branches/move_commit_file.rs | 190 ++++ .../suite/virtual_branches/reorder_commit.rs | 123 +++ .../suite/virtual_branches/undo_commit.rs | 71 ++ crates/gitbutler-tauri/src/main.rs | 5 + .../gitbutler-tauri/src/virtual_branches.rs | 79 +- 12 files changed, 1667 insertions(+), 338 deletions(-) create mode 100644 crates/gitbutler-core/tests/suite/virtual_branches/insert_blank_commit.rs create mode 100644 crates/gitbutler-core/tests/suite/virtual_branches/move_commit_file.rs create mode 100644 crates/gitbutler-core/tests/suite/virtual_branches/reorder_commit.rs create mode 100644 crates/gitbutler-core/tests/suite/virtual_branches/undo_commit.rs diff --git a/crates/gitbutler-core/src/snapshots/entry.rs b/crates/gitbutler-core/src/snapshots/entry.rs index e6823eed5..83d51e4ea 100644 --- a/crates/gitbutler-core/src/snapshots/entry.rs +++ b/crates/gitbutler-core/src/snapshots/entry.rs @@ -150,6 +150,9 @@ pub enum OperationType { UpdateCommitMessage, MoveCommit, RestoreFromSnapshot, + ReorderCommit, + InsertBlankCommit, + MoveCommitFile, #[default] Unknown, } diff --git a/crates/gitbutler-core/src/virtual_branches/controller.rs b/crates/gitbutler-core/src/virtual_branches/controller.rs index a916ca82f..6698a089f 100644 --- a/crates/gitbutler-core/src/virtual_branches/controller.rs +++ b/crates/gitbutler-core/src/virtual_branches/controller.rs @@ -231,11 +231,70 @@ impl Controller { &self, project_id: &ProjectId, branch_id: &BranchId, + commit_oid: git::Oid, ownership: &BranchOwnershipClaims, ) -> Result { self.inner(project_id) .await - .amend(project_id, branch_id, ownership) + .amend(project_id, branch_id, commit_oid, ownership) + .await + } + + pub async fn move_commit_file( + &self, + project_id: &ProjectId, + branch_id: &BranchId, + from_commit_oid: git::Oid, + to_commit_oid: git::Oid, + ownership: &BranchOwnershipClaims, + ) -> Result { + self.inner(project_id) + .await + .move_commit_file( + project_id, + branch_id, + from_commit_oid, + to_commit_oid, + ownership, + ) + .await + } + + pub async fn undo_commit( + &self, + project_id: &ProjectId, + branch_id: &BranchId, + commit_oid: git::Oid, + ) -> Result<(), Error> { + self.inner(project_id) + .await + .undo_commit(project_id, branch_id, commit_oid) + .await + } + + pub async fn insert_blank_commit( + &self, + project_id: &ProjectId, + branch_id: &BranchId, + commit_oid: git::Oid, + offset: i32, + ) -> Result<(), Error> { + self.inner(project_id) + .await + .insert_blank_commit(project_id, branch_id, commit_oid, offset) + .await + } + + pub async fn reorder_commit( + &self, + project_id: &ProjectId, + branch_id: &BranchId, + commit_oid: git::Oid, + offset: i32, + ) -> Result<(), Error> { + self.inner(project_id) + .await + .reorder_commit(project_id, branch_id, commit_oid, offset) .await } @@ -714,12 +773,14 @@ impl ControllerInner { &self, project_id: &ProjectId, branch_id: &BranchId, + commit_oid: git::Oid, ownership: &BranchOwnershipClaims, ) -> Result { let _permit = self.semaphore.acquire().await; self.with_verify_branch(project_id, |project_repository, _| { - let result = super::amend(project_repository, branch_id, ownership).map_err(Into::into); + let result = super::amend(project_repository, branch_id, commit_oid, ownership) + .map_err(Into::into); snapshot::create( project_repository.project(), SnapshotDetails::new(OperationType::AmendCommit), @@ -728,6 +789,93 @@ impl ControllerInner { }) } + pub async fn move_commit_file( + &self, + project_id: &ProjectId, + branch_id: &BranchId, + from_commit_oid: git::Oid, + to_commit_oid: git::Oid, + ownership: &BranchOwnershipClaims, + ) -> Result { + let _permit = self.semaphore.acquire().await; + + self.with_verify_branch(project_id, |project_repository, _| { + let result = super::move_commit_file( + project_repository, + branch_id, + from_commit_oid, + to_commit_oid, + ownership, + ) + .map_err(Into::into); + snapshot::create( + project_repository.project(), + SnapshotDetails::new(OperationType::MoveCommitFile), + )?; + result + }) + } + + pub async fn undo_commit( + &self, + project_id: &ProjectId, + branch_id: &BranchId, + commit_oid: git::Oid, + ) -> Result<(), Error> { + let _permit = self.semaphore.acquire().await; + + self.with_verify_branch(project_id, |project_repository, _| { + let result = + super::undo_commit(project_repository, branch_id, commit_oid).map_err(Into::into); + snapshot::create( + project_repository.project(), + SnapshotDetails::new(OperationType::UndoCommit), + )?; + result + }) + } + + pub async fn insert_blank_commit( + &self, + project_id: &ProjectId, + branch_id: &BranchId, + commit_oid: git::Oid, + offset: i32, + ) -> Result<(), Error> { + let _permit = self.semaphore.acquire().await; + + self.with_verify_branch(project_id, |project_repository, user| { + let result = + super::insert_blank_commit(project_repository, branch_id, commit_oid, user, offset) + .map_err(Into::into); + snapshot::create( + project_repository.project(), + SnapshotDetails::new(OperationType::InsertBlankCommit), + )?; + result + }) + } + + pub async fn reorder_commit( + &self, + project_id: &ProjectId, + branch_id: &BranchId, + commit_oid: git::Oid, + offset: i32, + ) -> Result<(), Error> { + let _permit = self.semaphore.acquire().await; + + self.with_verify_branch(project_id, |project_repository, _| { + let result = super::reorder_commit(project_repository, branch_id, commit_oid, offset) + .map_err(Into::into); + snapshot::create( + project_repository.project(), + SnapshotDetails::new(OperationType::ReorderCommit), + )?; + result + }) + } + pub async fn reset_virtual_branch( &self, project_id: &ProjectId, diff --git a/crates/gitbutler-core/src/virtual_branches/errors.rs b/crates/gitbutler-core/src/virtual_branches/errors.rs index 764e4ff60..9e52fed2e 100644 --- a/crates/gitbutler-core/src/virtual_branches/errors.rs +++ b/crates/gitbutler-core/src/virtual_branches/errors.rs @@ -6,6 +6,59 @@ use crate::{ projects::ProjectId, }; +// Generic error enum for use in the virtual branches module. +#[derive(Debug, thiserror::Error)] +pub enum VirtualBranchError { + #[error("project")] + Conflict(ProjectConflict), + #[error("branch not found")] + BranchNotFound(BranchNotFound), + #[error("default target not set")] + DefaultTargetNotSet(DefaultTargetNotSet), + #[error("target ownership not found")] + TargetOwnerhshipNotFound(BranchOwnershipClaims), + #[error("git object {0} not found")] + GitObjectNotFound(git::Oid), + #[error("commit failed")] + CommitFailed, + #[error("rebase failed")] + RebaseFailed, + #[error("force push not allowed")] + ForcePushNotAllowed(ForcePushNotAllowed), + #[error("branch has no commits")] + BranchHasNoCommits, + #[error(transparent)] + Other(#[from] anyhow::Error), +} + +impl ErrorWithContext for VirtualBranchError { + fn context(&self) -> Option { + Some(match self { + VirtualBranchError::Conflict(ctx) => ctx.to_context(), + VirtualBranchError::BranchNotFound(ctx) => ctx.to_context(), + VirtualBranchError::DefaultTargetNotSet(ctx) => ctx.to_context(), + VirtualBranchError::TargetOwnerhshipNotFound(_) => { + error::Context::new_static(Code::Branches, "target ownership not found") + } + VirtualBranchError::GitObjectNotFound(oid) => { + error::Context::new(Code::Branches, format!("git object {oid} not found")) + } + VirtualBranchError::CommitFailed => { + error::Context::new_static(Code::Branches, "commit failed") + } + VirtualBranchError::RebaseFailed => { + error::Context::new_static(Code::Branches, "rebase failed") + } + VirtualBranchError::BranchHasNoCommits => error::Context::new_static( + Code::Branches, + "Branch has no commits - there is nothing to amend to", + ), + VirtualBranchError::ForcePushNotAllowed(ctx) => ctx.to_context(), + VirtualBranchError::Other(error) => return error.custom_context_or_root_cause().into(), + }) + } +} + #[derive(Debug, thiserror::Error)] pub enum VerifyError { #[error("head is detached")] @@ -316,43 +369,6 @@ impl ForcePushNotAllowed { } } -#[derive(Debug, thiserror::Error)] -pub enum AmendError { - #[error("force push not allowed")] - ForcePushNotAllowed(ForcePushNotAllowed), - #[error("target ownership not found")] - TargetOwnerhshipNotFound(BranchOwnershipClaims), - #[error("branch has no commits")] - BranchHasNoCommits, - #[error("default target not set")] - DefaultTargetNotSet(DefaultTargetNotSet), - #[error("branch not found")] - BranchNotFound(BranchNotFound), - #[error("project is in conflict state")] - Conflict(ProjectConflict), - #[error(transparent)] - Other(#[from] anyhow::Error), -} - -impl ErrorWithContext for AmendError { - fn context(&self) -> Option { - Some(match self { - AmendError::ForcePushNotAllowed(ctx) => ctx.to_context(), - AmendError::Conflict(ctx) => ctx.to_context(), - AmendError::BranchNotFound(ctx) => ctx.to_context(), - AmendError::BranchHasNoCommits => error::Context::new_static( - Code::Branches, - "Branch has no commits - there is nothing to amend to", - ), - AmendError::DefaultTargetNotSet(ctx) => ctx.to_context(), - AmendError::TargetOwnerhshipNotFound(_) => { - error::Context::new_static(Code::Branches, "target ownership not found") - } - AmendError::Other(error) => return error.custom_context_or_root_cause().into(), - }) - } -} - #[derive(Debug, thiserror::Error)] pub enum CherryPickError { #[error("target commit {0} not found ")] diff --git a/crates/gitbutler-core/src/virtual_branches/virtual.rs b/crates/gitbutler-core/src/virtual_branches/virtual.rs index 3f6e0b5fc..17e2dfd58 100644 --- a/crates/gitbutler-core/src/virtual_branches/virtual.rs +++ b/crates/gitbutler-core/src/virtual_branches/virtual.rs @@ -2692,15 +2692,291 @@ pub fn is_virtual_branch_mergeable( Ok(is_mergeable) } +// this function takes a list of file ownership from a "from" commit and "moves" +// those changes to a "to" commit in a branch. This allows users to drag changes +// from one commit to another. +// if the "to" commit is below the "from" commit, the changes are simply added to the "to" commit +// and the rebase should be simple. if the "to" commit is above the "from" commit, +// the changes need to be removed from the "from" commit, everything rebased, +// then added to the "to" commit and everything above that rebased again. +pub fn move_commit_file( + project_repository: &project_repository::Repository, + branch_id: &BranchId, + from_commit_oid: git::Oid, + to_commit_oid: git::Oid, + target_ownership: &BranchOwnershipClaims, +) -> Result { + let vb_state = VirtualBranchesHandle::new(&project_repository.project().gb_dir()); + + let mut target_branch = match vb_state.get_branch(branch_id) { + Ok(branch) => Ok(branch), + Err(reader::Error::NotFound) => return Ok(to_commit_oid), // this is wrong + Err(error) => Err(error), + } + .context("failed to read branch")?; + + let default_target = get_default_target(&vb_state) + .context("failed to read default target")? + .ok_or_else(|| { + errors::VirtualBranchError::DefaultTargetNotSet(errors::DefaultTargetNotSet { + project_id: project_repository.project().id, + }) + })?; + + let mut to_amend_oid = to_commit_oid.clone(); + let mut amend_commit = project_repository + .git_repository + .find_commit(to_amend_oid) + .context("failed to find commit")?; + + // find all the commits upstream from the target "to" commit + let mut upstream_commits = project_repository.l( + target_branch.head, + project_repository::LogUntil::Commit(amend_commit.id()), + )?; + + // get a list of all the diffs across all the virtual branches + let base_file_diffs = diff::workdir(&project_repository.git_repository, &default_target.sha) + .context("failed to diff workdir")?; + + // filter base_file_diffs to HashMap> only for hunks in target_ownership + // this is essentially the group of patches that we're "moving" + let diffs_to_amend = target_ownership + .claims + .iter() + .filter_map(|file_ownership| { + let hunks = base_file_diffs + .get(&file_ownership.file_path) + .map(|hunks| { + hunks + .hunks + .iter() + .filter(|hunk| { + file_ownership.hunks.iter().any(|owned_hunk| { + owned_hunk.start == hunk.new_start + && owned_hunk.end == hunk.new_start + hunk.new_lines + }) + }) + .cloned() + .collect::>() + }) + .unwrap_or_default(); + if hunks.is_empty() { + None + } else { + Some((file_ownership.file_path.clone(), hunks)) + } + }) + .collect::>(); + + // if we're not moving anything, return an error + if diffs_to_amend.is_empty() { + return Err(errors::VirtualBranchError::TargetOwnerhshipNotFound( + target_ownership.clone(), + )); + } + + // is from_commit_oid in upstream_commits? + if !upstream_commits.contains(&from_commit_oid) { + // this means that the "from" commit is _below_ the "to" commit in the history + // which makes things a little more complicated because in this case we need to + // remove the changes from the lower "from" commit, rebase everything, then add the changes + // to the _rebased_ version of the "to" commit that is above it. + + // first, let's get the from commit data and it's parent data + let from_commit = project_repository + .git_repository + .find_commit(from_commit_oid) + .context("failed to find commit")?; + let from_tree = from_commit.tree().context("failed to find tree")?; + let from_parent = from_commit.parent(0).context("failed to find parent")?; + let from_parent_tree = from_parent.tree().context("failed to find parent tree")?; + + // ok, what is the entire patch introduced in the "from" commit? + // we need to remove the parts of this patch that are in target_ownership (the parts we're moving) + // and then apply the rest to the parent tree of the "from" commit to + // create the new "from" commit without the changes we're moving + let from_commit_diffs = diff::trees( + &project_repository.git_repository, + &from_parent_tree, + &from_tree, + ) + .context("failed to diff trees")?; + + // filter from_commit_diffs to HashMap> only for hunks NOT in target_ownership + // this is the patch parts we're keeping + let diffs_to_keep = from_commit_diffs + .iter() + .filter_map(|(filepath, file_diff)| { + let hunks = file_diff + .hunks + .iter() + .filter(|hunk| { + !target_ownership.claims.iter().any(|file_ownership| { + file_ownership.file_path.eq(filepath) + && file_ownership.hunks.iter().any(|owned_hunk| { + owned_hunk.start == hunk.new_start + && owned_hunk.end == hunk.new_start + hunk.new_lines + }) + }) + }) + .cloned() + .collect::>(); + if hunks.is_empty() { + None + } else { + Some((filepath.clone(), hunks)) + } + }) + .collect::>(); + + let repo = &project_repository.git_repository; + + // write our new tree and commit for the new "from" commit without the moved changes + let new_from_tree_oid = + write_tree_onto_commit(project_repository, from_parent.id(), &diffs_to_keep)?; + let new_from_tree = &repo.find_tree(new_from_tree_oid).or_else(|_error| { + return Err(errors::VirtualBranchError::GitObjectNotFound( + new_from_tree_oid, + )); + })?; + let new_from_commit_oid = repo + .commit( + None, + &from_commit.author(), + &from_commit.committer(), + &from_commit.message().to_str_lossy(), + &new_from_tree, + &[&from_parent], + ) + .or_else(|_error| { + return Err(errors::VirtualBranchError::CommitFailed); + })?; + + // rebase everything above the new "from" commit that has the moved changes removed + let new_head = match cherry_rebase( + project_repository, + new_from_commit_oid, + from_commit_oid, + target_branch.head, + ) { + Ok(Some(new_head)) => new_head, + _ => { + return Err(errors::VirtualBranchError::RebaseFailed); + } + }; + + // ok, now we need to identify which the new "to" commit is in the rebased history + // so we'll take a list of the upstream oids and find it simply based on location + // (since the order should not have changed in our simple rebase) + let old_upstream_commit_oids = project_repository.l( + target_branch.head, + project_repository::LogUntil::Commit(default_target.sha), + )?; + + let new_upstream_commit_oids = project_repository.l( + new_head, + project_repository::LogUntil::Commit(default_target.sha), + )?; + + // find to_commit_oid offset in upstream_commits vector + let to_commit_offset = old_upstream_commit_oids + .iter() + .position(|c| *c == to_amend_oid) + .ok_or(errors::VirtualBranchError::Other(anyhow!( + "failed to find commit in old commits" + )))?; + + // find the new "to" commit in our new rebased upstream commits + to_amend_oid = *new_upstream_commit_oids.get(to_commit_offset).ok_or( + errors::VirtualBranchError::Other(anyhow!("failed to find commit in new commits")), + )?; + + // reset the "to" commit variable for writing the changes back to + amend_commit = project_repository + .git_repository + .find_commit(to_amend_oid) + .context("failed to find commit")?; + + // reset the concept of what the upstream commits are to be the rebased ones + upstream_commits = project_repository.l( + new_head, + project_repository::LogUntil::Commit(amend_commit.id()), + )?; + } + + // ok, now we will apply the moved changes to the "to" commit. + // if we were moving the changes down, we didn't need to rewrite the "from" commit + // because it will be rewritten with the upcoming rebase. + // if we were moving the changes "up" we've already rewritten the "from" commit + + // apply diffs_to_amend to the commit tree + // and write a new commit with the changes we're moving + let new_tree_oid = write_tree_onto_commit(project_repository, to_amend_oid, &diffs_to_amend)?; + let new_tree = project_repository + .git_repository + .find_tree(new_tree_oid) + .context("failed to find new tree")?; + let parents = amend_commit + .parents() + .context("failed to find head commit parents")?; + let commit_oid = project_repository + .git_repository + .commit( + None, + &amend_commit.author(), + &amend_commit.committer(), + &amend_commit.message().to_str_lossy(), + &new_tree, + &parents.iter().collect::>(), + ) + .context("failed to create commit")?; + + // now rebase upstream commits, if needed + + // if there are no upstream commits (the "to" commit was the branch head), then we're done + if upstream_commits.is_empty() { + target_branch.head = commit_oid; + vb_state.set_branch(target_branch.clone())?; + super::integration::update_gitbutler_integration(&vb_state, project_repository)?; + return Ok(commit_oid); + } + + // otherwise, rebase the upstream commits onto the new commit + let last_commit = upstream_commits.first().cloned().unwrap(); + let new_head = cherry_rebase( + project_repository, + commit_oid, + amend_commit.id(), + last_commit, + )?; + + // if that rebase worked, update the branch head and the gitbutler integration + if let Some(new_head) = new_head { + target_branch.head = new_head; + vb_state.set_branch(target_branch.clone())?; + super::integration::update_gitbutler_integration(&vb_state, project_repository)?; + return Ok(commit_oid); + } else { + return Err(errors::VirtualBranchError::RebaseFailed); + } +} + +// takes a list of file ownership and a commit oid and rewrites that commit to +// add the file changes. The branch is then rebased onto the new commit +// and the respective branch head is updated pub fn amend( project_repository: &project_repository::Repository, branch_id: &BranchId, + commit_oid: git::Oid, target_ownership: &BranchOwnershipClaims, -) -> Result { +) -> Result { if conflicts::is_conflicting::<&Path>(project_repository, None)? { - return Err(errors::AmendError::Conflict(errors::ProjectConflict { - project_id: project_repository.project().id, - })); + return Err(errors::VirtualBranchError::Conflict( + errors::ProjectConflict { + project_id: project_repository.project().id, + }, + )); } let vb_state = VirtualBranchesHandle::new(&project_repository.project().gb_dir()); @@ -2710,10 +2986,12 @@ pub fn amend( .context("failed to read virtual branches")?; if !all_branches.iter().any(|b| b.id == *branch_id) { - return Err(errors::AmendError::BranchNotFound(errors::BranchNotFound { - project_id: project_repository.project().id, - branch_id: *branch_id, - })); + return Err(errors::VirtualBranchError::BranchNotFound( + errors::BranchNotFound { + project_id: project_repository.project().id, + branch_id: *branch_id, + }, + )); } let applied_branches = all_branches @@ -2722,16 +3000,18 @@ pub fn amend( .collect::>(); if !applied_branches.iter().any(|b| b.id == *branch_id) { - return Err(errors::AmendError::BranchNotFound(errors::BranchNotFound { - project_id: project_repository.project().id, - branch_id: *branch_id, - })); + return Err(errors::VirtualBranchError::BranchNotFound( + errors::BranchNotFound { + project_id: project_repository.project().id, + branch_id: *branch_id, + }, + )); } let default_target = get_default_target(&vb_state) .context("failed to read default target")? .ok_or_else(|| { - errors::AmendError::DefaultTargetNotSet(errors::DefaultTargetNotSet { + errors::VirtualBranchError::DefaultTargetNotSet(errors::DefaultTargetNotSet { project_id: project_repository.project().id, }) })?; @@ -2750,7 +3030,7 @@ pub fn amend( .iter_mut() .find(|(b, _)| b.id == *branch_id) .ok_or_else(|| { - errors::AmendError::BranchNotFound(errors::BranchNotFound { + errors::VirtualBranchError::BranchNotFound(errors::BranchNotFound { project_id: project_repository.project().id, branch_id: *branch_id, }) @@ -2758,7 +3038,7 @@ pub fn amend( if target_branch.upstream.is_some() && !project_repository.project().ok_with_force_push { // amending to a pushed head commit will cause a force push that is not allowed - return Err(errors::AmendError::ForcePushNotAllowed( + return Err(errors::VirtualBranchError::ForcePushNotAllowed( errors::ForcePushNotAllowed { project_id: project_repository.project().id, }, @@ -2772,13 +3052,14 @@ pub fn amend( )? .is_empty() { - return Err(errors::AmendError::BranchHasNoCommits); + return Err(errors::VirtualBranchError::BranchHasNoCommits); } - let head_commit = project_repository + // find commit oid + let amend_commit = project_repository .git_repository - .find_commit(target_branch.head) - .context("failed to find head commit")?; + .find_commit(commit_oid) + .context("failed to find commit")?; let diffs_to_amend = target_ownership .claims @@ -2808,19 +3089,19 @@ pub fn amend( .collect::>(); if diffs_to_amend.is_empty() { - return Err(errors::AmendError::TargetOwnerhshipNotFound( + return Err(errors::VirtualBranchError::TargetOwnerhshipNotFound( target_ownership.clone(), )); } - let new_tree_oid = - write_tree_onto_commit(project_repository, target_branch.head, diffs_to_amend)?; + // apply diffs_to_amend to the commit tree + let new_tree_oid = write_tree_onto_commit(project_repository, commit_oid, &diffs_to_amend)?; let new_tree = project_repository .git_repository .find_tree(new_tree_oid) .context("failed to find new tree")?; - let parents = head_commit + let parents = amend_commit .parents() .context("failed to find head commit parents")?; @@ -2828,20 +3109,430 @@ pub fn amend( .git_repository .commit( None, - &head_commit.author(), - &head_commit.committer(), - &head_commit.message().to_str_lossy(), + &amend_commit.author(), + &amend_commit.committer(), + &amend_commit.message().to_str_lossy(), &new_tree, &parents.iter().collect::>(), ) .context("failed to create commit")?; - target_branch.head = commit_oid; - vb_state.set_branch(target_branch.clone())?; + // now rebase upstream commits, if needed + let upstream_commits = project_repository.l( + target_branch.head, + project_repository::LogUntil::Commit(amend_commit.id()), + )?; + // if there are no upstream commits, we're done + if upstream_commits.is_empty() { + target_branch.head = commit_oid; + vb_state.set_branch(target_branch.clone())?; + super::integration::update_gitbutler_integration(&vb_state, project_repository)?; + return Ok(commit_oid); + } - super::integration::update_gitbutler_integration(&vb_state, project_repository)?; + let last_commit = upstream_commits.first().cloned().unwrap(); - Ok(commit_oid) + let new_head = cherry_rebase( + project_repository, + commit_oid, + amend_commit.id(), + last_commit, + )?; + + if let Some(new_head) = new_head { + target_branch.head = new_head; + vb_state.set_branch(target_branch.clone())?; + super::integration::update_gitbutler_integration(&vb_state, project_repository)?; + return Ok(commit_oid); + } else { + return Err(errors::VirtualBranchError::RebaseFailed); + } +} + +// 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 fn reorder_commit( + project_repository: &project_repository::Repository, + branch_id: &BranchId, + commit_oid: git::Oid, + offset: i32, +) -> Result<(), errors::VirtualBranchError> { + let vb_state = VirtualBranchesHandle::new(&project_repository.project().gb_dir()); + + let default_target = get_default_target(&vb_state) + .context("failed to read default target")? + .ok_or_else(|| { + errors::VirtualBranchError::DefaultTargetNotSet(errors::DefaultTargetNotSet { + project_id: project_repository.project().id, + }) + })?; + + let mut branch = match vb_state.get_branch(branch_id) { + Ok(branch) => Ok(branch), + Err(reader::Error::NotFound) => Err(errors::VirtualBranchError::BranchNotFound( + errors::BranchNotFound { + branch_id: *branch_id, + project_id: project_repository.project().id, + }, + )), + Err(error) => Err(errors::VirtualBranchError::Other(error.into())), + }?; + + // find the commit to offset from + let commit = project_repository + .git_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(); + + 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 = project_repository.l( + branch.head, + project_repository::LogUntil::Commit(commit.id()), + )?; + let last_oid = ids_to_rebase.pop().context("failed to pop last oid")?; + ids_to_rebase.push(commit_oid); + ids_to_rebase.push(last_oid); + + match cherry_rebase_group(project_repository, parent_oid, &mut ids_to_rebase) { + Ok(Some(new_head)) => { + branch.head = new_head; + vb_state + .set_branch(branch.clone()) + .context("failed to write branch")?; + + super::integration::update_gitbutler_integration(&vb_state, project_repository) + .context("failed to update gitbutler integration")?; + } + _ => { + return Err(errors::VirtualBranchError::RebaseFailed); + } + } + } else { + // move commit down + if default_target.sha == parent_oid { + // can't move the commit down past the target + return Ok(()); + } + + let target = parent.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 = project_repository.l( + branch.head, + project_repository::LogUntil::Commit(commit.id()), + )?; + ids_to_rebase.push(parent_oid); + ids_to_rebase.push(commit_oid); + + match cherry_rebase_group(project_repository, target_oid, &mut ids_to_rebase) { + Ok(Some(new_head)) => { + branch.head = new_head; + vb_state + .set_branch(branch.clone()) + .context("failed to write branch")?; + + super::integration::update_gitbutler_integration(&vb_state, project_repository) + .context("failed to update gitbutler integration")?; + } + _ => { + return Err(errors::VirtualBranchError::RebaseFailed); + } + } + } + + return 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 +pub fn insert_blank_commit( + project_repository: &project_repository::Repository, + branch_id: &BranchId, + commit_oid: git::Oid, + user: Option<&users::User>, + offset: i32, +) -> Result<(), errors::VirtualBranchError> { + let vb_state = VirtualBranchesHandle::new(&project_repository.project().gb_dir()); + + let mut branch = match vb_state.get_branch(branch_id) { + Ok(branch) => Ok(branch), + Err(reader::Error::NotFound) => Err(errors::VirtualBranchError::BranchNotFound( + errors::BranchNotFound { + branch_id: *branch_id, + project_id: project_repository.project().id, + }, + )), + Err(error) => Err(errors::VirtualBranchError::Other(error.into())), + }?; + + // find the commit to offset from + let mut commit = project_repository + .git_repository + .find_commit(commit_oid) + .context("failed to find commit")?; + + if offset > 0 { + commit = commit.parent(0).context("failed to find parent")?; + } + + let commit_tree = commit.tree().unwrap(); + let blank_commit_oid = project_repository.commit(user, "", &commit_tree, &[&commit], None)?; + + if commit.id() == branch.head && offset < 0 { + // inserting before the first commit + branch.head = blank_commit_oid; + vb_state + .set_branch(branch.clone()) + .context("failed to write branch")?; + super::integration::update_gitbutler_integration(&vb_state, project_repository) + .context("failed to update gitbutler integration")?; + } else { + // rebase all commits above it onto the new commit + match cherry_rebase( + project_repository, + blank_commit_oid, + commit.id(), + branch.head, + ) { + Ok(Some(new_head)) => { + branch.head = new_head; + vb_state + .set_branch(branch.clone()) + .context("failed to write branch")?; + + super::integration::update_gitbutler_integration(&vb_state, project_repository) + .context("failed to update gitbutler integration")?; + } + _ => { + return Err(errors::VirtualBranchError::RebaseFailed); + } + } + } + + return Ok(()); +} + +// remove a commit in a branch by rebasing all commits _except_ for it onto it's parent +// if successful, it will update the branch head to the new head commit +pub fn undo_commit( + project_repository: &project_repository::Repository, + branch_id: &BranchId, + commit_oid: git::Oid, +) -> Result<(), errors::VirtualBranchError> { + let vb_state = VirtualBranchesHandle::new(&project_repository.project().gb_dir()); + + let mut branch = match vb_state.get_branch(branch_id) { + Ok(branch) => Ok(branch), + Err(reader::Error::NotFound) => Err(errors::VirtualBranchError::BranchNotFound( + errors::BranchNotFound { + branch_id: *branch_id, + project_id: project_repository.project().id, + }, + )), + Err(error) => Err(errors::VirtualBranchError::Other(error.into())), + }?; + + let commit = project_repository + .git_repository + .find_commit(commit_oid) + .context("failed to find commit")?; + + let new_commit_oid; + + if branch.head == commit_oid { + // if commit is the head, just set head to the parent + new_commit_oid = commit.parent(0).context("failed to find parent")?.id(); + } else { + // if commit is not the head, rebase all commits above it onto it's parent + let parent_commit_oid = commit.parent(0).context("failed to find parent")?.id(); + + match cherry_rebase( + project_repository, + parent_commit_oid, + commit_oid, + branch.head, + ) { + Ok(Some(new_head)) => { + new_commit_oid = new_head; + } + _ => { + return Err(errors::VirtualBranchError::RebaseFailed); + } + } + } + + if new_commit_oid != commit_oid { + branch.head = new_commit_oid; + vb_state + .set_branch(branch.clone()) + .context("failed to write branch")?; + + super::integration::update_gitbutler_integration(&vb_state, project_repository) + .context("failed to update gitbutler integration")?; + } + + return Ok(()); +} + +// cherry-pick based rebase, which handles empty commits +// this function takes a commit range and generates a Vector of commit oids +// and then passes them to `cherry_rebase_group` to rebase them onto the target commit +pub fn cherry_rebase( + project_repository: &project_repository::Repository, + target_commit_oid: git::Oid, + start_commit_oid: git::Oid, + end_commit_oid: git::Oid, +) -> Result, anyhow::Error> { + // get a list of the commits to rebase + let mut ids_to_rebase = project_repository.l( + end_commit_oid, + project_repository::LogUntil::Commit(start_commit_oid), + )?; + + if ids_to_rebase.is_empty() { + return Ok(None); + } + + let new_head_id = + cherry_rebase_group(project_repository, target_commit_oid, &mut ids_to_rebase)?; + + return Ok(new_head_id); +} + +// takes a vector of commit oids and rebases them onto a target commit and returns the +// new head commit oid if it's successful +// the difference between this and a libgit2 based rebase is that this will successfully +// rebase empty commits (two commits with identical trees) +fn cherry_rebase_group( + project_repository: &project_repository::Repository, + target_commit_oid: git::Oid, + ids_to_rebase: &mut Vec, +) -> Result, anyhow::Error> { + ids_to_rebase.reverse(); + // now, rebase unchanged commits onto the new commit + let commits_to_rebase = ids_to_rebase + .iter() + .map(|oid| project_repository.git_repository.find_commit(*oid)) + .collect::, _>>() + .context("failed to read commits to rebase")?; + + let new_head_id = commits_to_rebase + .into_iter() + .fold( + project_repository + .git_repository + .find_commit(target_commit_oid) + .context("failed to find new commit"), + |head, to_rebase| { + let head = head?; + + let mut cherrypick_index = project_repository + .git_repository + .cherry_pick(&head, &to_rebase) + .context("failed to cherry pick")?; + + if cherrypick_index.has_conflicts() { + bail!("failed to rebase"); + } + + let merge_tree_oid = cherrypick_index + .write_tree_to(&project_repository.git_repository) + .context("failed to write merge tree")?; + + let merge_tree = project_repository + .git_repository + .find_tree(merge_tree_oid) + .context("failed to find merge tree")?; + + let commit_oid = project_repository + .git_repository + .commit( + None, + &to_rebase.author(), + &to_rebase.committer(), + &to_rebase.message().to_str_lossy(), + &merge_tree, + &[&head], + ) + .context("failed to create commit")?; + + project_repository + .git_repository + .find_commit(commit_oid) + .context("failed to find commit") + }, + )? + .id(); + + return Ok(Some(new_head_id)); +} + +// runs a simple libgit2 based in-memory rebase on a commit range onto a target commit +// possibly not used in favor of cherry_rebase +pub fn simple_rebase( + project_repository: &project_repository::Repository, + target_commit_oid: git::Oid, + start_commit_oid: git::Oid, + end_commit_oid: git::Oid, +) -> Result, anyhow::Error> { + let repo = &project_repository.git_repository; + let (_, committer) = project_repository.git_signatures(None)?; + + let mut rebase_options = git2::RebaseOptions::new(); + rebase_options.quiet(true); + rebase_options.inmemory(true); + let mut rebase = repo + .rebase( + Some(end_commit_oid), + Some(start_commit_oid), + Some(target_commit_oid), + Some(&mut rebase_options), + ) + .context("failed to rebase")?; + + let mut rebase_success = true; + // check to see if these commits have already been pushed + let mut last_rebase_head = target_commit_oid; + while rebase.next().is_some() { + let index = rebase + .inmemory_index() + .context("failed to get inmemory index")?; + if index.has_conflicts() { + rebase_success = false; + break; + } + + if let Ok(commit_id) = rebase.commit(None, &committer.clone().into(), None) { + last_rebase_head = commit_id.into(); + } else { + rebase_success = false; + break; + } + } + + if rebase_success { + // rebase worked out, rewrite the branch head + rebase.finish(None).context("failed to finish rebase")?; + return Ok(Some(last_rebase_head)); + } else { + // rebase failed, do a merge commit + rebase.abort().context("failed to abort rebase")?; + return Ok(None); + } } pub fn cherry_pick( @@ -3102,13 +3793,6 @@ pub fn squash( return Err(errors::SquashError::CantSquashRootCommit); } - let ids_to_rebase = { - let ids = branch_commit_oids - .split(|oid| oid.eq(&commit_oid)) - .collect::>(); - ids.first().copied() - }; - // create a commit that: // * has the tree of the target commit // * has the message combined of the target commit and parent commit @@ -3133,79 +3817,34 @@ pub fn squash( ) .context("failed to commit")?; - let new_head_id = if let Some(ids_to_rebase) = ids_to_rebase { - let mut ids_to_rebase = ids_to_rebase.to_vec(); - ids_to_rebase.reverse(); + let ids_to_rebase = { + let ids = branch_commit_oids + .split(|oid| oid.eq(&commit_oid)) + .collect::>(); + ids.first().copied() + } + .ok_or(errors::SquashError::CommitNotFound(commit_oid))?; + let mut ids_to_rebase = ids_to_rebase.to_vec(); - // now, rebase unchanged commits onto the new commit - let commits_to_rebase = ids_to_rebase - .iter() - .map(|oid| project_repository.git_repository.find_commit(*oid)) - .collect::, _>>() - .context("failed to read commits to rebase")?; + match cherry_rebase_group(project_repository, new_commit_oid, &mut ids_to_rebase) { + Ok(Some(new_head_id)) => { + // save new branch head + branch.head = new_head_id; + vb_state + .set_branch(branch.clone()) + .context("failed to write branch")?; - commits_to_rebase - .into_iter() - .fold( - project_repository - .git_repository - .find_commit(new_commit_oid) - .context("failed to find new commit"), - |head, to_rebase| { - let head = head?; - - let mut cherrypick_index = project_repository - .git_repository - .cherry_pick(&head, &to_rebase) - .context("failed to cherry pick")?; - - if cherrypick_index.has_conflicts() { - bail!("failed to rebase"); - } - - let merge_tree_oid = cherrypick_index - .write_tree_to(&project_repository.git_repository) - .context("failed to write merge tree")?; - - let merge_tree = project_repository - .git_repository - .find_tree(merge_tree_oid) - .context("failed to find merge tree")?; - - let commit_oid = project_repository - .git_repository - .commit( - None, - &to_rebase.author(), - &to_rebase.committer(), - &to_rebase.message().to_str_lossy(), - &merge_tree, - &[&head], - ) - .context("failed to create commit")?; - - project_repository - .git_repository - .find_commit(commit_oid) - .context("failed to find commit") - }, - )? - .id() - } else { - new_commit_oid - }; - - // save new branch head - branch.head = new_head_id; - vb_state - .set_branch(branch.clone()) - .context("failed to write branch")?; - - super::integration::update_gitbutler_integration(&vb_state, project_repository)?; - - Ok(()) + super::integration::update_gitbutler_integration(&vb_state, project_repository) + .context("failed to update gitbutler integration")?; + Ok(()) + } + _ => { + return Err(errors::SquashError::Other(anyhow!("rebase error"))); + } + } } +// changes a commit message for commit_oid, rebases everything above it, updates branch head if successful pub fn update_commit_message( project_repository: &project_repository::Repository, branch_id: &BranchId, @@ -3280,13 +3919,6 @@ pub fn update_commit_message( .find_commit(commit_oid) .context("failed to find commit")?; - let ids_to_rebase = { - let ids = branch_commit_oids - .split(|oid| oid.eq(&commit_oid)) - .collect::>(); - ids.first().copied() - }; - let parents = target_commit .parents() .context("failed to find head commit parents")?; @@ -3303,79 +3935,36 @@ pub fn update_commit_message( ) .context("failed to commit")?; - let new_head_id = if let Some(ids_to_rebase) = ids_to_rebase { - let mut ids_to_rebase = ids_to_rebase.to_vec(); - ids_to_rebase.reverse(); - // now, rebase unchanged commits onto the new commit - let commits_to_rebase = ids_to_rebase - .iter() - .map(|oid| project_repository.git_repository.find_commit(*oid)) - .collect::, _>>() - .context("failed to read commits to rebase")?; + let ids_to_rebase = { + let ids = branch_commit_oids + .split(|oid| oid.eq(&commit_oid)) + .collect::>(); + ids.first().copied() + } + .ok_or(errors::UpdateCommitMessageError::CommitNotFound(commit_oid))?; + let mut ids_to_rebase = ids_to_rebase.to_vec(); - commits_to_rebase - .into_iter() - .fold( - project_repository - .git_repository - .find_commit(new_commit_oid) - .context("failed to find new commit"), - |head, to_rebase| { - let head = head?; + match cherry_rebase_group(project_repository, new_commit_oid, &mut ids_to_rebase) { + Ok(Some(new_head_id)) => { + // save new branch head + branch.head = new_head_id; + vb_state + .set_branch(branch.clone()) + .context("failed to write branch")?; - let mut cherrypick_index = project_repository - .git_repository - .cherry_pick(&head, &to_rebase) - .context("failed to cherry pick")?; - - if cherrypick_index.has_conflicts() { - bail!("failed to rebase"); - } - - let merge_tree_oid = cherrypick_index - .write_tree_to(&project_repository.git_repository) - .context("failed to write merge tree")?; - - let merge_tree = project_repository - .git_repository - .find_tree(merge_tree_oid) - .context("failed to find merge tree")?; - - let commit_oid = project_repository - .git_repository - .commit( - None, - &to_rebase.author(), - &to_rebase.committer(), - &to_rebase.message().to_str_lossy(), - &merge_tree, - &[&head], - ) - .context("failed to create commit")?; - - project_repository - .git_repository - .find_commit(commit_oid) - .context("failed to find commit") - }, - )? - .id() - } else { - new_commit_oid - }; - - // save new branch head - branch.head = new_head_id; - vb_state - .set_branch(branch.clone()) - .context("failed to write branch")?; - - super::integration::update_gitbutler_integration(&vb_state, project_repository)?; - - Ok(()) + super::integration::update_gitbutler_integration(&vb_state, project_repository) + .context("failed to update gitbutler integration")?; + Ok(()) + } + _ => { + return Err(errors::UpdateCommitMessageError::Other(anyhow!( + "rebase error" + ))); + } + } } -/// moves commit on top of the to target branch +/// moves commit from the branch it's in to the top of the target branch pub fn move_commit( project_repository: &project_repository::Repository, target_branch_id: &BranchId, diff --git a/crates/gitbutler-core/tests/suite/virtual_branches/amend.rs b/crates/gitbutler-core/tests/suite/virtual_branches/amend.rs index 0faecfaac..f0252b1b8 100644 --- a/crates/gitbutler-core/tests/suite/virtual_branches/amend.rs +++ b/crates/gitbutler-core/tests/suite/virtual_branches/amend.rs @@ -1,37 +1,5 @@ use super::*; -#[tokio::test] -async fn to_default_target() { - let Test { - repository, - project_id, - controller, - .. - } = &Test::default(); - - controller - .set_base_branch(project_id, &"refs/remotes/origin/master".parse().unwrap()) - .await - .unwrap(); - - let branch_id = controller - .create_virtual_branch(project_id, &branch::BranchCreateRequest::default()) - .await - .unwrap(); - - // amend without head commit - fs::write(repository.path().join("file2.txt"), "content").unwrap(); - let to_amend: branch::BranchOwnershipClaims = "file2.txt:1-2".parse().unwrap(); - assert!(matches!( - controller - .amend(project_id, &branch_id, &to_amend) - .await - .unwrap_err() - .downcast_ref(), - Some(&errors::AmendError::BranchHasNoCommits) - )); -} - #[tokio::test] async fn forcepush_allowed() { let Test { @@ -70,14 +38,12 @@ async fn forcepush_allowed() { .await .unwrap(); - { - // create commit - fs::write(repository.path().join("file.txt"), "content").unwrap(); - controller - .create_commit(project_id, &branch_id, "commit one", None, false) - .await - .unwrap(); - }; + // create commit + fs::write(repository.path().join("file.txt"), "content").unwrap(); + let commit_id = controller + .create_commit(project_id, &branch_id, "commit one", None, false) + .await + .unwrap(); controller .push_virtual_branch(project_id, &branch_id, false, None) @@ -89,7 +55,7 @@ async fn forcepush_allowed() { fs::write(repository.path().join("file2.txt"), "content2").unwrap(); let to_amend: branch::BranchOwnershipClaims = "file2.txt:1-2".parse().unwrap(); controller - .amend(project_id, &branch_id, &to_amend) + .amend(project_id, &branch_id, commit_id, &to_amend) .await .unwrap(); @@ -137,14 +103,12 @@ async fn forcepush_forbidden() { .await .unwrap(); - { - // create commit - fs::write(repository.path().join("file.txt"), "content").unwrap(); - controller - .create_commit(project_id, &branch_id, "commit one", None, false) - .await - .unwrap(); - }; + // create commit + fs::write(repository.path().join("file.txt"), "content").unwrap(); + let commit_oid = controller + .create_commit(project_id, &branch_id, "commit one", None, false) + .await + .unwrap(); controller .push_virtual_branch(project_id, &branch_id, false, None) @@ -156,11 +120,11 @@ async fn forcepush_forbidden() { let to_amend: branch::BranchOwnershipClaims = "file2.txt:1-2".parse().unwrap(); assert!(matches!( controller - .amend(project_id, &branch_id, &to_amend) + .amend(project_id, &branch_id, commit_oid, &to_amend) .await .unwrap_err() .downcast_ref(), - Some(errors::AmendError::ForcePushNotAllowed(_)) + Some(errors::VirtualBranchError::ForcePushNotAllowed(_)) )); } } @@ -184,33 +148,31 @@ async fn non_locked_hunk() { .await .unwrap(); - { - // create commit - fs::write(repository.path().join("file.txt"), "content").unwrap(); - controller - .create_commit(project_id, &branch_id, "commit one", None, false) - .await - .unwrap(); + // create commit + fs::write(repository.path().join("file.txt"), "content").unwrap(); + let commit_oid = controller + .create_commit(project_id, &branch_id, "commit one", None, false) + .await + .unwrap(); - let branch = controller - .list_virtual_branches(project_id) - .await - .unwrap() - .0 - .into_iter() - .find(|b| b.id == branch_id) - .unwrap(); - assert_eq!(branch.commits.len(), 1); - assert_eq!(branch.files.len(), 0); - assert_eq!(branch.commits[0].files.len(), 1); - }; + let branch = controller + .list_virtual_branches(project_id) + .await + .unwrap() + .0 + .into_iter() + .find(|b| b.id == branch_id) + .unwrap(); + assert_eq!(branch.commits.len(), 1); + assert_eq!(branch.files.len(), 0); + assert_eq!(branch.commits[0].files.len(), 1); { // amend another hunk fs::write(repository.path().join("file2.txt"), "content2").unwrap(); let to_amend: branch::BranchOwnershipClaims = "file2.txt:1-2".parse().unwrap(); controller - .amend(project_id, &branch_id, &to_amend) + .amend(project_id, &branch_id, commit_oid, &to_amend) .await .unwrap(); @@ -247,37 +209,35 @@ async fn locked_hunk() { .await .unwrap(); - { - // create commit - fs::write(repository.path().join("file.txt"), "content").unwrap(); - controller - .create_commit(project_id, &branch_id, "commit one", None, false) - .await - .unwrap(); + // create commit + fs::write(repository.path().join("file.txt"), "content").unwrap(); + let commit_oid = controller + .create_commit(project_id, &branch_id, "commit one", None, false) + .await + .unwrap(); - let branch = controller - .list_virtual_branches(project_id) - .await - .unwrap() - .0 - .into_iter() - .find(|b| b.id == branch_id) - .unwrap(); - assert_eq!(branch.commits.len(), 1); - assert_eq!(branch.files.len(), 0); - assert_eq!(branch.commits[0].files.len(), 1); - assert_eq!( - branch.commits[0].files[0].hunks[0].diff, - "@@ -0,0 +1 @@\n+content\n\\ No newline at end of file\n" - ); - }; + let branch = controller + .list_virtual_branches(project_id) + .await + .unwrap() + .0 + .into_iter() + .find(|b| b.id == branch_id) + .unwrap(); + assert_eq!(branch.commits.len(), 1); + assert_eq!(branch.files.len(), 0); + assert_eq!(branch.commits[0].files.len(), 1); + assert_eq!( + branch.commits[0].files[0].hunks[0].diff, + "@@ -0,0 +1 @@\n+content\n\\ No newline at end of file\n" + ); { // amend another hunk fs::write(repository.path().join("file.txt"), "more content").unwrap(); let to_amend: branch::BranchOwnershipClaims = "file.txt:1-2".parse().unwrap(); controller - .amend(project_id, &branch_id, &to_amend) + .amend(project_id, &branch_id, commit_oid, &to_amend) .await .unwrap(); @@ -319,37 +279,35 @@ async fn non_existing_ownership() { .await .unwrap(); - { - // create commit - fs::write(repository.path().join("file.txt"), "content").unwrap(); - controller - .create_commit(project_id, &branch_id, "commit one", None, false) - .await - .unwrap(); + // create commit + fs::write(repository.path().join("file.txt"), "content").unwrap(); + let commit_oid = controller + .create_commit(project_id, &branch_id, "commit one", None, false) + .await + .unwrap(); - let branch = controller - .list_virtual_branches(project_id) - .await - .unwrap() - .0 - .into_iter() - .find(|b| b.id == branch_id) - .unwrap(); - assert_eq!(branch.commits.len(), 1); - assert_eq!(branch.files.len(), 0); - assert_eq!(branch.commits[0].files.len(), 1); - }; + let branch = controller + .list_virtual_branches(project_id) + .await + .unwrap() + .0 + .into_iter() + .find(|b| b.id == branch_id) + .unwrap(); + assert_eq!(branch.commits.len(), 1); + assert_eq!(branch.files.len(), 0); + assert_eq!(branch.commits[0].files.len(), 1); { // amend non existing hunk let to_amend: branch::BranchOwnershipClaims = "file2.txt:1-2".parse().unwrap(); assert!(matches!( controller - .amend(project_id, &branch_id, &to_amend) + .amend(project_id, &branch_id, commit_oid, &to_amend) .await .unwrap_err() .downcast_ref(), - Some(errors::AmendError::TargetOwnerhshipNotFound(_)) + Some(errors::VirtualBranchError::TargetOwnerhshipNotFound(_)) )); } } diff --git a/crates/gitbutler-core/tests/suite/virtual_branches/insert_blank_commit.rs b/crates/gitbutler-core/tests/suite/virtual_branches/insert_blank_commit.rs new file mode 100644 index 000000000..186814279 --- /dev/null +++ b/crates/gitbutler-core/tests/suite/virtual_branches/insert_blank_commit.rs @@ -0,0 +1,145 @@ +use super::*; + +#[tokio::test] +async fn insert_blank_commit_down() { + let Test { + repository, + project_id, + controller, + .. + } = &Test::default(); + + controller + .set_base_branch(project_id, &"refs/remotes/origin/master".parse().unwrap()) + .await + .unwrap(); + + let branch_id = controller + .create_virtual_branch(project_id, &branch::BranchCreateRequest::default()) + .await + .unwrap(); + + // create commit + fs::write(repository.path().join("file.txt"), "content").unwrap(); + let _commit1_id = controller + .create_commit(project_id, &branch_id, "commit one", None, false) + .await + .unwrap(); + + // create commit + fs::write(repository.path().join("file2.txt"), "content2").unwrap(); + fs::write(repository.path().join("file3.txt"), "content3").unwrap(); + let commit2_id = controller + .create_commit(project_id, &branch_id, "commit two", None, false) + .await + .unwrap(); + + // create commit + fs::write(repository.path().join("file4.txt"), "content4").unwrap(); + let _commit3_id = controller + .create_commit(project_id, &branch_id, "commit three", None, false) + .await + .unwrap(); + + controller + .insert_blank_commit(project_id, &branch_id, commit2_id, 1) + .await + .unwrap(); + + let branch = controller + .list_virtual_branches(project_id) + .await + .unwrap() + .0 + .into_iter() + .find(|b| b.id == branch_id) + .unwrap(); + + assert_eq!(branch.commits.len(), 4); + assert_eq!(branch.commits[0].files.len(), 1); + assert_eq!(branch.commits[1].files.len(), 2); + assert_eq!(branch.commits[2].files.len(), 0); // blank commit + + let descriptions = branch + .commits + .iter() + .map(|c| c.description.clone()) + .collect::>(); + + assert_eq!( + descriptions, + vec!["commit three", "commit two", "", "commit one"] + ); +} + +#[tokio::test] +async fn insert_blank_commit_up() { + let Test { + repository, + project_id, + controller, + .. + } = &Test::default(); + + controller + .set_base_branch(project_id, &"refs/remotes/origin/master".parse().unwrap()) + .await + .unwrap(); + + let branch_id = controller + .create_virtual_branch(project_id, &branch::BranchCreateRequest::default()) + .await + .unwrap(); + + // create commit + fs::write(repository.path().join("file.txt"), "content").unwrap(); + let _commit1_id = controller + .create_commit(project_id, &branch_id, "commit one", None, false) + .await + .unwrap(); + + // create commit + fs::write(repository.path().join("file2.txt"), "content2").unwrap(); + fs::write(repository.path().join("file3.txt"), "content3").unwrap(); + let commit2_id = controller + .create_commit(project_id, &branch_id, "commit two", None, false) + .await + .unwrap(); + + // create commit + fs::write(repository.path().join("file4.txt"), "content4").unwrap(); + let _commit3_id = controller + .create_commit(project_id, &branch_id, "commit three", None, false) + .await + .unwrap(); + + controller + .insert_blank_commit(project_id, &branch_id, commit2_id, -1) + .await + .unwrap(); + + let branch = controller + .list_virtual_branches(project_id) + .await + .unwrap() + .0 + .into_iter() + .find(|b| b.id == branch_id) + .unwrap(); + + assert_eq!(branch.commits.len(), 4); + assert_eq!(branch.commits[0].files.len(), 1); + assert_eq!(branch.commits[1].files.len(), 0); // blank commit + assert_eq!(branch.commits[2].files.len(), 2); + + let descriptions = branch + .commits + .iter() + .map(|c| c.description.clone()) + .collect::>(); + + assert_eq!( + descriptions, + vec!["commit three", "", "commit two", "commit one"] + ); +} diff --git a/crates/gitbutler-core/tests/suite/virtual_branches/mod.rs b/crates/gitbutler-core/tests/suite/virtual_branches/mod.rs index 464e8c9de..cd211e436 100644 --- a/crates/gitbutler-core/tests/suite/virtual_branches/mod.rs +++ b/crates/gitbutler-core/tests/suite/virtual_branches/mod.rs @@ -57,14 +57,18 @@ mod create_virtual_branch_from_branch; mod delete_virtual_branch; mod fetch_from_target; mod init; +mod insert_blank_commit; +mod move_commit_file; mod move_commit_to_vbranch; mod references; +mod reorder_commit; mod reset_virtual_branch; mod selected_for_changes; mod set_base_branch; mod squash; mod unapply; mod unapply_ownership; +mod undo_commit; mod update_base_branch; mod update_commit_message; mod upstream; diff --git a/crates/gitbutler-core/tests/suite/virtual_branches/move_commit_file.rs b/crates/gitbutler-core/tests/suite/virtual_branches/move_commit_file.rs new file mode 100644 index 000000000..bdb57a685 --- /dev/null +++ b/crates/gitbutler-core/tests/suite/virtual_branches/move_commit_file.rs @@ -0,0 +1,190 @@ +use super::*; + +#[tokio::test] +async fn move_file_down() { + let Test { + repository, + project_id, + controller, + .. + } = &Test::default(); + + controller + .set_base_branch(project_id, &"refs/remotes/origin/master".parse().unwrap()) + .await + .unwrap(); + + let branch_id = controller + .create_virtual_branch(project_id, &branch::BranchCreateRequest::default()) + .await + .unwrap(); + + // create commit + fs::write(repository.path().join("file.txt"), "content").unwrap(); + let commit1_id = controller + .create_commit(project_id, &branch_id, "commit one", None, false) + .await + .unwrap(); + + // create commit + fs::write(repository.path().join("file2.txt"), "content2").unwrap(); + fs::write(repository.path().join("file3.txt"), "content3").unwrap(); + let commit2_id = controller + .create_commit(project_id, &branch_id, "commit two", None, false) + .await + .unwrap(); + + // amend another hunk + let to_amend: branch::BranchOwnershipClaims = "file2.txt:1-2".parse().unwrap(); + controller + .move_commit_file(project_id, &branch_id, commit2_id, commit1_id, &to_amend) + .await + .unwrap(); + + let branch = controller + .list_virtual_branches(project_id) + .await + .unwrap() + .0 + .into_iter() + .find(|b| b.id == branch_id) + .unwrap(); + + assert_eq!(branch.commits.len(), 2); + assert_eq!(branch.commits[0].files.len(), 1); + assert_eq!(branch.commits[1].files.len(), 2); // this now has both file changes +} + +#[tokio::test] +async fn move_file_up() { + let Test { + repository, + project_id, + controller, + .. + } = &Test::default(); + + controller + .set_base_branch(project_id, &"refs/remotes/origin/master".parse().unwrap()) + .await + .unwrap(); + + let branch_id = controller + .create_virtual_branch(project_id, &branch::BranchCreateRequest::default()) + .await + .unwrap(); + + // create commit + fs::write(repository.path().join("file.txt"), "content").unwrap(); + fs::write(repository.path().join("file2.txt"), "content2").unwrap(); + let commit1_id = controller + .create_commit(project_id, &branch_id, "commit one", None, false) + .await + .unwrap(); + + // create commit + fs::write(repository.path().join("file3.txt"), "content3").unwrap(); + let commit2_id = controller + .create_commit(project_id, &branch_id, "commit two", None, false) + .await + .unwrap(); + + // amend another hunk + let to_amend: branch::BranchOwnershipClaims = "file2.txt:1-2".parse().unwrap(); + controller + .move_commit_file(project_id, &branch_id, commit1_id, commit2_id, &to_amend) + .await + .unwrap(); + + let branch = controller + .list_virtual_branches(project_id) + .await + .unwrap() + .0 + .into_iter() + .find(|b| b.id == branch_id) + .unwrap(); + + assert_eq!(branch.commits.len(), 2); + assert_eq!(branch.commits[0].files.len(), 2); // this now has both file changes + assert_eq!(branch.commits[1].files.len(), 1); +} + +// This test is failing because the file is not being moved up to the correct commit +// This is out of scope for the first release, but should be fixed in the future +// where you can take overlapping hunks between commits and resolve a move between them +/* +#[tokio::test] +async fn move_file_up_overlapping_hunks() { + let Test { + repository, + project_id, + controller, + .. + } = &Test::default(); + + controller + .set_base_branch(project_id, &"refs/remotes/origin/master".parse().unwrap()) + .await + .unwrap(); + + let branch_id = controller + .create_virtual_branch(project_id, &branch::BranchCreateRequest::default()) + .await + .unwrap(); + + // create bottom commit + fs::write(repository.path().join("file.txt"), "content").unwrap(); + let _commit1_id = controller + .create_commit(project_id, &branch_id, "commit one", None, false) + .await + .unwrap(); + + // create middle commit one + fs::write(repository.path().join("file2.txt"), "content2\ncontent2a\n").unwrap(); + fs::write(repository.path().join("file3.txt"), "content3").unwrap(); + let commit2_id = controller + .create_commit(project_id, &branch_id, "commit two", None, false) + .await + .unwrap(); + + // create middle commit two + fs::write( + repository.path().join("file2.txt"), + "content2\ncontent2a\ncontent2b\ncontent2c\ncontent2d", + ) + .unwrap(); + fs::write(repository.path().join("file4.txt"), "content4").unwrap(); + let commit3_id = controller + .create_commit(project_id, &branch_id, "commit three", None, false) + .await + .unwrap(); + + // create top commit + fs::write(repository.path().join("file5.txt"), "content5").unwrap(); + let _commit4_id = controller + .create_commit(project_id, &branch_id, "commit four", None, false) + .await + .unwrap(); + + // move one line from middle commit two up to middle commit one + let to_amend: branch::BranchOwnershipClaims = "file2.txt:1-6".parse().unwrap(); + controller + .move_commit_file(project_id, &branch_id, commit2_id, commit3_id, &to_amend) + .await + .unwrap(); + + let branch = controller + .list_virtual_branches(project_id) + .await + .unwrap() + .0 + .into_iter() + .find(|b| b.id == branch_id) + .unwrap(); + + dbg!(&branch.commits); + assert_eq!(branch.commits.len(), 4); + // +} + */ diff --git a/crates/gitbutler-core/tests/suite/virtual_branches/reorder_commit.rs b/crates/gitbutler-core/tests/suite/virtual_branches/reorder_commit.rs new file mode 100644 index 000000000..5a1c8465e --- /dev/null +++ b/crates/gitbutler-core/tests/suite/virtual_branches/reorder_commit.rs @@ -0,0 +1,123 @@ +use super::*; + +#[tokio::test] +async fn reorder_commit_down() { + let Test { + repository, + project_id, + controller, + .. + } = &Test::default(); + + controller + .set_base_branch(project_id, &"refs/remotes/origin/master".parse().unwrap()) + .await + .unwrap(); + + let branch_id = controller + .create_virtual_branch(project_id, &branch::BranchCreateRequest::default()) + .await + .unwrap(); + + // create commit + fs::write(repository.path().join("file.txt"), "content").unwrap(); + let _commit1_id = controller + .create_commit(project_id, &branch_id, "commit one", None, false) + .await + .unwrap(); + + // create commit + fs::write(repository.path().join("file2.txt"), "content2").unwrap(); + fs::write(repository.path().join("file3.txt"), "content3").unwrap(); + let commit2_id = controller + .create_commit(project_id, &branch_id, "commit two", None, false) + .await + .unwrap(); + + controller + .reorder_commit(project_id, &branch_id, commit2_id, 1) + .await + .unwrap(); + + let branch = controller + .list_virtual_branches(project_id) + .await + .unwrap() + .0 + .into_iter() + .find(|b| b.id == branch_id) + .unwrap(); + + assert_eq!(branch.commits.len(), 2); + assert_eq!(branch.commits[0].files.len(), 1); // this now has the 2 file changes + assert_eq!(branch.commits[1].files.len(), 2); // and this has the single file change + + let descriptions = branch + .commits + .iter() + .map(|c| c.description.clone()) + .collect::>(); + + assert_eq!(descriptions, vec!["commit one", "commit two"]); +} + +#[tokio::test] +async fn reorder_commit_up() { + let Test { + repository, + project_id, + controller, + .. + } = &Test::default(); + + controller + .set_base_branch(project_id, &"refs/remotes/origin/master".parse().unwrap()) + .await + .unwrap(); + + let branch_id = controller + .create_virtual_branch(project_id, &branch::BranchCreateRequest::default()) + .await + .unwrap(); + + // create commit + fs::write(repository.path().join("file.txt"), "content").unwrap(); + let commit1_id = controller + .create_commit(project_id, &branch_id, "commit one", None, false) + .await + .unwrap(); + + // create commit + fs::write(repository.path().join("file2.txt"), "content2").unwrap(); + fs::write(repository.path().join("file3.txt"), "content3").unwrap(); + let _commit2_id = controller + .create_commit(project_id, &branch_id, "commit two", None, false) + .await + .unwrap(); + + controller + .reorder_commit(project_id, &branch_id, commit1_id, -1) + .await + .unwrap(); + + let branch = controller + .list_virtual_branches(project_id) + .await + .unwrap() + .0 + .into_iter() + .find(|b| b.id == branch_id) + .unwrap(); + + assert_eq!(branch.commits.len(), 2); + assert_eq!(branch.commits[0].files.len(), 1); // this now has the 2 file changes + assert_eq!(branch.commits[1].files.len(), 2); // and this has the single file change + + let descriptions = branch + .commits + .iter() + .map(|c| c.description.clone()) + .collect::>(); + + assert_eq!(descriptions, vec!["commit one", "commit two"]); +} diff --git a/crates/gitbutler-core/tests/suite/virtual_branches/undo_commit.rs b/crates/gitbutler-core/tests/suite/virtual_branches/undo_commit.rs new file mode 100644 index 000000000..6411adbc8 --- /dev/null +++ b/crates/gitbutler-core/tests/suite/virtual_branches/undo_commit.rs @@ -0,0 +1,71 @@ +use super::*; + +#[tokio::test] +async fn undo_commit_simple() { + let Test { + repository, + project_id, + controller, + .. + } = &Test::default(); + + controller + .set_base_branch(project_id, &"refs/remotes/origin/master".parse().unwrap()) + .await + .unwrap(); + + let branch_id = controller + .create_virtual_branch(project_id, &branch::BranchCreateRequest::default()) + .await + .unwrap(); + + // create commit + fs::write(repository.path().join("file.txt"), "content").unwrap(); + let _commit1_id = controller + .create_commit(project_id, &branch_id, "commit one", None, false) + .await + .unwrap(); + + // create commit + fs::write(repository.path().join("file2.txt"), "content2").unwrap(); + fs::write(repository.path().join("file3.txt"), "content3").unwrap(); + let commit2_id = controller + .create_commit(project_id, &branch_id, "commit two", None, false) + .await + .unwrap(); + + // create commit + fs::write(repository.path().join("file4.txt"), "content4").unwrap(); + let _commit3_id = controller + .create_commit(project_id, &branch_id, "commit three", None, false) + .await + .unwrap(); + + controller + .undo_commit(project_id, &branch_id, commit2_id) + .await + .unwrap(); + + let branch = controller + .list_virtual_branches(project_id) + .await + .unwrap() + .0 + .into_iter() + .find(|b| b.id == branch_id) + .unwrap(); + + // should be two uncommitted files now (file2.txt and file3.txt) + assert_eq!(branch.files.len(), 2); + assert_eq!(branch.commits.len(), 2); + assert_eq!(branch.commits[0].files.len(), 1); + assert_eq!(branch.commits[1].files.len(), 1); + + let descriptions = branch + .commits + .iter() + .map(|c| c.description.clone()) + .collect::>(); + + assert_eq!(descriptions, vec!["commit three", "commit one"]); +} diff --git a/crates/gitbutler-tauri/src/main.rs b/crates/gitbutler-tauri/src/main.rs index 66ddaf9cd..8bc223561 100644 --- a/crates/gitbutler-tauri/src/main.rs +++ b/crates/gitbutler-tauri/src/main.rs @@ -255,6 +255,11 @@ fn main() { virtual_branches::commands::reset_virtual_branch, virtual_branches::commands::cherry_pick_onto_virtual_branch, virtual_branches::commands::amend_virtual_branch, + virtual_branches::commands::move_commit_file, + virtual_branches::commands::undo_commit, + virtual_branches::commands::insert_blank_commit, + virtual_branches::commands::reorder_commit, + virtual_branches::commands::update_commit_message, virtual_branches::commands::list_remote_branches, virtual_branches::commands::get_remote_branch_data, virtual_branches::commands::squash_branch_commit, diff --git a/crates/gitbutler-tauri/src/virtual_branches.rs b/crates/gitbutler-tauri/src/virtual_branches.rs index 457c14929..b1a9d9d43 100644 --- a/crates/gitbutler-tauri/src/virtual_branches.rs +++ b/crates/gitbutler-tauri/src/virtual_branches.rs @@ -350,11 +350,86 @@ pub mod commands { handle: AppHandle, project_id: ProjectId, branch_id: BranchId, + commit_oid: git::Oid, ownership: BranchOwnershipClaims, ) -> Result { let oid = handle .state::() - .amend(&project_id, &branch_id, &ownership) + .amend(&project_id, &branch_id, commit_oid, &ownership) + .await?; + emit_vbranches(&handle, &project_id).await; + Ok(oid) + } + + #[tauri::command(async)] + #[instrument(skip(handle), err(Debug))] + pub async fn move_commit_file( + handle: AppHandle, + project_id: ProjectId, + branch_id: BranchId, + from_commit_oid: git::Oid, + to_commit_oid: git::Oid, + ownership: BranchOwnershipClaims, + ) -> Result { + let oid = handle + .state::() + .move_commit_file( + &project_id, + &branch_id, + from_commit_oid, + to_commit_oid, + &ownership, + ) + .await?; + emit_vbranches(&handle, &project_id).await; + Ok(oid) + } + + #[tauri::command(async)] + #[instrument(skip(handle), err(Debug))] + pub async fn undo_commit( + handle: AppHandle, + project_id: ProjectId, + branch_id: BranchId, + commit_oid: git::Oid, + ) -> Result<(), Error> { + let oid = handle + .state::() + .undo_commit(&project_id, &branch_id, commit_oid) + .await?; + emit_vbranches(&handle, &project_id).await; + Ok(oid) + } + + #[tauri::command(async)] + #[instrument(skip(handle), err(Debug))] + pub async fn insert_blank_commit( + handle: AppHandle, + project_id: ProjectId, + branch_id: BranchId, + commit_oid: git::Oid, + offset: i32, + ) -> Result<(), Error> { + let oid = handle + .state::() + .insert_blank_commit(&project_id, &branch_id, commit_oid, offset) + .await?; + emit_vbranches(&handle, &project_id).await; + Ok(oid) + } + + #[tauri::command(async)] + #[instrument(skip(handle), err(Debug))] + pub async fn reorder_commit( + handle: AppHandle, + project_id: ProjectId, + branch_id: BranchId, + commit_oid: git::Oid, + offset: i32, + ) -> Result<(), Error> { + let oid = handle + .state::() + .reorder_commit(&project_id, &branch_id, commit_oid, offset) .await?; emit_vbranches(&handle, &project_id).await; Ok(oid) @@ -445,6 +520,8 @@ pub mod commands { Ok(()) } + #[tauri::command(async)] + #[instrument(skip(handle), err(Debug))] pub async fn update_commit_message( handle: tauri::AppHandle, project_id: ProjectId,