Surface warning if hunk locked to multiple branches

- now returning `HunkLock` to front end
- detect if locked to more than one branch and warn user

app/src/lib/vbranches/tooltip.ts

@@ -1,15 +1,17 @@
+import { HunkLock, type Commit } from './types';
 import { unique } from '$lib/utils/filters';
-import type { Commit } from './types';
 
-export function getLockText(commitId: string[] | string, commits: Commit[]): string {
+export function getLockText(hunkLocks: HunkLock | HunkLock[] | string, commits: Commit[]): string {
-	if (!commitId || commits === undefined) return 'Depends on a committed change';
+	if (!hunkLocks || commits === undefined) return 'Depends on a committed change';
 
-	const lockedIds = typeof commitId == 'string' ? [commitId] : (commitId as string[]);
+	const locks = hunkLocks instanceof HunkLock ? [hunkLocks] : (hunkLocks as HunkLock[]);
 
-	const descriptions = lockedIds
+	const descriptions = locks
 		.filter(unique)
-		.map((id) => {
+		.map((lock) => {
-			const commit = commits.find((commit) => commit.id == id);
+			const commit = commits.find((commit) => {
+				commit.id == lock.commitId;
+			});
 			const shortCommitId = commit?.id.slice(0, 7);
 			if (commit) {
 				const shortTitle = commit.descriptionTitle?.slice(0, 35) + '...';
@@ -19,5 +21,13 @@ export function getLockText(commitId: string[] | string, commits: Commit[]): str
 			}
 		})
 		.join('\n');
-	return 'Locked due to dependency on:\n' + descriptions;
+	const branchCount = locks.map((lock) => lock.branchId).filter(unique).length;
+	if (branchCount > 1) {
+		return (
+			'Warning, undefined behavior due to lock on multiple branches!\n\n' +
+			'Locked because changes depend on:\n' +
+			descriptions
+		);
+	}
+	return 'Locked because changes depend on:\n' + descriptions;
 }

app/src/lib/vbranches/types.ts

@@ -22,10 +22,16 @@ export class Hunk {
 	filePath!: string;
 	hash?: string;
 	locked!: boolean;
-	lockedTo!: string[] | undefined;
+	@Type(() => HunkLock)
+	lockedTo!: HunkLock[];
 	changeType!: ChangeType;
 }
 
+export class HunkLock {
+	branchId!: string;
+	commitId!: string;
+}
+
 export type AnyFile = LocalFile | RemoteFile;
 
 export class LocalFile {
@@ -63,7 +69,7 @@ export class LocalFile {
 			: false;
 	}
 
-	get lockedIds(): string[] {
+	get lockedIds(): HunkLock[] {
 		return this.hunks
 			.flatMap((hunk) => hunk.lockedTo)
 			.filter(notNull)
@@ -252,7 +258,7 @@ export class RemoteFile {
 		return this.hunks.map((h) => h.id);
 	}
 
-	get lockedIds(): string[] {
+	get lockedIds(): HunkLock[] {
 		return [];
 	}
 

crates/gitbutler-core/src/git/diff.rs

@@ -53,7 +53,7 @@ pub struct GitHunk {
     #[serde(rename = "diff", serialize_with = "crate::serde::as_string_lossy")]
     pub diff_lines: BString,
     pub binary: bool,
-    pub locked_to: Box<[git::Oid]>,
+    pub locked_to: Box<[HunkLock]>,
     pub change_type: ChangeType,
 }
 
@@ -95,12 +95,22 @@ impl GitHunk {
         self.new_start <= line && self.new_start + self.new_lines >= line
     }
 
-    pub fn with_locks(mut self, locks: &[git::Oid]) -> Self {
+    pub fn with_locks(mut self, locks: &[HunkLock]) -> Self {
         self.locked_to = locks.to_owned().into();
         self
     }
 }
 
+// A hunk is locked when it depends on changes in commits that are in your
+// workspace. A hunk can be locked to more than one branch if it overlaps
+// with more than one committed hunk.
+#[derive(Debug, PartialEq, Eq, Clone, Serialize, Copy)]
+#[serde(rename_all = "camelCase")]
+pub struct HunkLock {
+    pub branch_id: uuid::Uuid,
+    pub commit_id: git::Oid,
+}
+
 #[derive(Debug, PartialEq, Clone, Serialize, Default)]
 #[serde(rename_all = "camelCase")]
 pub struct FileDiff {

crates/gitbutler-core/src/virtual_branches/branch/hunk.rs

@@ -3,7 +3,7 @@ use std::{fmt::Display, ops::RangeInclusive, str::FromStr};
 use anyhow::{anyhow, Context, Result};
 use bstr::{BStr, ByteSlice};
 
-use crate::git::{self, diff};
+use crate::git::diff;
 
 pub type HunkHash = md5::Digest;
 
@@ -13,7 +13,7 @@ pub struct Hunk {
     pub timestamp_ms: Option<u128>,
     pub start: u32,
     pub end: u32,
-    pub locked_to: Vec<git::Oid>,
+    pub locked_to: Vec<diff::HunkLock>,
 }
 
 impl From<&diff::GitHunk> for Hunk {

crates/gitbutler-core/src/virtual_branches/virtual.rs

@@ -24,6 +24,7 @@ use super::{
     branch_to_remote_branch, errors, target, RemoteBranch, VirtualBranchesHandle,
 };
 use crate::git::diff::{diff_files_into_hunks, trees, DiffByPathMap};
+use crate::id::Id;
 use crate::virtual_branches::branch::HunkHash;
 use crate::{
     askpass::AskpassBroker,
@@ -143,7 +144,7 @@ pub struct VirtualBranchHunk {
     pub end: u32,
     pub binary: bool,
     pub locked: bool,
-    pub locked_to: Option<Box<[git::Oid]>>,
+    pub locked_to: Option<Box<[diff::HunkLock]>>,
     pub change_type: diff::ChangeType,
 }
 
@@ -1734,13 +1735,20 @@ 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);
+        }
+    }
+
     let mut diffs_by_branch: HashMap<BranchId, BranchStatus> = virtual_branches
         .iter()
         .map(|branch| (branch.id, HashMap::new()))
         .collect();
 
     let mut mtimes = MTimeCache::default();
-    let mut locked_hunk_map = HashMap::<HunkHash, Vec<git::Oid>>::new();
+    let mut locked_hunk_map = HashMap::<HunkHash, Vec<diff::HunkLock>>::new();
 
     let merge_base = project_repository
         .git_repository
@@ -1783,15 +1791,22 @@ fn get_applied_status(
 
                 if let Ok(blame) = blame {
                     for blame_hunk in blame.iter() {
-                        let commit = git::Oid::from(blame_hunk.orig_commit_id());
+                        let commit_id = git::Oid::from(blame_hunk.orig_commit_id());
-                        if commit != *target_sha && commit != *integration_commit {
+                        if commit_id != *target_sha && commit_id != *integration_commit {
                             let hash = Hunk::hash_diff(hunk.diff_lines.as_ref());
+                            let branch_id = uuid::Uuid::parse_str(
+                                &commit_to_branch.get(&commit_id).unwrap().to_string(),
+                            )?;
+                            let hunk_lock = diff::HunkLock {
+                                branch_id,
+                                commit_id,
+                            };
                             locked_hunk_map
                                 .entry(hash)
-                                .and_modify(|commits| {
+                                .and_modify(|locks| {
-                                    commits.push(commit);
+                                    locks.push(hunk_lock);
                                 })
-                                .or_insert(vec![commit]);
+                                .or_insert(vec![hunk_lock]);
                         }
                     }
                 }
@@ -1799,13 +1814,6 @@ 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);
@@ -1902,9 +1910,11 @@ fn get_applied_status(
             let hash = Hunk::hash_diff(hunk.diff_lines.as_ref());
             let locked_to = locked_hunk_map.get(&hash);
 
-            let vbranch_pos = if let Some(locked_to) = locked_to {
+            let vbranch_pos = if let Some(locks) = locked_to {
-                let branch_id = commit_to_branch.get(&locked_to[0]).unwrap();
+                let first_lock = &locks[0];
-                let p = virtual_branches.iter().position(|vb| vb.id == *branch_id);
+                let p = virtual_branches
+                    .iter()
+                    .position(|vb| vb.id == Id::<Branch>::from(first_lock.branch_id));
                 match p {
                     Some(p) => p,
                     _ => default_vbranch_pos,

crates/gitbutler-core/tests/suite/virtual_branches/create_commit.rs

@@ -2,6 +2,7 @@ use gitbutler_core::{
     id::Id,
     virtual_branches::{Branch, VirtualBranch},
 };
+use uuid::Uuid;
 
 use super::*;
 
@@ -275,8 +276,8 @@ async fn should_double_lock() {
     let locks = &branch.files[0].hunks[0].locked_to.clone().unwrap();
 
     assert_eq!(locks.len(), 2);
-    assert_eq!(locks[0], commit_1);
+    assert_eq!(locks[0].commit_id, commit_1);
-    assert_eq!(locks[1], commit_2);
+    assert_eq!(locks[1].commit_id, commit_2);
 }
 
 // This test only validates that locks are detected across virtual branches, it does
@@ -311,7 +312,11 @@ async fn should_double_lock_across_branches() {
         .await
         .unwrap();
 
-    controller
+    // TODO(mg): Make `create_commit` clean up ownership of committed hunks.
+    // TODO(mg): Needed because next hunk overlaps with previous ownership.
+    get_virtual_branch(controller, project_id, branch_1_id).await;
+
+    let branch_2_id = controller
         .create_virtual_branch(
             project_id,
             &branch::BranchCreateRequest {
@@ -326,19 +331,26 @@ async fn should_double_lock_across_branches() {
     write_file(repository, "file.txt", &lines);
 
     let commit_2 = controller
-        .create_commit(project_id, &branch_1_id, "commit 2", None, false)
+        .create_commit(project_id, &branch_2_id, "commit 2", None, false)
         .await
         .unwrap();
 
     lines[3] = "change3".to_string();
     write_file(repository, "file.txt", &lines);
 
-    let branch = get_virtual_branch(controller, project_id, branch_1_id).await;
+    let branch_1 = get_virtual_branch(controller, project_id, branch_1_id).await;
-    let locks = &branch.files[0].hunks[0].locked_to.clone().unwrap();
+    let locks = &branch_1.files[0].hunks[0].locked_to.clone().unwrap();
-
     assert_eq!(locks.len(), 2);
-    assert_eq!(locks[0], commit_1);
+    assert_eq!(locks[0].commit_id, commit_1);
-    assert_eq!(locks[1], commit_2);
+    assert_eq!(locks[1].commit_id, commit_2);
+    assert_eq!(
+        locks[0].branch_id,
+        Uuid::parse_str(&branch_1_id.to_string()).unwrap()
+    );
+    assert_eq!(
+        locks[1].branch_id,
+        Uuid::parse_str(&branch_2_id.to_string()).unwrap()
+    );
 }
 
 fn write_file(repository: &TestProject, path: &str, lines: &[String]) {