Integrate new hunk dependency algorithm

- behind a project level feature flag

crates/gitbutler-branch-actions/Cargo.toml

@@ -30,6 +30,7 @@ gitbutler-cherry-pick.workspace = true
 gitbutler-oxidize.workspace = true
 gitbutler-stack.workspace = true
 gitbutler-patch-reference.workspace = true
+gitbutler-hunk-dependency.workspace = true
 serde = { workspace = true, features = ["std"] }
 bstr.workspace = true
 diffy = "0.4.0"

crates/gitbutler-branch-actions/src/hunk.rs

@@ -6,8 +6,8 @@ use std::{
 };
 
 use gitbutler_diff::{GitHunk, Hunk, HunkHash};
+use gitbutler_hunk_dependency::locks::HunkLock;
 use gitbutler_serde::BStringForFrontend;
-use gitbutler_stack::StackId;
 use itertools::Itertools;
 use md5::Digest;
 use serde::Serialize;
@@ -42,17 +42,6 @@ pub struct VirtualBranchHunk {
     pub poisoned: bool,
 }
 
-// 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: StackId,
-    #[serde(with = "gitbutler_serde::oid")]
-    pub commit_id: git2::Oid,
-}
-
 /// Lifecycle
 impl VirtualBranchHunk {
     pub(crate) fn gen_id(new_start: u32, new_lines: u32) -> String {

crates/gitbutler-branch-actions/src/status.rs

@@ -1,10 +1,13 @@
+use std::collections::HashSet;
 use std::{collections::HashMap, path::PathBuf, vec};
 
+use crate::file::list_virtual_commit_files;
 use crate::integration::get_workspace_head;
+use crate::BranchStatus;
 use crate::{
     conflicts::RepoConflictsExt,
     file::{virtual_hunks_into_virtual_files, VirtualBranchFile},
-    hunk::{file_hunks_from_diffs, HunkLock, VirtualBranchHunk},
+    hunk::{file_hunks_from_diffs, VirtualBranchHunk},
     BranchManagerExt, VirtualBranchesExt,
 };
 use anyhow::{bail, Context, Result};
@@ -13,9 +16,15 @@ use gitbutler_branch::BranchCreateRequest;
 use gitbutler_cherry_pick::RepositoryExt as _;
 use gitbutler_command_context::CommandContext;
 use gitbutler_diff::{diff_files_into_hunks, GitHunk, Hunk, HunkHash};
+use gitbutler_hunk_dependency::{
+    compute_hunk_locks, HunkDependencyOptions, HunkLock, InputCommit, InputDiff, InputFile,
+    InputStack,
+};
 use gitbutler_operating_modes::assure_open_workspace_mode;
 use gitbutler_project::access::WorktreeWritePermission;
+use gitbutler_repo::{LogUntil, RepositoryExt as _};
 use gitbutler_stack::{BranchOwnershipClaims, OwnershipClaim, Stack, StackId};
+use itertools::Itertools;
 use tracing::instrument;
 
 /// Represents the uncommitted status of the applied virtual branches in the workspace.
@@ -47,15 +56,12 @@ pub fn get_applied_status_cached(
     worktree_changes: Option<gitbutler_diff::DiffByPathMap>,
 ) -> Result<VirtualBranchesStatus> {
     assure_open_workspace_mode(ctx).context("ng applied status requires open workspace mode")?;
+    let workspace_head = get_workspace_head(ctx)?;
     let mut virtual_branches = ctx
         .project()
         .virtual_branches()
         .list_branches_in_workspace()?;
     let base_file_diffs = worktree_changes.map(Ok).unwrap_or_else(|| {
-        // TODO(ST): Ideally, we can avoid calling `get_workspace_head()` as everyone who modifies
-        //           any of its inputs will update the intragration commit right away.
-        //           It's for another day though - right now the integration commit may be slightly stale.
-        let workspace_head = get_workspace_head(ctx)?;
         gitbutler_diff::workdir(ctx.repository(), workspace_head.to_owned())
             .context("failed to diff workdir")
     })?;
@@ -90,11 +96,20 @@ pub fn get_applied_status_cached(
             .collect();
 
     let vb_state = ctx.project().virtual_branches();
-    let base_tree = ctx
-        .repository()
-        .find_commit(vb_state.get_default_target()?.sha)?
-        .tree()?;
-    let locks = compute_locks(ctx.repository(), &base_diffs, &virtual_branches, base_tree)?;
+    let default_target = vb_state.get_default_target()?;
+
+    let locks = if ctx.project().use_new_locking {
+        compute_locks(
+            ctx,
+            &workspace_head,
+            &default_target.sha,
+            &base_diffs,
+            &virtual_branches,
+        )?
+    } else {
+        let base_tree = ctx.repository().find_commit(default_target.sha)?.tree()?;
+        compute_old_locks(ctx.repository(), &base_diffs, &virtual_branches, base_tree)?
+    };
 
     for branch in &mut virtual_branches {
         if let Err(e) = branch.initialize(ctx) {
@@ -245,6 +260,80 @@ pub fn get_applied_status_cached(
 }
 
 fn compute_locks(
+    ctx: &CommandContext,
+    workspace_head: &git2::Oid,
+    target_sha: &git2::Oid,
+    base_diffs: &BranchStatus,
+    stacks: &Vec<Stack>,
+) -> Result<HashMap<HunkHash, Vec<HunkLock>>> {
+    let repo = ctx.repository();
+    let base_commit = repo.find_commit(*target_sha)?;
+    let workspace_commit = repo.find_commit(*workspace_head)?;
+
+    let diff = &ctx.repository().diff_tree_to_tree(
+        Some(&base_commit.tree()?),
+        Some(&workspace_commit.tree()?),
+        None,
+    )?;
+
+    let files_touched_by_commits = diff
+        .deltas()
+        .filter_map(|d| d.new_file().path())
+        .map(|c| c.to_path_buf())
+        .unique()
+        .sorted()
+        .collect::<HashSet<_>>();
+    let files_touched_by_diffs = base_diffs.keys().cloned().collect::<HashSet<_>>();
+
+    let touched_by_both = files_touched_by_commits
+        .intersection(&files_touched_by_diffs)
+        .cloned()
+        .collect_vec();
+
+    let mut stacks_input: Vec<InputStack> = vec![];
+    for stack in stacks {
+        let mut commits_input: Vec<InputCommit> = vec![];
+        let commit_ids = repo.l(stack.head(), LogUntil::Commit(*target_sha), false)?;
+        for commit_id in commit_ids {
+            let mut files_input: Vec<InputFile> = vec![];
+            let commit = repo.find_commit(commit_id)?;
+            let files = list_virtual_commit_files(ctx, &commit, false)?;
+            for file in files {
+                if touched_by_both.contains(&file.path) {
+                    let value = InputFile {
+                        path: file.path,
+                        diffs: file
+                            .hunks
+                            .iter()
+                            .map(|hunk| InputDiff {
+                                old_start: hunk.old_start,
+                                old_lines: hunk.old_lines,
+                                new_start: hunk.start,
+                                new_lines: hunk.end - hunk.start,
+                            })
+                            .collect::<Vec<_>>(),
+                    };
+                    files_input.push(value);
+                }
+            }
+            commits_input.push(InputCommit {
+                commit_id,
+                files: files_input,
+            });
+        }
+        stacks_input.push(InputStack {
+            stack_id: stack.id,
+            commits: commits_input,
+        });
+    }
+
+    compute_hunk_locks(HunkDependencyOptions {
+        workdir: base_diffs,
+        stacks: stacks_input,
+    })
+}
+
+fn compute_old_locks(
     repository: &git2::Repository,
     unstaged_hunks_by_path: &HashMap<PathBuf, Vec<gitbutler_diff::GitHunk>>,
     virtual_branches: &[Stack],

crates/gitbutler-branch-actions/tests/virtual_branches/locking.rs

@@ -0,0 +1,131 @@
+use super::*;
+use gitbutler_branch::BranchCreateRequest;
+use gitbutler_branch_actions::{
+    create_commit, create_virtual_branch, list_virtual_branches, set_base_branch,
+};
+
+// This test ensures hunk lock detection works when a lines are shifted.
+//
+// The idea is you introduce change A, then you shift it down in a a different
+// (default) branch with commit B, so that new line numbers no longer intersect
+// with those applied to the isolated branch.
+#[tokio::test]
+async fn hunk_locking_confused_by_line_number_shift() -> anyhow::Result<()> {
+    let Test {
+        repository,
+        project,
+        ..
+    } = &Test::default();
+    let mut lines = repository.gen_file("file.txt", 9);
+    repository.commit_all("initial commit");
+    repository.push();
+
+    set_base_branch(project, &"refs/remotes/origin/master".parse().unwrap()).unwrap();
+
+    // Introduce a change that should lock the last change to the first branch.
+    lines[8] = "modification 1 to line 8".to_string();
+    repository.write_file("file.txt", &lines);
+
+    // We're forced to call this before making a second commit.
+    let (branches, _) = list_virtual_branches(project).unwrap();
+    create_commit(project, branches[0].id, "first commit", None, false)?;
+
+    let (branches, _) = list_virtual_branches(project).unwrap();
+    assert_eq!(branches[0].files.len(), 0);
+    assert_eq!(branches[0].commits.len(), 1);
+
+    // Commit some changes to the second branch that will push the first
+    // changes down when diffing workspace head against the default target.
+    create_virtual_branch(
+        project,
+        &BranchCreateRequest {
+            selected_for_changes: Some(true),
+            ..Default::default()
+        },
+    )
+    .unwrap();
+
+    let new_lines: Vec<_> = (0_i32..5_i32)
+        .map(|i| format!("inserted line {}", i))
+        .collect();
+    lines.splice(0..0, new_lines);
+    repository.write_file("file.txt", &lines);
+
+    // We're forced to call this before making a second commit.
+    let (branches, _) = list_virtual_branches(project).unwrap();
+    create_commit(project, branches[1].id, "second commit", None, false)?;
+
+    // At this point we expect no uncommitted files, and one commit per branch.
+    let (branches, _) = list_virtual_branches(project).unwrap();
+    assert_eq!(branches[0].commits.len(), 1);
+    assert_eq!(branches[0].files.len(), 0);
+    assert_eq!(branches[1].commits.len(), 1);
+    assert_eq!(branches[1].files.len(), 0);
+
+    // Now we change line we already changed in the first commit.
+    lines[13] = "modification 2 to original line 8".to_string();
+    repository.write_file("file.txt", &lines);
+
+    // And ensure that the new change is assigned to the first branch, despite the second
+    // branch being default for new changes.
+    let (branches, _) = list_virtual_branches(project).unwrap();
+    assert_eq!(branches[0].files.len(), 1);
+
+    // For good measure, let's ensure the hunk lock points to the right branch.
+    let branch = &branches[0];
+    let file = &branch.files[0];
+    let hunk_locks = file.hunks[0].locked_to.clone().unwrap();
+    assert_eq!(hunk_locks[0].branch_id, branch.id);
+    Ok(())
+}
+
+// This test ensures hunk lock detection works when a lines are shifted.
+//
+// The idea is you introduce change A, then you shift it down in a a different
+// (default) branch with commit B, so that new line numbers no longer intersect
+// with those applied to the isolated branch.
+#[tokio::test]
+async fn hunk_locking_with_deleted_lines_only() -> anyhow::Result<()> {
+    let Test {
+        repository,
+        project,
+        ..
+    } = &Test::default();
+    repository.gen_file("file.txt", 3);
+    repository.commit_all("initial commit");
+    repository.push();
+
+    set_base_branch(project, &"refs/remotes/origin/master".parse().unwrap()).unwrap();
+
+    // Introduce a change that should lock the last change to the first branch.
+    let mut lines = repository.gen_file("file.txt", 2);
+    lines[1] = "line 2".to_string();
+    repository.write_file("file.txt", &lines);
+
+    // We have to do this before creating a commit.
+    let (branches, _) = list_virtual_branches(project).unwrap();
+    create_commit(project, branches[0].id, "first commit", None, false)?;
+
+    let (branches, _) = list_virtual_branches(project).unwrap();
+    assert_eq!(branches[0].commits.len(), 1);
+    assert_eq!(branches[0].files.len(), 0);
+
+    // Commit some changes to the second branch that will push the first changes
+    // down when diffing workspace head against the default target.
+    create_virtual_branch(
+        project,
+        &BranchCreateRequest {
+            selected_for_changes: Some(true),
+            ..Default::default()
+        },
+    )
+    .unwrap();
+
+    lines[1] = "modified line 2".to_string();
+    repository.write_file("file.txt", &lines);
+
+    let (branches, _) = list_virtual_branches(project)?;
+    assert_eq!(branches[0].files.len(), 1);
+    assert_eq!(branches[1].files.len(), 0);
+    Ok(())
+}

crates/gitbutler-branch-actions/tests/virtual_branches/mod.rs

@@ -30,9 +30,11 @@ impl Default for Test {
         let projects = projects::Controller::from_path(data_dir.path());
 
         let test_project = TestProject::default();
-        let project = projects
+        let mut project = projects
             .add(test_project.path())
             .expect("failed to add project");
+        // TODO: Remove after transition is complete.
+        project.use_new_locking = true;
 
         Self {
             repository: test_project,
@@ -62,6 +64,7 @@ mod init;
 mod insert_blank_commit;
 mod list;
 mod list_details;
+mod locking;
 mod move_commit_file;
 mod move_commit_to_vbranch;
 mod oplog;

crates/gitbutler-project/src/project.rs

@@ -97,6 +97,9 @@ pub struct Project {
     pub snapshot_lines_threshold: Option<usize>,
     #[serde(default)]
     pub git_host: GitHostSettings,
+    // Experimental flag for new hunk dependency algorithm
+    #[serde(default)]
+    pub use_new_locking: bool,
 }
 
 #[derive(Debug, Deserialize, Serialize, Clone, Default)]

crates/gitbutler-project/src/storage.rs

@@ -28,6 +28,7 @@ pub struct UpdateRequest {
     pub use_diff_context: Option<bool>,
     pub snapshot_lines_threshold: Option<usize>,
     pub git_host: Option<GitHostSettings>,
+    pub use_new_locking: Option<bool>,
 }
 
 impl Storage {
@@ -128,6 +129,10 @@ impl Storage {
             project.git_host = git_host.clone();
         }
 
+        if let Some(use_new_locking) = &update_request.use_new_locking {
+            project.use_new_locking = *use_new_locking;
+        }
+
         self.inner
             .write(PROJECTS_FILE, &serde_json::to_string_pretty(&projects)?)?;