diff --git a/gitbutler-app/src/git/oid.rs b/gitbutler-app/src/git/oid.rs index f5ad7e6a6..3e0718db4 100644 --- a/gitbutler-app/src/git/oid.rs +++ b/gitbutler-app/src/git/oid.rs @@ -7,6 +7,12 @@ pub struct Oid { oid: git2::Oid, } +impl Default for Oid { + fn default() -> Self { + git2::Oid::zero().into() + } +} + impl Serialize for Oid { fn serialize(&self, serializer: S) -> Result where diff --git a/gitbutler-app/src/virtual_branches/branch.rs b/gitbutler-app/src/virtual_branches/branch.rs index 2fe293f34..ab6bf4012 100644 --- a/gitbutler-app/src/virtual_branches/branch.rs +++ b/gitbutler-app/src/virtual_branches/branch.rs @@ -6,6 +6,7 @@ mod writer; pub use file_ownership::OwnershipClaim; pub use hunk::Hunk; +pub use ownership::reconcile_claims; pub use ownership::BranchOwnershipClaims; pub use reader::BranchReader as Reader; pub use writer::BranchWriter as Writer; @@ -22,7 +23,7 @@ pub type BranchId = Id; // store. it is more or less equivalent to a git branch reference, but it is not // stored or accessible from the git repository itself. it is stored in our // session storage under the branches/ directory. -#[derive(Serialize, Deserialize, Debug, PartialEq, Clone)] +#[derive(Serialize, Deserialize, Debug, PartialEq, Clone, Default)] pub struct Branch { pub id: BranchId, pub name: String, diff --git a/gitbutler-app/src/virtual_branches/branch/ownership.rs b/gitbutler-app/src/virtual_branches/branch/ownership.rs index 06374c808..a0c3e5608 100644 --- a/gitbutler-app/src/virtual_branches/branch/ownership.rs +++ b/gitbutler-app/src/virtual_branches/branch/ownership.rs @@ -1,8 +1,9 @@ -use std::{fmt, str::FromStr}; +use std::{collections::HashSet, fmt, str::FromStr}; use serde::{Deserialize, Serialize, Serializer}; -use super::OwnershipClaim; +use super::{Branch, OwnershipClaim}; +use anyhow::Result; #[derive(Debug, Clone, Default, PartialEq, Eq)] pub struct BranchOwnershipClaims { @@ -120,12 +121,177 @@ impl BranchOwnershipClaims { } } +#[derive(Debug, Clone)] +pub struct ClaimOutcome { + pub updated_branch: Branch, + pub removed_claims: Vec, +} +pub fn reconcile_claims( + all_branches: Vec, + claiming_branch: &Branch, + new_claims: &Vec, +) -> Result> { + let mut other_branches = all_branches + .into_iter() + .filter(|branch| branch.applied) + .filter(|branch| branch.id != claiming_branch.id) + .collect::>(); + + let mut claim_outcomes: Vec = Vec::new(); + + for file_ownership in new_claims { + for branch in &mut other_branches { + let taken = branch.ownership.take(file_ownership); + claim_outcomes.push(ClaimOutcome { + updated_branch: branch.clone(), + removed_claims: taken, + }); + } + } + + // Add the claiming branch to the list of outcomes + claim_outcomes.push(ClaimOutcome { + updated_branch: Branch { + ownership: BranchOwnershipClaims { + claims: new_claims.clone(), + }, + ..claiming_branch.clone() + }, + removed_claims: Vec::new(), + }); + + // Check the outcomes consistency and error out if they would result in a hunk being claimed by multiple branches + let mut seen = HashSet::new(); + for outcome in claim_outcomes.clone() { + for claim in outcome.updated_branch.ownership.claims { + for hunk in claim.hunks { + if !seen.insert(format!( + "{}-{}-{}", + claim.file_path.to_str().unwrap_or_default(), + hunk.start, + hunk.end + )) { + return Err(anyhow::anyhow!("inconsistent ownership claims")); + } + } + } + } + + Ok(claim_outcomes) +} + #[cfg(test)] mod tests { - use std::vec; + use std::{path::PathBuf, vec}; + + use crate::virtual_branches::branch::Hunk; use super::*; + #[test] + fn test_reconcile_ownership_simple() { + let branch_a = Branch { + name: "a".to_string(), + ownership: BranchOwnershipClaims { + claims: vec![OwnershipClaim { + file_path: PathBuf::from("foo"), + hunks: vec![ + Hunk { + start: 1, + end: 3, + hash: Some("1,3".to_string()), + timestamp_ms: None, + }, + Hunk { + start: 4, + end: 6, + hash: Some("4,6".to_string()), + timestamp_ms: None, + }, + ], + }], + }, + applied: true, + ..Default::default() + }; + let branch_b = Branch { + name: "b".to_string(), + ownership: BranchOwnershipClaims { + claims: vec![OwnershipClaim { + file_path: PathBuf::from("foo"), + hunks: vec![Hunk { + start: 7, + end: 9, + hash: Some("7,9".to_string()), + timestamp_ms: None, + }], + }], + }, + applied: true, + ..Default::default() + }; + let all_branches: Vec = vec![branch_a.clone(), branch_b.clone()]; + let claim: Vec = vec![OwnershipClaim { + file_path: PathBuf::from("foo"), + hunks: vec![ + Hunk { + start: 4, + end: 6, + hash: Some("4,6".to_string()), + timestamp_ms: None, + }, + Hunk { + start: 7, + end: 9, + hash: Some("9,7".to_string()), + timestamp_ms: None, + }, + ], + }]; + let claim_outcomes = reconcile_claims(all_branches.clone(), &branch_b, &claim).unwrap(); + assert_eq!(claim_outcomes.len(), all_branches.len()); + assert_eq!(claim_outcomes[0].updated_branch.id, branch_a.id); + assert_eq!(claim_outcomes[1].updated_branch.id, branch_b.id); + + assert_eq!( + claim_outcomes[0].updated_branch.ownership, + BranchOwnershipClaims { + claims: vec![OwnershipClaim { + file_path: PathBuf::from("foo"), + hunks: vec![Hunk { + start: 1, + end: 3, + hash: Some("1,3".to_string()), + timestamp_ms: None, + },], + }], + } + ); + + assert_eq!( + claim_outcomes[1].updated_branch.ownership, + BranchOwnershipClaims { + claims: vec![OwnershipClaim { + file_path: PathBuf::from("foo"), + hunks: vec![ + Hunk { + start: 4, + end: 6, + hash: Some("4,6".to_string()), + timestamp_ms: None, + }, + Hunk { + start: 7, + end: 9, + hash: Some("9,7".to_string()), + timestamp_ms: None, + }, + ], + }], + } + ); + } + #[test] fn test_ownership() { let ownership = "src/main.rs:0-100\nsrc/main2.rs:200-300".parse::(); diff --git a/gitbutler-app/src/virtual_branches/virtual.rs b/gitbutler-app/src/virtual_branches/virtual.rs index d9edae1be..36ff89036 100644 --- a/gitbutler-app/src/virtual_branches/virtual.rs +++ b/gitbutler-app/src/virtual_branches/virtual.rs @@ -1,5 +1,5 @@ use std::{ - collections::{HashMap, HashSet}, + collections::HashMap, hash::Hash, path::{Path, PathBuf}, time, vec, @@ -1791,48 +1791,23 @@ fn set_ownership( return Ok(()); } - let mut virtual_branches = Iterator::new(session_reader) + let virtual_branches = Iterator::new(session_reader) .context("failed to create branch iterator")? .collect::, reader::Error>>() - .context("failed to read virtual branches")? - .into_iter() - .filter(|branch| branch.applied) - .filter(|branch| branch.id != target_branch.id) - .collect::>(); + .context("failed to read virtual branches")?; - let mut branch_claims: Vec<(branch::Branch, Vec)> = Vec::new(); - for file_ownership in &ownership.claims { - for branch in &mut virtual_branches { - let taken = branch.ownership.take(file_ownership); - branch_claims.push((branch.clone(), taken)); - } - } - - // Check consistency and reject changes if they would result in a hunk being claimed by multiple branches - let mut seen = HashSet::new(); - for (_, claims) in branch_claims.clone() { - for claim in claims { - for hunk in claim.hunks { - if !seen.insert(format!( - "{}-{}-{}", - claim.file_path.to_str().unwrap_or_default(), - hunk.start, - hunk.end - )) { - return Err(anyhow::anyhow!("inconsistent ownership claims")); - } - } - } - } - - for (branch, taken) in &mut branch_claims { - if !taken.is_empty() { + let mut claim_outcomes = + branch::reconcile_claims(virtual_branches, target_branch, &ownership.claims)?; + for claim_outcome in &mut claim_outcomes { + if !claim_outcome.removed_claims.is_empty() { branch_writer - .write(branch) + .write(&mut claim_outcome.updated_branch) .context("failed to write ownership for branch".to_string())?; } } + // Updates the claiming branch that was passed as mutable state with the new ownership claims + // TODO: remove mutable reference to target_branch target_branch.ownership = ownership.clone(); Ok(())