Merge pull request #2750 from gitbutlerapp/Move-commit-to-vbranch-tests

move commit functionality
This commit is contained in:
Kiril Videlov 2024-02-20 14:55:40 +01:00 committed by GitHub
commit fb56cad857
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 666 additions and 5 deletions

View File

@ -182,6 +182,7 @@ fn main() {
virtual_branches::commands::get_remote_branch_data,
virtual_branches::commands::squash_branch_commit,
virtual_branches::commands::fetch_from_target,
virtual_branches::commands::move_commit,
menu::menu_item_set_enabled,
keys::commands::get_public_key,
github::commands::init_device_oauth,

View File

@ -589,6 +589,34 @@ pub async fn fetch_from_target(
Ok(base_branch)
}
#[tauri::command(async)]
#[instrument(skip(handle))]
pub async fn move_commit(
handle: tauri::AppHandle,
project_id: &str,
commit_oid: &str,
target_branch_id: &str,
) -> Result<(), Error> {
let project_id = project_id.parse().map_err(|_| Error::UserError {
code: Code::Validation,
message: "Malformed project id".into(),
})?;
let commit_oid = commit_oid.parse().map_err(|_| Error::UserError {
code: Code::Validation,
message: "Malformed commit oid".into(),
})?;
let target_branch_id = target_branch_id.parse().map_err(|_| Error::UserError {
code: Code::Validation,
message: "Malformed branch id".into(),
})?;
handle
.state::<Controller>()
.move_commit(&project_id, &target_branch_id, commit_oid)
.await?;
emit_vbranches(&handle, &project_id).await;
Ok(())
}
pub async fn update_commit_message(
handle: tauri::AppHandle,
project_id: &str,

View File

@ -359,6 +359,18 @@ impl Controller {
.fetch_from_target(project_id)
.await
}
pub async fn move_commit(
&self,
project_id: &ProjectId,
target_branch_id: &BranchId,
commit_oid: git::Oid,
) -> Result<(), ControllerError<errors::MoveCommitError>> {
self.inner(project_id)
.await
.move_commit(project_id, target_branch_id, commit_oid)
.await
}
}
#[derive(Clone)]
@ -916,6 +928,37 @@ impl ControllerInner {
Ok(base_branch)
}
pub async fn move_commit(
&self,
project_id: &ProjectId,
target_branch_id: &BranchId,
commit_oid: git::Oid,
) -> Result<(), ControllerError<errors::MoveCommitError>> {
let _permit = self.semaphore.acquire().await;
self.with_verify_branch(project_id, |gb_repository, project_repository, user| {
let signing_key = project_repository
.config()
.sign_commits()
.context("failed to get sign commits option")?
.then(|| {
self.keys
.get_or_create()
.context("failed to get private key")
})
.transpose()?;
super::move_commit(
gb_repository,
project_repository,
target_branch_id,
commit_oid,
user,
signing_key.as_ref(),
)
.map_err(Into::into)
})
}
}
impl ControllerInner {

View File

@ -327,6 +327,7 @@ pub enum SetBaseBranchError {
#[error(transparent)]
Other(#[from] anyhow::Error),
}
#[derive(Debug, thiserror::Error)]
pub enum UpdateBaseBranchError {
#[error("project is in conflicting state")]
@ -337,6 +338,50 @@ pub enum UpdateBaseBranchError {
Other(#[from] anyhow::Error),
}
#[derive(Debug, thiserror::Error)]
pub enum MoveCommitError {
#[error("source branch contains hunks locked to the target commit")]
SourceLocked,
#[error("branch not applied")]
NotApllied,
#[error("project is in conflicted state")]
Conflicted(ProjectConflictError),
#[error("default target not set")]
DefaultTargetNotSet(DefaultTargetNotSetError),
#[error("branch not found")]
BranchNotFound(BranchNotFoundError),
#[error("commit not found")]
CommitNotFound(git::Oid),
#[error(transparent)]
Other(#[from] anyhow::Error),
}
impl From<MoveCommitError> for crate::error::Error {
fn from(value: MoveCommitError) -> Self {
match value {
MoveCommitError::SourceLocked => Error::UserError {
message: "Source branch contains hunks locked to the target commit".to_string(),
code: crate::error::Code::Branches,
},
MoveCommitError::NotApllied => Error::UserError {
message: "Branch not applied".to_string(),
code: crate::error::Code::Branches,
},
MoveCommitError::Conflicted(error) => error.into(),
MoveCommitError::DefaultTargetNotSet(error) => error.into(),
MoveCommitError::BranchNotFound(error) => error.into(),
MoveCommitError::CommitNotFound(oid) => Error::UserError {
message: format!("Commit {} not found", oid),
code: crate::error::Code::Branches,
},
MoveCommitError::Other(error) => {
tracing::error!(?error, "move commit to vbranch error");
Error::Unknown
}
}
}
}
#[derive(Debug, thiserror::Error)]
pub enum CreateVirtualBranchFromBranchError {
#[error("failed to apply")]

View File

@ -3481,6 +3481,194 @@ pub fn update_commit_message(
Ok(())
}
/// moves commit on top of the to target branch
pub fn move_commit(
gb_repository: &gb_repository::Repository,
project_repository: &project_repository::Repository,
target_branch_id: &BranchId,
commit_oid: git::Oid,
user: Option<&users::User>,
signing_key: Option<&keys::PrivateKey>,
) -> Result<(), errors::MoveCommitError> {
if project_repository.is_resolving() {
return Err(errors::MoveCommitError::Conflicted(
errors::ProjectConflictError {
project_id: project_repository.project().id,
},
));
}
let latest_session = gb_repository
.get_latest_session()
.context("failed to get or create current session")?
.ok_or_else(|| {
errors::MoveCommitError::DefaultTargetNotSet(errors::DefaultTargetNotSetError {
project_id: project_repository.project().id,
})
})?;
let latest_session_reader = sessions::Reader::open(gb_repository, &latest_session)
.context("failed to open current session")?;
let applied_branches = Iterator::new(&latest_session_reader)
.context("failed to create branch iterator")?
.collect::<Result<Vec<branch::Branch>, reader::Error>>()
.context("failed to read virtual branches")?
.into_iter()
.filter(|b| b.applied)
.collect::<Vec<_>>();
if !applied_branches.iter().any(|b| b.id == *target_branch_id) {
return Err(errors::MoveCommitError::BranchNotFound(
errors::BranchNotFoundError {
project_id: project_repository.project().id,
branch_id: *target_branch_id,
},
));
}
let default_target = super::get_default_target(&latest_session_reader)
.context("failed to get default target")?
.ok_or_else(|| {
errors::MoveCommitError::DefaultTargetNotSet(errors::DefaultTargetNotSetError {
project_id: project_repository.project().id,
})
})?;
let mut applied_statuses = get_applied_status(
gb_repository,
project_repository,
&default_target,
applied_branches,
)?;
let (ref mut source_branch, source_status) = applied_statuses
.iter_mut()
.find(|(b, _)| b.head == commit_oid)
.ok_or_else(|| errors::MoveCommitError::CommitNotFound(commit_oid))?;
let source_branch_non_comitted_files = calculate_non_commited_diffs(
project_repository,
source_branch,
&default_target,
source_status,
)?;
let source_branch_head = project_repository
.git_repository
.find_commit(commit_oid)
.context("failed to find commit")?;
let source_branch_head_parent = source_branch_head
.parent(0)
.context("failed to get parent commit")?;
let source_branch_head_tree = source_branch_head
.tree()
.context("failed to get commit tree")?;
let source_branch_head_parent_tree = source_branch_head_parent
.tree()
.context("failed to get parent tree")?;
let branch_head_diff = diff::trees(
&project_repository.git_repository,
&source_branch_head_parent_tree,
&source_branch_head_tree,
)?;
let is_source_locked = source_branch_non_comitted_files
.iter()
.any(|(path, hunks)| {
branch_head_diff.get(path).map_or(false, |head_diff_hunks| {
hunks.iter().any(|hunk| {
head_diff_hunks.iter().any(|head_hunk| {
joined(
head_hunk.new_start,
head_hunk.new_start + head_hunk.new_lines,
hunk.new_start,
hunk.new_start + hunk.new_lines,
)
})
})
})
});
if is_source_locked {
return Err(errors::MoveCommitError::SourceLocked);
}
let branch_writer = branch::Writer::new(gb_repository).context("failed to create writer")?;
let branch_reader = branch::Reader::new(&latest_session_reader);
// move files ownerships from source branch to the destination branch
let ownerships_to_transfer = branch_head_diff
.iter()
.map(|(file_path, hunks)| {
(
file_path.clone(),
hunks.iter().map(Into::into).collect::<Vec<_>>(),
)
})
.map(|(file_path, hunks)| FileOwnership { file_path, hunks })
.flat_map(|file_ownership| source_branch.ownership.take(&file_ownership))
.collect::<Vec<_>>();
// reset the source branch to the parent commit
{
source_branch.head = source_branch_head_parent.id();
branch_writer.write(source_branch)?;
}
// move the commit to destination branch target branch
{
let mut destination_branch =
branch_reader
.read(target_branch_id)
.map_err(|error| match error {
reader::Error::NotFound => {
errors::MoveCommitError::BranchNotFound(errors::BranchNotFoundError {
project_id: project_repository.project().id,
branch_id: *target_branch_id,
})
}
error => errors::MoveCommitError::Other(error.into()),
})?;
for ownership in ownerships_to_transfer {
destination_branch.ownership.put(&ownership);
}
let new_destination_tree_oid = write_tree_onto_commit(
project_repository,
destination_branch.head,
&branch_head_diff,
)
.context("failed to write tree onto commit")?;
let new_destination_tree = project_repository
.git_repository
.find_tree(new_destination_tree_oid)
.context("failed to find tree")?;
let new_destination_head_oid = project_repository
.commit(
user,
source_branch_head.message().unwrap_or_default(),
&new_destination_tree,
&[&project_repository
.git_repository
.find_commit(destination_branch.head)
.context("failed to get dst branch head commit")?],
signing_key,
)
.context("failed to commit")?;
destination_branch.head = new_destination_head_oid;
branch_writer.write(&mut destination_branch)?;
}
super::integration::update_gitbutler_integration(gb_repository, project_repository)
.context("failed to update gitbutler integration")?;
Ok(())
}
pub fn create_virtual_branch_from_branch(
gb_repository: &gb_repository::Repository,
project_repository: &project_repository::Repository,

View File

@ -6185,3 +6185,322 @@ mod selected_for_changes {
assert!(branches[0].selected_for_changes);
}
}
mod move_commit_to_vbranch {
use gblib::virtual_branches::BranchId;
use super::*;
#[tokio::test]
async fn no_diffs() {
let Test {
repository,
project_id,
controller,
..
} = Test::default();
controller
.set_base_branch(&project_id, &"refs/remotes/origin/master".parse().unwrap())
.await
.unwrap();
std::fs::write(repository.path().join("file.txt"), "content").unwrap();
let branches = controller.list_virtual_branches(&project_id).await.unwrap();
assert_eq!(branches.len(), 1);
let source_branch_id = branches[0].id;
let commit_oid = controller
.create_commit(&project_id, &source_branch_id, "commit", None, false)
.await
.unwrap();
let target_branch_id = controller
.create_virtual_branch(&project_id, &branch::BranchCreateRequest::default())
.await
.unwrap();
controller
.move_commit(&project_id, &target_branch_id, commit_oid)
.await
.unwrap();
let destination_branch = controller
.list_virtual_branches(&project_id)
.await
.unwrap()
.into_iter()
.find(|b| b.id == target_branch_id)
.unwrap();
let source_branch = controller
.list_virtual_branches(&project_id)
.await
.unwrap()
.into_iter()
.find(|b| b.id == source_branch_id)
.unwrap();
assert_eq!(destination_branch.commits.len(), 1);
assert_eq!(destination_branch.files.len(), 0);
assert_eq!(source_branch.commits.len(), 0);
assert_eq!(source_branch.files.len(), 0);
}
#[tokio::test]
async fn diffs_on_source_branch() {
let Test {
repository,
project_id,
controller,
..
} = Test::default();
controller
.set_base_branch(&project_id, &"refs/remotes/origin/master".parse().unwrap())
.await
.unwrap();
std::fs::write(repository.path().join("file.txt"), "content").unwrap();
let branches = controller.list_virtual_branches(&project_id).await.unwrap();
assert_eq!(branches.len(), 1);
let source_branch_id = branches[0].id;
let commit_oid = controller
.create_commit(&project_id, &source_branch_id, "commit", None, false)
.await
.unwrap();
std::fs::write(
repository.path().join("another file.txt"),
"another content",
)
.unwrap();
let target_branch_id = controller
.create_virtual_branch(&project_id, &branch::BranchCreateRequest::default())
.await
.unwrap();
controller
.move_commit(&project_id, &target_branch_id, commit_oid)
.await
.unwrap();
let destination_branch = controller
.list_virtual_branches(&project_id)
.await
.unwrap()
.into_iter()
.find(|b| b.id == target_branch_id)
.unwrap();
let source_branch = controller
.list_virtual_branches(&project_id)
.await
.unwrap()
.into_iter()
.find(|b| b.id == source_branch_id)
.unwrap();
assert_eq!(destination_branch.commits.len(), 1);
assert_eq!(destination_branch.files.len(), 0);
assert_eq!(source_branch.commits.len(), 0);
assert_eq!(source_branch.files.len(), 1);
}
#[tokio::test]
async fn diffs_on_target_branch() {
let Test {
repository,
project_id,
controller,
..
} = Test::default();
controller
.set_base_branch(&project_id, &"refs/remotes/origin/master".parse().unwrap())
.await
.unwrap();
std::fs::write(repository.path().join("file.txt"), "content").unwrap();
let branches = controller.list_virtual_branches(&project_id).await.unwrap();
assert_eq!(branches.len(), 1);
let source_branch_id = branches[0].id;
let commit_oid = controller
.create_commit(&project_id, &source_branch_id, "commit", None, false)
.await
.unwrap();
let target_branch_id = controller
.create_virtual_branch(
&project_id,
&branch::BranchCreateRequest {
selected_for_changes: Some(true),
..Default::default()
},
)
.await
.unwrap();
std::fs::write(
repository.path().join("another file.txt"),
"another content",
)
.unwrap();
controller
.move_commit(&project_id, &target_branch_id, commit_oid)
.await
.unwrap();
let destination_branch = controller
.list_virtual_branches(&project_id)
.await
.unwrap()
.into_iter()
.find(|b| b.id == target_branch_id)
.unwrap();
let source_branch = controller
.list_virtual_branches(&project_id)
.await
.unwrap()
.into_iter()
.find(|b| b.id == source_branch_id)
.unwrap();
assert_eq!(destination_branch.commits.len(), 1);
assert_eq!(destination_branch.files.len(), 1);
assert_eq!(source_branch.commits.len(), 0);
assert_eq!(source_branch.files.len(), 0);
}
#[tokio::test]
async fn locked_hunks_on_source_branch() {
let Test {
repository,
project_id,
controller,
..
} = Test::default();
controller
.set_base_branch(&project_id, &"refs/remotes/origin/master".parse().unwrap())
.await
.unwrap();
std::fs::write(repository.path().join("file.txt"), "content").unwrap();
let branches = controller.list_virtual_branches(&project_id).await.unwrap();
assert_eq!(branches.len(), 1);
let source_branch_id = branches[0].id;
let commit_oid = controller
.create_commit(&project_id, &source_branch_id, "commit", None, false)
.await
.unwrap();
std::fs::write(repository.path().join("file.txt"), "locked content").unwrap();
let target_branch_id = controller
.create_virtual_branch(&project_id, &branch::BranchCreateRequest::default())
.await
.unwrap();
assert!(matches!(
controller
.move_commit(&project_id, &target_branch_id, commit_oid)
.await
.unwrap_err(),
ControllerError::Action(errors::MoveCommitError::SourceLocked)
));
}
#[tokio::test]
async fn no_commit() {
let Test {
repository,
project_id,
controller,
..
} = Test::default();
controller
.set_base_branch(&project_id, &"refs/remotes/origin/master".parse().unwrap())
.await
.unwrap();
std::fs::write(repository.path().join("file.txt"), "content").unwrap();
let branches = controller.list_virtual_branches(&project_id).await.unwrap();
assert_eq!(branches.len(), 1);
let source_branch_id = branches[0].id;
controller
.create_commit(&project_id, &source_branch_id, "commit", None, false)
.await
.unwrap();
let target_branch_id = controller
.create_virtual_branch(&project_id, &branch::BranchCreateRequest::default())
.await
.unwrap();
assert!(matches!(
controller
.move_commit(
&project_id,
&target_branch_id,
git::Oid::from_str("a99c95cca7a60f1a2180c2f86fb18af97333c192").unwrap()
)
.await
.unwrap_err(),
ControllerError::Action(errors::MoveCommitError::CommitNotFound(_))
));
}
#[tokio::test]
async fn no_branch() {
let Test {
repository,
project_id,
controller,
..
} = Test::default();
controller
.set_base_branch(&project_id, &"refs/remotes/origin/master".parse().unwrap())
.await
.unwrap();
std::fs::write(repository.path().join("file.txt"), "content").unwrap();
let branches = controller.list_virtual_branches(&project_id).await.unwrap();
assert_eq!(branches.len(), 1);
let source_branch_id = branches[0].id;
let commit_oid = controller
.create_commit(&project_id, &source_branch_id, "commit", None, false)
.await
.unwrap();
assert!(matches!(
controller
.move_commit(&project_id, &BranchId::generate(), commit_oid)
.await
.unwrap_err(),
ControllerError::Action(errors::MoveCommitError::BranchNotFound(_))
));
}
}

View File

@ -6,12 +6,14 @@
import DropzoneOverlay from './DropzoneOverlay.svelte';
import ImgThemed from '$lib/components/ImgThemed.svelte';
import Resizer from '$lib/components/Resizer.svelte';
import { projectAiGenEnabled } from '$lib/config/config';
import { projectAiGenAutoBranchNamingEnabled } from '$lib/config/config';
import { projectAiGenEnabled } from '$lib/config/config';
import {
isDraggableCommit,
isDraggableFile,
isDraggableHunk,
isDraggableRemoteCommit,
type DraggableCommit,
type DraggableFile,
type DraggableHunk,
type DraggableRemoteCommit
@ -95,6 +97,13 @@
laneWidth = lscache.get(laneWidthKey + branch.id);
});
function acceptMoveCommit(data: any) {
return isDraggableCommit(data) && data.branchId != branch.id && data.isHeadCommit;
}
function onCommitDrop(data: DraggableCommit) {
branchController.moveCommit(branch.id, data.commit.id);
}
function acceptCherrypick(data: any) {
return isDraggableRemoteCommit(data) && data.branchId == branch.id;
}
@ -181,10 +190,18 @@
/>
<!-- DROPZONES -->
<DropzoneOverlay class="cherrypick-dz-marker" label="Apply here" />
<DropzoneOverlay class="cherrypick-dz-marker" label="Apply here" />
<DropzoneOverlay class="lane-dz-marker" label="Move here" />
<div
class="branch-card__dropzone-wrapper"
use:dropzone={{
hover: 'move-commit-dz-hover',
active: 'move-commit-dz-active',
accepts: acceptMoveCommit,
onDrop: onCommitDrop,
disabled: isUnapplied
}}
use:dropzone={{
hover: 'cherrypick-dz-hover',
active: 'cherrypick-dz-active',
@ -202,6 +219,8 @@
>
<DropzoneOverlay class="cherrypick-dz-marker" label="Apply here" />
<DropzoneOverlay class="lane-dz-marker" label="Move here" />
<DropzoneOverlay class="move-commit-dz-marker" label="Move here" />
{#if branch.files?.length > 0}
<div class="card">
{#if branch.active && branch.conflicted}
@ -428,6 +447,11 @@
@apply flex;
}
/* move commit drop zone */
:global(.move-commit-dz-active .move-commit-dz-marker) {
@apply flex;
}
/* squash drop zone */
:global(.squash-dz-active .squash-dz-marker) {
@apply flex;

View File

@ -45,7 +45,7 @@
<div
use:draggable={commit instanceof Commit
? draggableCommit(commit.branchId, commit)
? draggableCommit(commit.branchId, commit, isHeadCommit)
: nonDraggable()}
class="commit"
class:is-commit-open={showFiles}

View File

@ -38,14 +38,15 @@ export function isDraggableFile(obj: any): obj is DraggableFile {
export type DraggableCommit = {
branchId: string;
commit: Commit;
isHeadCommit: boolean;
};
export function draggableCommit(branchId: string, commit: Commit) {
return { data: { branchId, commit } };
export function draggableCommit(branchId: string, commit: Commit, isHeadCommit: boolean) {
return { data: { branchId, commit, isHeadCommit } };
}
export function isDraggableCommit(obj: any): obj is DraggableCommit {
return obj && obj.branchId && obj.commit;
return obj && obj.branchId && obj.commit && obj.isHeadCommit;
}
export type DraggableRemoteCommit = {

View File

@ -280,4 +280,16 @@ export class BranchController {
toasts.error(`Failed to amend commit: ${err.message}`);
}
}
async moveCommit(targetBranchId: string, commitOid: string) {
try {
await invoke<void>('move_commit', {
projectId: this.projectId,
targetBranchId,
commitOid
});
} catch (err: any) {
toasts.error(`Failed to move commit: ${err.message}`);
}
}
}