Move commits: Renamings, copy changes and a small fix

- Update the variable names so that they convey the right things that we're moving around.
- Correct the thrown error messages
- Append the source commit to the list of commits we're testing against the source commit diffs.
This commit is contained in:
estib 2024-10-11 16:26:26 +02:00
parent f71187ed05
commit c9f2f479e5
2 changed files with 98 additions and 38 deletions

View File

@ -49,17 +49,17 @@ pub(crate) fn move_commit(
bail!("Can not move conflicted commits");
}
let source_branch_head_parent = source_commit
let source_commit_parent = source_commit
.parent(0)
.context("failed to get parent commit")?;
let source_branch_head_tree = source_commit.tree().context("failed to get commit tree")?;
let source_branch_head_parent_tree = source_branch_head_parent
let source_commit_tree = source_commit.tree().context("failed to get commit tree")?;
let source_commit_parent_tree = source_commit_parent
.tree()
.context("failed to get parent tree")?;
let branch_head_diff = gitbutler_diff::trees(
let source_commit_diff = gitbutler_diff::trees(
ctx.repository(),
&source_branch_head_parent_tree,
&source_branch_head_tree,
&source_commit_parent_tree,
&source_commit_tree,
)?;
let default_target = vb_state.get_default_target()?;
@ -72,50 +72,50 @@ pub(crate) fn move_commit(
.find_commit(merge_base)
.context("failed to find merge base")?;
let branch_head_diff: HashMap<_, _> =
gitbutler_diff::diff_files_into_hunks(branch_head_diff).collect();
let is_source_locked = check_source_lock(source_branch_non_comitted_files, &branch_head_diff);
let source_commit_diff: HashMap<_, _> =
gitbutler_diff::diff_files_into_hunks(source_commit_diff).collect();
let is_source_locked = check_source_lock(source_branch_non_comitted_files, &source_commit_diff);
let mut ancestor_commits = ctx.repository().log(
source_branch_head_parent.id(),
LogUntil::Commit(merge_base.id()),
)?;
let mut ancestor_commits = ctx
.repository()
.log(source_commit_parent.id(), LogUntil::Commit(merge_base.id()))?;
ancestor_commits.push(merge_base);
let ancestor_commits = ancestor_commits;
let mut child_commits = None;
let mut descendant_commits = None;
if !is_head_commit {
child_commits = Some(
descendant_commits = Some(
ctx.repository()
.log(source_branch.head(), LogUntil::Commit(commit_id))
.unwrap(),
.log(source_branch.head(), LogUntil::Commit(commit_id))?,
);
}
let child_commits = child_commits;
let is_ancestor_locked =
check_source_lock_to_commits(ctx.repository(), &ancestor_commits, &branch_head_diff);
check_source_lock_to_commits(ctx.repository(), &ancestor_commits, &source_commit_diff);
if is_source_locked {
bail!("the source branch contains hunks locked to the target commit")
}
if is_ancestor_locked {
bail!("the source branch contains hunks locked to the target commit ancestors")
bail!("the target commit contains hunks locked to its ancestors")
}
if let Some(child_commits) = child_commits.as_ref() {
let is_parent_locked =
check_source_lock_to_commits(ctx.repository(), child_commits, &branch_head_diff);
if let Some(commits_to_check) = descendant_commits.as_mut() {
// we append the source commit so that we can create the diff between
// the source commit and its first descendant
commits_to_check.push(source_commit.clone());
let is_descendant_locked =
check_source_lock_to_commits(ctx.repository(), commits_to_check, &source_commit_diff);
if is_parent_locked {
bail!("the source branch contains hunks locked to the target commit children")
if is_descendant_locked {
bail!("the target commit contains hunks locked to its descendants")
}
}
// move files ownerships from source branch to the destination branch
let ownerships_to_transfer = branch_head_diff
let ownerships_to_transfer = source_commit_diff
.iter()
.map(|(file_path, hunks)| {
(
@ -143,14 +143,11 @@ pub(crate) fn move_commit(
// if the source commit has children, move them to the source commit's parent
let mut new_source_head_oid = source_branch_head_parent.id();
if let Some(child_commits) = child_commits.as_ref() {
let mut new_source_head_oid = source_commit_parent.id();
if let Some(child_commits) = descendant_commits.as_ref() {
let ids_to_rebase: Vec<git2::Oid> = child_commits.iter().map(|c| c.id()).collect();
new_source_head_oid = cherry_rebase_group(
ctx.repository(),
source_branch_head_parent.id(),
&ids_to_rebase,
)?;
new_source_head_oid =
cherry_rebase_group(ctx.repository(), source_commit_parent.id(), &ids_to_rebase)?;
}
// reset the source branch to the newer parent commit
@ -176,13 +173,13 @@ fn check_source_lock(
let is_source_locked = source_branch_non_comitted_files.iter().any(|file| {
source_commit_diff
.get(&file.path)
.map_or(false, |head_diff_hunks| {
.map_or(false, |source_diff_hunks| {
file.hunks.iter().any(|hunk| {
let hunk: gitbutler_diff::GitHunk = hunk.clone().into();
head_diff_hunks.iter().any(|head_hunk| {
source_diff_hunks.iter().any(|source_hunk| {
lines_overlap(
head_hunk.new_start,
head_hunk.new_start + head_hunk.new_lines,
source_hunk.new_start,
source_hunk.new_start + source_hunk.new_lines,
hunk.new_start,
hunk.new_start + hunk.new_lines,
)
@ -194,6 +191,12 @@ fn check_source_lock(
}
/// determines if the source commit is locked to any commits
///
/// The commits are used to calculate the diffs between them in the following way:
/// - Let A be the source commit and B, C its ancestors.
/// - `source_commit_diff` is the diff between A and B
/// - `commits` is a list of commits [B, C]
/// - This function calculates the diff between B and C check it against the hunks in `source_commit_diff`
fn check_source_lock_to_commits(
repository: &git2::Repository,
commits: &Vec<git2::Commit>,

View File

@ -556,7 +556,64 @@ fn target_commit_locked_to_ancestors() {
assert_eq!(
result.unwrap_err().to_string(),
"the source branch contains hunks locked to the target commit ancestors"
"the target commit contains hunks locked to its ancestors"
);
}
#[test]
fn target_commit_locked_to_descendants() {
let Test {
repository,
project,
..
} = &Test::default();
gitbutler_branch_actions::set_base_branch(
project,
&"refs/remotes/origin/master".parse().unwrap(),
)
.unwrap();
std::fs::write(repository.path().join("a.txt"), "This is a").unwrap();
let (branches, _) = gitbutler_branch_actions::list_virtual_branches(project).unwrap();
assert_eq!(branches.len(), 1);
let source_branch_id = branches[0].id;
gitbutler_branch_actions::create_commit(project, source_branch_id, "Add a", None, false)
.unwrap();
std::fs::write(repository.path().join("b.txt"), "This is b").unwrap();
let commit_oid = gitbutler_branch_actions::create_commit(
project,
source_branch_id,
"Add b and update b",
None,
false,
)
.unwrap();
std::fs::write(repository.path().join("b.txt"), "This is b and an update").unwrap();
gitbutler_branch_actions::create_commit(project, source_branch_id, "Update b", None, false)
.unwrap();
let target_branch_id =
gitbutler_branch_actions::create_virtual_branch(project, &BranchCreateRequest::default())
.unwrap();
let result = gitbutler_branch_actions::move_commit(
project,
target_branch_id,
commit_oid,
source_branch_id,
);
assert_eq!(
result.unwrap_err().to_string(),
"the target commit contains hunks locked to its descendants"
);
}