refactor cherry picking

This commit is contained in:
Nikita Galaiko 2023-10-31 08:45:53 +01:00 committed by GitButler
parent b773901683
commit 97d6be48f0
5 changed files with 245 additions and 92 deletions

View File

@ -197,6 +197,13 @@ impl Repository {
self.0.blob_path(path).map(Into::into).map_err(Into::into)
}
pub fn cherry_pick(&self, base: &Commit, target: &Commit) -> Result<Index> {
self.0
.cherrypick_commit(target.into(), base.into(), 0, None)
.map(Into::into)
.map_err(Into::into)
}
pub fn blob(&self, data: &[u8]) -> Result<Oid> {
self.0.blob(data).map(Into::into).map_err(Into::into)
}

View File

@ -84,18 +84,23 @@ pub fn conflicting_files(repository: &Repository) -> Result<Option<Vec<String>>>
pub fn is_conflicting(repository: &Repository, path: Option<&str>) -> Result<bool> {
let conflicts_path = repository.git_repository.path().join("conflicts");
if !conflicts_path.exists() {
return Ok(false);
}
let file = std::fs::File::open(conflicts_path)?;
let reader = std::io::BufReader::new(file);
let files = reader.lines().flatten().collect::<Vec<_>>();
if let Some(pathname) = path {
// check if pathname is one of the lines in conflicts_path file
let file = std::fs::File::open(conflicts_path)?;
let reader = std::io::BufReader::new(file);
for line in reader.lines().flatten() {
for line in files {
if line == pathname {
return Ok(true);
}
}
Ok(false)
} else {
Ok(conflicts_path.exists())
Ok(!files.is_empty())
}
}

View File

@ -2397,23 +2397,17 @@ pub fn cherry_pick(
.git_repository
.find_commit(target_commit_oid)
.context("failed to find target commit")?;
let target_commit_tree = target_commit
.tree()
.context("failed to find target commit tree")?;
let branch_head_commit = project_repository
.git_repository
.find_commit(branch.head)
.context("failed to find branch head commit")?;
let branch_tree = project_repository
.git_repository
.find_tree(branch.tree)
.context("failed to find branch tree")?;
let default_target = get_default_target(&current_session_reader)
.context("failed to read default target")?
.context("no default target set")?;
// if any other branches are applied, unapply them
let applied_branches = Iterator::new(&current_session_reader)
.context("failed to create branch iterator")?
.collect::<Result<Vec<branch::Branch>, reader::Error>>()
@ -2426,44 +2420,58 @@ pub fn cherry_pick(
gb_repository,
project_repository,
&default_target,
applied_branches.clone(),
)
.context("failed to get status by branch")?;
applied_branches,
)?;
let files = applied_statuses
let branch_files = applied_statuses
.iter()
.find_map(|(b, f)| (b.id == *branch_id).then_some(f))
.find(|(b, _)| b.id == *branch_id)
.map(|(_, f)| f)
.context("branch status not found")?;
let wip_branch_tree_oid = write_tree_onto_commit(project_repository, branch.head, files)?;
let wip_branch_tree = project_repository
// create a wip commit. we'll use it to offload cherrypick conflicts calculation to libgit.
let wip_commit = {
let wip_tree_oid = write_tree(project_repository, &default_target, branch_files)?;
let wip_tree = project_repository.git_repository.find_tree(wip_tree_oid)?;
let oid = project_repository.git_repository.commit(
None,
&git::Signature::now("GitButler", "gitbutler@gitbutler.com")?,
&git::Signature::now("GitButler", "gitbutler@gitbutler.com")?,
"wip cherry picking commit",
&wip_tree,
&[&branch_head_commit],
)?;
project_repository.git_repository.find_commit(oid)?
};
let mut cherrypick_index = project_repository
.git_repository
.find_tree(wip_branch_tree_oid)
.context("failed to find wip branch tree")?;
.cherry_pick(&wip_commit, &target_commit)
.context("failed to cherry pick")?;
let mut merge_index = project_repository
.git_repository
.merge_trees(&branch_tree, &wip_branch_tree, &target_commit_tree)
.context("failed to merge trees")?;
let commit_oid = if merge_index.has_conflicts() {
// unapply other branches
for other_branch in applied_branches.iter().filter(|b| b.id != branch.id) {
unapply_branch(gb_repository, project_repository, &other_branch.id)
.context("failed to unapply branch")?;
}
// unapply other branches
for other_branch in applied_statuses
.iter()
.filter(|(b, _)| b.id != branch.id)
.map(|(b, _)| b)
{
unapply_branch(gb_repository, project_repository, &other_branch.id)
.context("failed to unapply branch")?;
}
let commit_oid = if cherrypick_index.has_conflicts() {
// checkout the conflicts
project_repository
.git_repository
.checkout_index(&mut merge_index)
.checkout_index(&mut cherrypick_index)
.allow_conflicts()
.conflict_style_merge()
.force()
.checkout()?;
// mark conflicts
let conflicts = merge_index.conflicts()?;
let conflicts = cherrypick_index.conflicts()?;
let mut merge_conflicts = Vec::new();
for path in conflicts.flatten() {
if let Some(ours) = path.our {
@ -2479,7 +2487,7 @@ pub fn cherry_pick(
None
} else {
let merge_tree_oid = merge_index
let merge_tree_oid = cherrypick_index
.write_tree_to(&project_repository.git_repository)
.context("failed to write merge tree")?;
let merge_tree = project_repository
@ -2487,6 +2495,11 @@ pub fn cherry_pick(
.find_tree(merge_tree_oid)
.context("failed to find merge tree")?;
let branch_head_commit = project_repository
.git_repository
.find_commit(branch.head)
.context("failed to find branch head commit")?;
let commit_oid = project_repository
.git_repository
.commit(
@ -2499,34 +2512,10 @@ pub fn cherry_pick(
)
.context("failed to create commit")?;
// go through the other applied branches and merge them into the final tree
let final_tree = applied_statuses
.into_iter()
.filter(|(b, _)| b.id != *branch_id)
.fold(
target_commit.tree().context("failed to get target tree"),
|final_tree, status| {
let final_tree = final_tree?;
let tree_oid = write_tree(project_repository, &default_target, &status.1)?;
let branch_tree = project_repository.git_repository.find_tree(tree_oid)?;
let mut result = project_repository.git_repository.merge_trees(
&target_commit_tree,
&final_tree,
&branch_tree,
)?;
let final_tree_oid =
result.write_tree_to(&project_repository.git_repository)?;
project_repository
.git_repository
.find_tree(final_tree_oid)
.context("failed to find tree")
},
)?;
// checkout final_tree into the working directory
project_repository
.git_repository
.checkout_tree(&final_tree)
.checkout_tree(&merge_tree)
.force()
.remove_untracked()
.checkout()?;

View File

@ -75,8 +75,23 @@ impl TestProject {
.unwrap();
}
pub fn reset_hard(&self, oid: git::Oid) {
let commit = self.local_repository.find_commit(oid).unwrap();
/// git add -A
/// git reset --hard
pub fn reset_hard(&self, oid: Option<git::Oid>) {
let mut index = self.local_repository.index().expect("failed to get index");
index
.add_all(["."], git2::IndexAddOption::DEFAULT, None)
.expect("failed to add all");
index.write().expect("failed to write index");
let commit = oid.map_or(
self.local_repository
.head()
.unwrap()
.peel_to_commit()
.unwrap(),
|oid| self.local_repository.find_commit(oid).unwrap(),
);
self.local_repository
.reset(&commit, git2::ResetType::Hard, None)
.unwrap();

View File

@ -586,7 +586,7 @@ mod conflicts {
fs::write(repository.path().join("file.txt"), "two").unwrap();
repository.commit_all("second");
repository.push();
repository.reset_hard(first_commit_oid);
repository.reset_hard(Some(first_commit_oid));
}
controller
@ -708,7 +708,7 @@ mod conflicts {
fs::write(repository.path().join("file.txt"), "").unwrap();
repository.commit_all("second");
repository.push();
repository.reset_hard(first_commit_oid);
repository.reset_hard(Some(first_commit_oid));
}
controller
@ -811,7 +811,7 @@ mod conflicts {
fs::write(repository.path().join("file.txt"), "second").unwrap();
repository.commit_all("second");
repository.push();
repository.reset_hard(first_commit_oid);
repository.reset_hard(Some(first_commit_oid));
}
controller
@ -1334,15 +1334,79 @@ mod cherry_pick {
controller,
} = Test::default();
let commit_oid = {
let first = repository.commit_all("commit");
controller
.set_base_branch(
&project_id,
&git::RemoteBranchName::from_str("refs/remotes/origin/master").unwrap(),
)
.unwrap();
let branch_id = controller
.create_virtual_branch(&project_id, &branch::BranchCreateRequest::default())
.await
.unwrap();
let commit_one = {
fs::write(repository.path().join("file.txt"), "content").unwrap();
let second = repository.commit_all("commit");
repository.push();
repository.reset_hard(first);
second
controller
.create_commit(&project_id, &branch_id, "commit", None)
.await
.unwrap()
};
let commit_two = {
fs::write(repository.path().join("file.txt"), "content two").unwrap();
controller
.create_commit(&project_id, &branch_id, "commit", None)
.await
.unwrap()
};
controller
.push_virtual_branch(&project_id, &branch_id, false)
.await
.unwrap();
controller
.reset_virtual_branch(&project_id, &branch_id, commit_one)
.await
.unwrap();
repository.reset_hard(None);
assert_eq!(
fs::read_to_string(repository.path().join("file.txt")).unwrap(),
"content"
);
let cherry_picked_commit_oid = controller
.cherry_pick(&project_id, &branch_id, commit_two)
.await
.unwrap();
assert!(cherry_picked_commit_oid.is_some());
assert!(repository.path().join("file.txt").exists());
assert_eq!(
fs::read_to_string(repository.path().join("file.txt")).unwrap(),
"content two"
);
let branches = controller.list_virtual_branches(&project_id).await.unwrap();
assert_eq!(branches.len(), 1);
assert_eq!(branches[0].id, branch_id);
assert!(branches[0].active);
assert_eq!(branches[0].commits.len(), 2);
assert_eq!(branches[0].commits[0].id, cherry_picked_commit_oid.unwrap());
assert_eq!(branches[0].commits[1].id, commit_one);
}
#[tokio::test]
async fn to_different_branch() {
let Test {
repository,
project_id,
controller,
} = Test::default();
controller
.set_base_branch(
&project_id,
@ -1355,19 +1419,68 @@ mod cherry_pick {
.await
.unwrap();
let commit_one = {
fs::write(repository.path().join("file.txt"), "content").unwrap();
controller
.create_commit(&project_id, &branch_id, "commit", None)
.await
.unwrap()
};
let commit_two = {
fs::write(repository.path().join("file_two.txt"), "content two").unwrap();
controller
.create_commit(&project_id, &branch_id, "commit", None)
.await
.unwrap()
};
controller
.push_virtual_branch(&project_id, &branch_id, false)
.await
.unwrap();
controller
.reset_virtual_branch(&project_id, &branch_id, commit_one)
.await
.unwrap();
repository.reset_hard(None);
assert_eq!(
fs::read_to_string(repository.path().join("file.txt")).unwrap(),
"content"
);
assert!(!repository.path().join("file_two.txt").exists());
let branch_two_id = controller
.create_virtual_branch(&project_id, &branch::BranchCreateRequest::default())
.await
.unwrap();
let cherry_picked_commit_oid = controller
.cherry_pick(&project_id, &branch_id, commit_oid)
.cherry_pick(&project_id, &branch_two_id, commit_two)
.await
.unwrap();
assert!(cherry_picked_commit_oid.is_some());
assert!(repository.path().join("file.txt").exists());
let branches = controller.list_virtual_branches(&project_id).await.unwrap();
assert_eq!(branches.len(), 1);
assert!(repository.path().join("file_two.txt").exists());
assert_eq!(
fs::read_to_string(repository.path().join("file_two.txt")).unwrap(),
"content two"
);
assert_eq!(branches.len(), 2);
assert_eq!(branches[0].id, branch_id);
assert!(branches[0].active);
assert!(!branches[0].active);
assert_eq!(branches[0].commits.len(), 1);
assert_eq!(branches[0].commits[0].id, cherry_picked_commit_oid.unwrap());
assert_eq!(branches[0].commits[0].id, commit_one);
assert_eq!(branches[1].id, branch_two_id);
assert!(branches[1].active);
assert_eq!(branches[1].commits.len(), 1);
assert_eq!(branches[1].commits[0].id, cherry_picked_commit_oid.unwrap());
}
#[tokio::test]
@ -1444,15 +1557,6 @@ mod cherry_pick {
controller,
} = Test::default();
let commit_oid = {
let first = repository.commit_all("commit");
fs::write(repository.path().join("file.txt"), "content").unwrap();
let second = repository.commit_all("commit");
repository.push();
repository.reset_hard(first);
second
};
controller
.set_base_branch(
&project_id,
@ -1465,20 +1569,53 @@ mod cherry_pick {
.await
.unwrap();
let commit_one = {
fs::write(repository.path().join("file.txt"), "content").unwrap();
controller
.create_commit(&project_id, &branch_id, "commit", None)
.await
.unwrap()
};
let commit_two = {
fs::write(repository.path().join("file_two.txt"), "content two").unwrap();
controller
.create_commit(&project_id, &branch_id, "commit", None)
.await
.unwrap()
};
controller
.push_virtual_branch(&project_id, &branch_id, false)
.await
.unwrap();
controller
.reset_virtual_branch(&project_id, &branch_id, commit_one)
.await
.unwrap();
repository.reset_hard(None);
assert_eq!(
fs::read_to_string(repository.path().join("file.txt")).unwrap(),
"content"
);
assert!(!repository.path().join("file_two.txt").exists(),);
// introduce conflict with the remote commit
fs::write(repository.path().join("file.txt"), "conflict").unwrap();
fs::write(repository.path().join("file_two.txt"), "conflict").unwrap();
{
// cherry picking leads to conflict
let cherry_picked_commit_oid = controller
.cherry_pick(&project_id, &branch_id, commit_oid)
.cherry_pick(&project_id, &branch_id, commit_two)
.await
.unwrap();
assert!(cherry_picked_commit_oid.is_none());
assert_eq!(
fs::read_to_string(repository.path().join("file.txt")).unwrap(),
"<<<<<<< ours\nconflict\n=======\ncontent\n>>>>>>> theirs\n"
fs::read_to_string(repository.path().join("file_two.txt")).unwrap(),
"<<<<<<< ours\nconflict\n=======\ncontent two\n>>>>>>> theirs\n"
);
let branches = controller.list_virtual_branches(&project_id).await.unwrap();
@ -1488,12 +1625,12 @@ mod cherry_pick {
assert!(branches[0].conflicted);
assert_eq!(branches[0].files.len(), 1);
assert!(branches[0].files[0].conflicted);
assert_eq!(branches[0].commits.len(), 0);
assert_eq!(branches[0].commits.len(), 1);
}
{
// conflict can be resolved
fs::write(repository.path().join("file.txt"), "resolved").unwrap();
fs::write(repository.path().join("file_two.txt"), "resolved").unwrap();
let commited_oid = controller
.create_commit(&project_id, &branch_id, "resolution", None)
.await
@ -1507,7 +1644,7 @@ mod cherry_pick {
assert_eq!(branches[0].id, branch_id);
assert!(branches[0].active);
assert!(!branches[0].conflicted);
assert_eq!(branches[0].commits.len(), 2);
assert_eq!(branches[0].commits.len(), 3);
// resolution commit is there
assert_eq!(branches[0].commits[0].id, commited_oid);
// cherry picked commit is there
@ -1531,7 +1668,7 @@ mod cherry_pick {
fs::write(repository.path().join("file.txt"), "content").unwrap();
let second = repository.commit_all("commit");
repository.push();
repository.reset_hard(first);
repository.reset_hard(Some(first));
second
};