From 811f0ee35b11685feb1539511f2772a5be8a15fa Mon Sep 17 00:00:00 2001 From: Mattias Granlund Date: Tue, 23 Apr 2024 16:11:27 +0200 Subject: [PATCH] Blame across workspace instead of by branch - refactors `update_gitbutler_integration` - creates new `get_workspace_head` function - ignore locking when resolving merge conflicts --- .../src/virtual_branches/base.rs | 342 +++++++++--------- .../src/virtual_branches/branch/hunk.rs | 6 +- .../src/virtual_branches/integration.rs | 178 ++++++--- .../src/virtual_branches/virtual.rs | 144 +++++--- .../virtual_branches/update_base_branch.rs | 2 +- 5 files changed, 394 insertions(+), 278 deletions(-) diff --git a/crates/gitbutler-core/src/virtual_branches/base.rs b/crates/gitbutler-core/src/virtual_branches/base.rs index 468a761d9..be39546f1 100644 --- a/crates/gitbutler-core/src/virtual_branches/base.rs +++ b/crates/gitbutler-core/src/virtual_branches/base.rs @@ -362,197 +362,193 @@ pub fn update_base_branch( let vb_state = VirtualBranchesHandle::new(&project_repository.project().gb_dir()); // try to update every branch - let updated_vbranches = - super::get_status_by_branch(project_repository, Some(&new_target_commit.id()))? - .0 - .into_iter() - .map(|(branch, _)| branch) - .map( - |mut branch: branch::Branch| -> Result> { - let branch_tree = repo.find_tree(branch.tree)?; + let updated_vbranches = super::get_status_by_branch(project_repository, None)? + .0 + .into_iter() + .map(|(branch, _)| branch) + .map( + |mut branch: branch::Branch| -> Result> { + let branch_tree = repo.find_tree(branch.tree)?; - let branch_head_commit = repo.find_commit(branch.head).context(format!( - "failed to find commit {} for branch {}", - branch.head, branch.id - ))?; - let branch_head_tree = branch_head_commit.tree().context(format!( - "failed to find tree for commit {} for branch {}", - branch.head, branch.id - ))?; + let branch_head_commit = repo.find_commit(branch.head).context(format!( + "failed to find commit {} for branch {}", + branch.head, branch.id + ))?; + let branch_head_tree = branch_head_commit.tree().context(format!( + "failed to find tree for commit {} for branch {}", + branch.head, branch.id + ))?; - let result_integrated_detected = - |mut branch: branch::Branch| -> Result> { - // branch head tree is the same as the new target tree. - // meaning we can safely use the new target commit as the branch head. + let result_integrated_detected = + |mut branch: branch::Branch| -> Result> { + // branch head tree is the same as the new target tree. + // meaning we can safely use the new target commit as the branch head. - branch.head = new_target_commit.id(); - - // it also means that the branch is fully integrated into the target. - // disconnect it from the upstream - branch.upstream = None; - branch.upstream_head = None; - - let non_commited_files = diff::trees( - &project_repository.git_repository, - &branch_head_tree, - &branch_tree, - )?; - if non_commited_files.is_empty() { - // if there are no commited files, then the branch is fully merged - // and we can delete it. - vb_state.remove_branch(branch.id)?; - project_repository.delete_branch_reference(&branch)?; - Ok(None) - } else { - vb_state.set_branch(branch.clone())?; - Ok(Some(branch)) - } - }; - - if branch_head_tree.id() == new_target_tree.id() { - return result_integrated_detected(branch); - } - - // try to merge branch head with new target - let mut branch_tree_merge_index = repo - .merge_trees(&old_target_tree, &branch_tree, &new_target_tree) - .context(format!("failed to merge trees for branch {}", branch.id))?; - - if branch_tree_merge_index.has_conflicts() { - // branch tree conflicts with new target, unapply branch for now. we'll handle it later, when user applies it back. - branch.applied = false; - vb_state.set_branch(branch.clone())?; - return Ok(Some(branch)); - } - - let branch_merge_index_tree_oid = - branch_tree_merge_index.write_tree_to(repo)?; - - if branch_merge_index_tree_oid == new_target_tree.id() { - return result_integrated_detected(branch); - } - - if branch.head == target.sha { - // there are no commits on the branch, so we can just update the head to the new target and calculate the new tree branch.head = new_target_commit.id(); - branch.tree = branch_merge_index_tree_oid; - vb_state.set_branch(branch.clone())?; - return Ok(Some(branch)); - } - let mut branch_head_merge_index = repo - .merge_trees(&old_target_tree, &branch_head_tree, &new_target_tree) - .context(format!( - "failed to merge head tree for branch {}", - branch.id - ))?; + // it also means that the branch is fully integrated into the target. + // disconnect it from the upstream + branch.upstream = None; + branch.upstream_head = None; - if branch_head_merge_index.has_conflicts() { - // branch commits conflict with new target, make sure the branch is - // unapplied. conflicts witll be dealt with when applying it back. - branch.applied = false; - vb_state.set_branch(branch.clone())?; - return Ok(Some(branch)); - } - - // branch commits do not conflict with new target, so lets merge them - let branch_head_merge_tree_oid = branch_head_merge_index - .write_tree_to(repo) - .context(format!( - "failed to write head merge index for {}", - branch.id - ))?; - - let ok_with_force_push = project_repository.project().ok_with_force_push; - - let result_merge = - |mut branch: branch::Branch| -> Result> { - // branch was pushed to upstream, and user doesn't like force pushing. - // create a merge commit to avoid the need of force pushing then. - let branch_head_merge_tree = repo - .find_tree(branch_head_merge_tree_oid) - .context("failed to find tree")?; - - let new_target_head = project_repository - .commit( - user, - format!( - "Merged {}/{} into {}", - target.branch.remote(), - target.branch.branch(), - branch.name - ) - .as_str(), - &branch_head_merge_tree, - &[&branch_head_commit, &new_target_commit], - signing_key, - ) - .context("failed to commit merge")?; - - branch.head = new_target_head; - branch.tree = branch_merge_index_tree_oid; + let non_commited_files = diff::trees( + &project_repository.git_repository, + &branch_head_tree, + &branch_tree, + )?; + if non_commited_files.is_empty() { + // if there are no commited files, then the branch is fully merged + // and we can delete it. + vb_state.remove_branch(branch.id)?; + project_repository.delete_branch_reference(&branch)?; + Ok(None) + } else { vb_state.set_branch(branch.clone())?; Ok(Some(branch)) - }; + } + }; - if branch.upstream.is_some() && !ok_with_force_push { - return result_merge(branch); - } + if branch_head_tree.id() == new_target_tree.id() { + return result_integrated_detected(branch); + } - // branch was not pushed to upstream yet. attempt a rebase, - let (_, committer) = project_repository.git_signatures(user)?; - let mut rebase_options = git2::RebaseOptions::new(); - rebase_options.quiet(true); - rebase_options.inmemory(true); - let mut rebase = repo - .rebase( - Some(branch.head), - Some(new_target_commit.id()), - None, - Some(&mut rebase_options), + // try to merge branch head with new target + let mut branch_tree_merge_index = repo + .merge_trees(&old_target_tree, &branch_tree, &new_target_tree) + .context(format!("failed to merge trees for branch {}", branch.id))?; + + if branch_tree_merge_index.has_conflicts() { + // branch tree conflicts with new target, unapply branch for now. we'll handle it later, when user applies it back. + branch.applied = false; + vb_state.set_branch(branch.clone())?; + return Ok(Some(branch)); + } + + let branch_merge_index_tree_oid = branch_tree_merge_index.write_tree_to(repo)?; + + if branch_merge_index_tree_oid == new_target_tree.id() { + return result_integrated_detected(branch); + } + + if branch.head == target.sha { + // there are no commits on the branch, so we can just update the head to the new target and calculate the new tree + branch.head = new_target_commit.id(); + branch.tree = branch_merge_index_tree_oid; + vb_state.set_branch(branch.clone())?; + return Ok(Some(branch)); + } + + let mut branch_head_merge_index = repo + .merge_trees(&old_target_tree, &branch_head_tree, &new_target_tree) + .context(format!( + "failed to merge head tree for branch {}", + branch.id + ))?; + + if branch_head_merge_index.has_conflicts() { + // branch commits conflict with new target, make sure the branch is + // unapplied. conflicts witll be dealt with when applying it back. + branch.applied = false; + vb_state.set_branch(branch.clone())?; + return Ok(Some(branch)); + } + + // branch commits do not conflict with new target, so lets merge them + let branch_head_merge_tree_oid = branch_head_merge_index + .write_tree_to(repo) + .context(format!( + "failed to write head merge index for {}", + branch.id + ))?; + + let ok_with_force_push = project_repository.project().ok_with_force_push; + + let result_merge = |mut branch: branch::Branch| -> Result> { + // branch was pushed to upstream, and user doesn't like force pushing. + // create a merge commit to avoid the need of force pushing then. + let branch_head_merge_tree = repo + .find_tree(branch_head_merge_tree_oid) + .context("failed to find tree")?; + + let new_target_head = project_repository + .commit( + user, + format!( + "Merged {}/{} into {}", + target.branch.remote(), + target.branch.branch(), + branch.name + ) + .as_str(), + &branch_head_merge_tree, + &[&branch_head_commit, &new_target_commit], + signing_key, ) - .context("failed to rebase")?; + .context("failed to commit merge")?; - let mut rebase_success = true; - // check to see if these commits have already been pushed - let mut last_rebase_head = branch.head; - while rebase.next().is_some() { - let index = rebase - .inmemory_index() - .context("failed to get inmemory index")?; - if index.has_conflicts() { - rebase_success = false; - break; - } + branch.head = new_target_head; + branch.tree = branch_merge_index_tree_oid; + vb_state.set_branch(branch.clone())?; + Ok(Some(branch)) + }; - if let Ok(commit_id) = rebase.commit(None, &committer.clone().into(), None) - { - last_rebase_head = commit_id.into(); - } else { - rebase_success = false; - break; - } + if branch.upstream.is_some() && !ok_with_force_push { + return result_merge(branch); + } + + // branch was not pushed to upstream yet. attempt a rebase, + let (_, committer) = project_repository.git_signatures(user)?; + let mut rebase_options = git2::RebaseOptions::new(); + rebase_options.quiet(true); + rebase_options.inmemory(true); + let mut rebase = repo + .rebase( + Some(branch.head), + Some(new_target_commit.id()), + None, + 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 = branch.head; + 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 rebase_success { - // rebase worked out, rewrite the branch head - rebase.finish(None).context("failed to finish rebase")?; - branch.head = last_rebase_head; - branch.tree = branch_merge_index_tree_oid; - vb_state.set_branch(branch.clone())?; - return Ok(Some(branch)); + if let Ok(commit_id) = rebase.commit(None, &committer.clone().into(), None) { + last_rebase_head = commit_id.into(); + } else { + rebase_success = false; + break; } + } - // rebase failed, do a merge commit - rebase.abort().context("failed to abort rebase")?; + if rebase_success { + // rebase worked out, rewrite the branch head + rebase.finish(None).context("failed to finish rebase")?; + branch.head = last_rebase_head; + branch.tree = branch_merge_index_tree_oid; + vb_state.set_branch(branch.clone())?; + return Ok(Some(branch)); + } - result_merge(branch) - }, - ) - .collect::>>()? - .into_iter() - .flatten() - .collect::>(); + // rebase failed, do a merge commit + rebase.abort().context("failed to abort rebase")?; + + result_merge(branch) + }, + ) + .collect::>>()? + .into_iter() + .flatten() + .collect::>(); // ok, now all the problematic branches have been unapplied // now we calculate and checkout new tree for the working directory diff --git a/crates/gitbutler-core/src/virtual_branches/branch/hunk.rs b/crates/gitbutler-core/src/virtual_branches/branch/hunk.rs index 619f586d9..e21c82e23 100644 --- a/crates/gitbutler-core/src/virtual_branches/branch/hunk.rs +++ b/crates/gitbutler-core/src/virtual_branches/branch/hunk.rs @@ -3,9 +3,7 @@ use std::{fmt::Display, ops::RangeInclusive, str::FromStr}; use anyhow::{anyhow, Context, Result}; use bstr::{BStr, ByteSlice}; -use crate::{git::diff, id::Id}; - -use super::Branch; +use crate::git::{self, diff}; pub type HunkHash = md5::Digest; @@ -15,7 +13,7 @@ pub struct Hunk { pub timestamp_ms: Option, pub start: u32, pub end: u32, - pub locked_to: Vec>, + pub locked_to: Vec, } impl From<&diff::GitHunk> for Hunk { diff --git a/crates/gitbutler-core/src/virtual_branches/integration.rs b/crates/gitbutler-core/src/virtual_branches/integration.rs index 2e7e7db32..4944c9560 100644 --- a/crates/gitbutler-core/src/virtual_branches/integration.rs +++ b/crates/gitbutler-core/src/virtual_branches/integration.rs @@ -1,6 +1,6 @@ -use std::io::{Read, Write}; +use std::path::PathBuf; -use anyhow::{Context, Result}; +use anyhow::{anyhow, Context, Result}; use bstr::ByteSlice; use lazy_static::lazy_static; @@ -16,9 +16,112 @@ lazy_static! { git::LocalRefname::new("gitbutler/integration", None); } +const WORKSPACE_HEAD: &str = "Workspace Head"; const GITBUTLER_INTEGRATION_COMMIT_AUTHOR_NAME: &str = "GitButler"; const GITBUTLER_INTEGRATION_COMMIT_AUTHOR_EMAIL: &str = "gitbutler@gitbutler.com"; +fn get_committer<'a>() -> Result> { + Ok(git::Signature::now( + GITBUTLER_INTEGRATION_COMMIT_AUTHOR_NAME, + GITBUTLER_INTEGRATION_COMMIT_AUTHOR_EMAIL, + )?) +} + +// Creates and returns a merge commit of all active branch heads. +// +// This is the base against which we diff the working directory to understand +// what files have been modified. +pub fn get_workspace_head( + vb_state: &VirtualBranchesHandle, + project_repository: &project_repository::Repository, +) -> Result { + let target = vb_state + .get_default_target() + .context("failed to get target")?; + let repo = &project_repository.git_repository; + let vb_state = VirtualBranchesHandle::new(&project_repository.project().gb_dir()); + + let all_virtual_branches = vb_state.list_branches()?; + let applied_virtual_branches = all_virtual_branches + .iter() + .filter(|branch| branch.applied) + .collect::>(); + + let target_commit = repo.find_commit(target.sha)?; + let target_tree = target_commit.tree()?; + let mut workspace_tree = target_commit.tree()?; + + // Merge applied branches into one `workspace_tree``. + for branch in &applied_virtual_branches { + let branch_head = repo.find_commit(branch.head)?; + let branch_tree = branch_head.tree()?; + + if let Ok(mut result) = repo.merge_trees(&target_tree, &workspace_tree, &branch_tree) { + if !result.has_conflicts() { + let final_tree_oid = result.write_tree_to(repo)?; + workspace_tree = repo.find_tree(final_tree_oid)?; + } else { + // TODO: Create error type and provide context. + return Err(anyhow!("Unexpected merge conflict")); + } + } + } + + let branch_heads = applied_virtual_branches + .iter() + .map(|b| repo.find_commit(b.head)) + .collect::, _>>()?; + let branch_head_refs = branch_heads.iter().collect::>(); + + // If no branches are applied then the workspace head is the target. + if branch_head_refs.is_empty() { + return Ok(target_commit.id()); + } + + // TODO(mg): Can we make this a constant? + let committer = get_committer()?; + + // Create merge commit of branch heads. + let workspace_head_id = repo.commit( + None, + &committer, + &committer, + WORKSPACE_HEAD, + &workspace_tree, + branch_head_refs.as_slice(), + )?; + Ok(workspace_head_id) +} + +// Before switching the user to our gitbutler integration branch we save +// the current branch into a text file. It is used in generating the commit +// message for integration branch, as a helpful hint about how to get back +// to where you were. +struct PreviousHead { + head: String, + sha: String, +} + +fn read_integration_file(path: &PathBuf) -> Result> { + if let Ok(prev_data) = std::fs::read_to_string(path) { + let parts: Vec<&str> = prev_data.split(':').collect(); + let prev_head = parts[0].to_string(); + let prev_sha = parts[1].to_string(); + Ok(Some(PreviousHead { + head: prev_head, + sha: prev_sha, + })) + } else { + Ok(None) + } +} + +fn write_integration_file(head: &git::Reference, path: PathBuf) -> Result<()> { + let sha = head.target().unwrap().to_string(); + std::fs::write(path, format!(":{}", sha))?; + Ok(()) +} + pub fn update_gitbutler_integration( vb_state: &VirtualBranchesHandle, project_repository: &project_repository::Repository, @@ -41,27 +144,19 @@ pub fn update_gitbutler_integration( let target_commit = repo.find_commit(target.sha)?; // get current repo head for reference - let head = repo.head()?; - let mut prev_head = head.name().unwrap().to_string(); - let mut prev_sha = head.target().unwrap().to_string(); - let integration_file = repo.path().join("integration"); - if prev_head == GITBUTLER_INTEGRATION_REFERENCE.to_string() { - // read the .git/integration file - if let Ok(mut integration_file) = std::fs::File::open(integration_file) { - let mut prev_data = String::new(); - integration_file.read_to_string(&mut prev_data)?; - let parts: Vec<&str> = prev_data.split(':').collect(); - - prev_head = parts[0].to_string(); - prev_sha = parts[1].to_string(); + let head_ref = repo.head()?; + let integration_filepath = repo.path().join("integration"); + let mut prev_branch = read_integration_file(&integration_filepath)?; + if let Some(branch) = &prev_branch { + if branch.head != GITBUTLER_INTEGRATION_REFERENCE.to_string() { + // we are moving from a regular branch to our gitbutler integration branch, write a file to + // .git/integration with the previous head and name + write_integration_file(&head_ref, integration_filepath)?; + prev_branch = Some(PreviousHead { + head: head_ref.target().unwrap().to_string(), + sha: head_ref.target().unwrap().to_string(), + }); } - } else { - // we are moving from a regular branch to our gitbutler integration branch, save the original - // write a file to .git/integration with the previous head and name - let mut file = std::fs::File::create(integration_file)?; - prev_head.push(':'); - prev_head.push_str(&prev_sha); - file.write_all(prev_head.as_bytes())?; } // commit index to temp head for the merge @@ -80,19 +175,9 @@ pub fn update_gitbutler_integration( .filter(|branch| branch.applied) .collect::>(); - let base_tree = target_commit.tree()?; - let mut final_tree = target_commit.tree()?; - for branch in &applied_virtual_branches { - // merge this branches tree with our tree - let branch_head = repo.find_commit(branch.head)?; - let branch_tree = branch_head.tree()?; - if let Ok(mut result) = repo.merge_trees(&base_tree, &final_tree, &branch_tree) { - if !result.has_conflicts() { - let final_tree_oid = result.write_tree_to(repo)?; - final_tree = repo.find_tree(final_tree_oid)?; - } - } - } + let integration_commit_id = get_workspace_head(&vb_state, project_repository)?; + let integration_commit = repo.find_commit(integration_commit_id).unwrap(); + let integration_tree = integration_commit.tree()?; // message that says how to get back to where they were let mut message = "GitButler Integration Commit".to_string(); @@ -125,32 +210,31 @@ pub fn update_gitbutler_integration( message.push('\n'); } } - message.push_str("\nYour previous branch was: "); - message.push_str(&prev_head); - message.push_str("\n\n"); - message.push_str("The sha for that commit was: "); - message.push_str(&prev_sha); - message.push_str("\n\n"); + if let Some(prev_branch) = prev_branch { + message.push_str("\nYour previous branch was: "); + message.push_str(&prev_branch.head); + message.push_str("\n\n"); + message.push_str("The sha for that commit was: "); + message.push_str(&prev_branch.sha); + message.push_str("\n\n"); + } message.push_str("For more information about what we're doing here, check out our docs:\n"); message.push_str("https://docs.gitbutler.com/features/virtual-branches/integration-branch\n"); - let committer = git::Signature::now( - GITBUTLER_INTEGRATION_COMMIT_AUTHOR_NAME, - GITBUTLER_INTEGRATION_COMMIT_AUTHOR_EMAIL, - )?; + let committer = get_committer()?; let final_commit = repo.commit( Some(&"refs/heads/gitbutler/integration".parse().unwrap()), &committer, &committer, &message, - &final_tree, + &integration_commit.tree()?, &[&target_commit], )?; // write final_tree as the current index let mut index = repo.index()?; - index.read_tree(&final_tree)?; + index.read_tree(&integration_tree)?; index.write()?; // finally, update the refs/gitbutler/ heads to the states of the current virtual branches diff --git a/crates/gitbutler-core/src/virtual_branches/virtual.rs b/crates/gitbutler-core/src/virtual_branches/virtual.rs index de089edc0..8cf9e9d2a 100644 --- a/crates/gitbutler-core/src/virtual_branches/virtual.rs +++ b/crates/gitbutler-core/src/virtual_branches/virtual.rs @@ -497,12 +497,12 @@ pub fn unapply_ownership( .filter(|b| b.applied) .collect::>(); - let integration_commit = - super::integration::update_gitbutler_integration(&vb_state, project_repository)?; + let integration_commit_id = + super::integration::get_workspace_head(&vb_state, project_repository)?; let (applied_statuses, _) = get_applied_status( project_repository, - &integration_commit, + &integration_commit_id, &default_target.sha, applied_branches, ) @@ -551,7 +551,7 @@ pub fn unapply_ownership( let repo = &project_repository.git_repository; let target_commit = repo - .find_commit(integration_commit) + .find_commit(integration_commit_id) .context("failed to find target commit")?; let base_tree = target_commit.tree().context("failed to get target tree")?; @@ -559,7 +559,7 @@ pub fn unapply_ownership( target_commit.tree().context("failed to get target tree"), |final_tree, status| { let final_tree = final_tree?; - let tree_oid = write_tree(project_repository, &integration_commit, &status.1)?; + let tree_oid = write_tree(project_repository, &integration_commit_id, &status.1)?; let branch_tree = repo.find_tree(tree_oid)?; let mut result = repo.merge_trees(&base_tree, &final_tree, &branch_tree)?; let final_tree_oid = result.write_tree_to(repo)?; @@ -781,11 +781,17 @@ pub fn list_virtual_branches( .get_default_target() .context("failed to get default target")?; - let integration_commit = - super::integration::update_gitbutler_integration(&vb_state, project_repository)?; + let integration_commit_id = + super::integration::get_workspace_head(&vb_state, project_repository)?; + let integration_commit = project_repository + .git_repository + .find_commit(integration_commit_id) + .unwrap(); + + super::integration::update_gitbutler_integration(&vb_state, project_repository)?; let (statuses, skipped_files) = - get_status_by_branch(project_repository, Some(&integration_commit))?; + get_status_by_branch(project_repository, Some(&integration_commit.id()))?; let max_selected_for_changes = statuses .iter() .filter_map(|(branch, _)| branch.selected_for_changes) @@ -1661,7 +1667,7 @@ pub type BranchStatus = HashMap>; pub fn get_status_by_branch( project_repository: &project_repository::Repository, integration_commit: Option<&git::Oid>, -) -> Result<(Vec<(branch::Branch, BranchStatus)>, Vec)> { +) -> Result<(AppliedStatuses, Vec)> { let vb_state = VirtualBranchesHandle::new(&project_repository.project().gb_dir()); let default_target = @@ -1740,10 +1746,8 @@ fn get_non_applied_status( .collect::>>() } -// given a list of applied virtual branches, return the status of each file, comparing the default target with -// the working directory -// -// ownerships are updated if nessessary +// Returns branches and their associated file changes, in addition to a list +// of skipped files. fn get_applied_status( project_repository: &project_repository::Repository, integration_commit: &git::Oid, @@ -1765,7 +1769,6 @@ fn get_applied_status( virtual_branches.sort_by(|a, b| a.order.cmp(&b.order)); if virtual_branches.is_empty() && !base_diffs.is_empty() { - // no virtual branches, but hunks: create default branch virtual_branches = vec![ create_virtual_branch(project_repository, &BranchCreateRequest::default()) @@ -1773,22 +1776,43 @@ fn get_applied_status( ]; } - // align branch ownership to the real hunks: - // - update shifted hunks - // - remove non existent hunks - let mut diffs_by_branch: HashMap = virtual_branches .iter() .map(|branch| (branch.id, HashMap::new())) .collect(); let mut mtimes = MTimeCache::default(); - let mut git_hunk_map = HashMap::new(); + let mut locked_hunk_map = HashMap::>::new(); - for branch in &virtual_branches { - if !branch.applied { - bail!("branch {} is not applied", branch.name); - } + let merge_base = project_repository + .git_repository + .merge_base(*target_sha, *integration_commit)?; + + // The merge base between the integration commit and target _is_ the + // target, unless you are resolving merge conflicts. If that's the case + // we ignore locks until we are back to normal mode. + // + // If we keep the test repo and panic when `berge_base != target_sha` + // when running the test below, then we have the following commit graph. + // + // Test: virtual_branches::update_base_branch::applied_branch::integrated_with_locked_conflicting_hunks + // Command: `git log --oneline --all --graph`` + // * 244b526 GitButler WIP Commit + // | * ea2956e (HEAD -> gitbutler/integration) GitButler Integration Commit + // | * 3f2ccce (origin/master) Merge pull request from refs/heads/Virtual-branch + // | |\ + // | |/ + // |/| + // | * a6a0ed8 second + // | | * f833dbe (int) Workspace Head + // | |/ + // |/| + // * | dee400c (origin/Virtual-branch, merge_base) third + // |/ + // * 56c139c (master) first + // * 6276165 Initial commit + // (END) + if merge_base == *target_sha { for (path, hunks) in base_diffs.clone().into_iter() { for hunk in hunks { let blame = project_repository.git_repository.blame( @@ -1796,16 +1820,20 @@ fn get_applied_status( hunk.old_start, (hunk.old_start + hunk.old_lines).saturating_sub(1), target_sha, - &branch.head, + integration_commit, ); if let Ok(blame) = blame { for blame_hunk in blame.iter() { - let commit = blame_hunk.orig_commit_id(); - if git::Oid::from(commit) != *target_sha { - let hash = Hunk::hash(hunk.diff_lines.as_ref()); - git_hunk_map.insert(hash, branch.id); - break; + let commit = git::Oid::from(blame_hunk.orig_commit_id()); + if commit != *target_sha && commit != *integration_commit { + let hash = Hunk::hash_diff(hunk.diff_lines.as_ref()); + locked_hunk_map + .entry(hash) + .and_modify(|commits| { + commits.push(commit); + }) + .or_insert(vec![commit]); } } } @@ -1813,6 +1841,13 @@ fn get_applied_status( } } + let mut commit_to_branch = HashMap::new(); + for branch in &mut virtual_branches { + for commit in project_repository.log(branch.head, LogUntil::Commit(*target_sha))? { + commit_to_branch.insert(commit.id(), branch.id); + } + } + for branch in &mut virtual_branches { if !branch.applied { bail!("branch {} is not applied", branch.name); @@ -1836,13 +1871,10 @@ fn get_applied_status( // if any of the current hunks intersects with the owned hunk, we want to keep it for (i, git_diff_hunk) in git_diff_hunks.iter().enumerate() { let hash = Hunk::hash_diff(git_diff_hunk.diff_lines.as_ref()); - if let Some(locked_to) = git_hunk_map.get(&hash) { - if locked_to != &branch.id { - return None; - } + if locked_hunk_map.contains_key(&hash) { + return None; // Defer allocation to unclaimed hunks processing } if claimed_hunk.eq(&Hunk::from(git_diff_hunk)) { - // try to re-use old timestamp let timestamp = claimed_hunk.timestam_ms().unwrap_or(mtime); diffs_by_branch .entry(branch.id) @@ -1905,11 +1937,16 @@ fn get_applied_status( .position(|b| b.selected_for_changes == Some(max_selected_for_changes)) .unwrap_or(0); + // Everything claimed has been removed from `base_diffs`, here we just + // process the remaining ones. for (filepath, hunks) in base_diffs { for hunk in hunks { let hash = Hunk::hash_diff(hunk.diff_lines.as_ref()); - let vbranch_pos = if let Some(locked_to) = git_hunk_map.get(&hash) { - let p = virtual_branches.iter().position(|vb| vb.id == *locked_to); + let locked_to = locked_hunk_map.get(&hash); + + let vbranch_pos = if let Some(locked_to) = locked_to { + let branch_id = commit_to_branch.get(&locked_to[0]).unwrap(); + let p = virtual_branches.iter().position(|vb| vb.id == *branch_id); match p { Some(p) => p, _ => default_vbranch_pos, @@ -1918,14 +1955,15 @@ fn get_applied_status( default_vbranch_pos }; - let hash = Hunk::hash(hunk.diff_lines.as_ref()); + let hash = Hunk::hash_diff(hunk.diff_lines.as_ref()); let mut new_hunk = Hunk::from(&hunk) .with_timestamp(mtimes.mtime_by_path(filepath.as_path())) .with_hash(hash); - new_hunk.locked_to = git_hunk_map - .get(&hash) - .map(|locked_to| vec![*locked_to]) - .unwrap_or_default(); + new_hunk.locked_to = match locked_to { + Some(locked_to) => locked_to.clone(), + _ => vec![], + }; + virtual_branches[vbranch_pos] .ownership .put(&OwnershipClaim { @@ -2287,10 +2325,10 @@ pub fn commit( let message = &message_buffer; - let integration_commit = - super::integration::update_gitbutler_integration(&vb_state, project_repository)?; + let integration_commit_id = + super::integration::get_workspace_head(&vb_state, project_repository)?; // get the files to commit - let (mut statuses, _) = get_status_by_branch(project_repository, Some(&integration_commit)) + let (mut statuses, _) = get_status_by_branch(project_repository, Some(&integration_commit_id)) .context("failed to get status by branch")?; let (ref mut branch, files) = statuses @@ -2709,12 +2747,12 @@ pub fn amend( }) })?; - let integration_commit = - super::integration::update_gitbutler_integration(&vb_state, project_repository)?; + let integration_commit_id = + super::integration::get_workspace_head(&vb_state, project_repository)?; let (mut applied_statuses, _) = get_applied_status( project_repository, - &integration_commit, + &integration_commit_id, &default_target.sha, applied_branches, )?; @@ -2864,12 +2902,12 @@ pub fn cherry_pick( .filter(|b| b.applied) .collect::>(); - let integration_commit = - super::integration::update_gitbutler_integration(&vb_state, project_repository)?; + let integration_commit_id = + super::integration::get_workspace_head(&vb_state, project_repository)?; let (applied_statuses, _) = get_applied_status( project_repository, - &integration_commit, + &integration_commit_id, &default_target.sha, applied_branches, )?; @@ -3390,12 +3428,12 @@ pub fn move_commit( }) })?; - let integration_commit = - super::integration::update_gitbutler_integration(&vb_state, project_repository)?; + let integration_commit_id = + super::integration::get_workspace_head(&vb_state, project_repository)?; let (mut applied_statuses, _) = get_applied_status( project_repository, - &integration_commit, + &integration_commit_id, &default_target.sha, applied_branches, )?; diff --git a/crates/gitbutler-core/tests/suite/virtual_branches/update_base_branch.rs b/crates/gitbutler-core/tests/suite/virtual_branches/update_base_branch.rs index c0503d60e..216773d72 100644 --- a/crates/gitbutler-core/tests/suite/virtual_branches/update_base_branch.rs +++ b/crates/gitbutler-core/tests/suite/virtual_branches/update_base_branch.rs @@ -1576,7 +1576,7 @@ mod applied_branch { .unwrap(); controller - .create_commit(project_id, &branch_id, "first", None, false) + .create_commit(project_id, &branch_id, "third", None, false) .await .unwrap();