From 61d6494cbdb5f6c1a8afcff637c52541e1a4b765 Mon Sep 17 00:00:00 2001
From: Mattias Granlund <mtsgrd@gmail.com>
Date: Mon, 28 Oct 2024 14:49:21 +0100
Subject: [PATCH] Remove feature flag for new locking algorithm

---
 apps/desktop/src/lib/backend/projects.ts      |   1 -
 .../userPreferences/CommitSigningForm.svelte  |  10 +-
 .../userPreferences/PreferencesForm.svelte    |  19 ---
 crates/gitbutler-branch-actions/src/status.rs | 108 ++----------------
 .../tests/virtual_branches/mod.rs             |   4 +-
 crates/gitbutler-project/src/project.rs       |   3 -
 crates/gitbutler-project/src/storage.rs       |   5 -
 7 files changed, 10 insertions(+), 140 deletions(-)

diff --git a/apps/desktop/src/lib/backend/projects.ts b/apps/desktop/src/lib/backend/projects.ts
index 8001c987d2..878b1447d7 100644
--- a/apps/desktop/src/lib/backend/projects.ts
+++ b/apps/desktop/src/lib/backend/projects.ts
@@ -29,7 +29,6 @@ export class Project {
 	omit_certificate_check: boolean | undefined;
 	use_diff_context: boolean | undefined;
 	snapshot_lines_threshold!: number | undefined;
-	use_new_locking!: boolean;
 	git_host!: {
 		hostType: HostType | undefined;
 		reviewTemplatePath: string | undefined;
diff --git a/apps/desktop/src/lib/settings/userPreferences/CommitSigningForm.svelte b/apps/desktop/src/lib/settings/userPreferences/CommitSigningForm.svelte
index ed1434dd36..7dc4497df4 100644
--- a/apps/desktop/src/lib/settings/userPreferences/CommitSigningForm.svelte
+++ b/apps/desktop/src/lib/settings/userPreferences/CommitSigningForm.svelte
@@ -1,6 +1,6 @@
 <script lang="ts">
 	import { GitConfigService } from '$lib/backend/gitConfigService';
-	import { Project, ProjectsService } from '$lib/backend/projects';
+	import { Project } from '$lib/backend/projects';
 	import SectionCard from '$lib/components/SectionCard.svelte';
 	import SectionCardDisclaimer from '$lib/components/SectionCardDisclaimer.svelte';
 	import Select from '$lib/select/Select.svelte';
@@ -18,7 +18,6 @@
 	const projectsService = getContext(ProjectsService);
 	const project = getContext(Project);
 
-	let useNewLocking = project?.use_new_locking || false;
 	let signCommits = false;
 
 	const gitConfig = getContext(GitConfigService);
@@ -77,13 +76,6 @@
 		await gitConfig.setGbConfig(project.id, signUpdate);
 	}
 
-	async function setUseNewLocking(value: boolean) {
-		project.use_new_locking = value;
-		await projectsService.updateProject(project);
-	}
-
-	$: setUseNewLocking(useNewLocking);
-
 	onMount(async () => {
 		let gitConfigSettings = await gitConfig.getGbConfig(project.id);
 		signCommits = gitConfigSettings.signCommits || false;
diff --git a/apps/desktop/src/lib/settings/userPreferences/PreferencesForm.svelte b/apps/desktop/src/lib/settings/userPreferences/PreferencesForm.svelte
index 63af520d5f..ef5a4be573 100644
--- a/apps/desktop/src/lib/settings/userPreferences/PreferencesForm.svelte
+++ b/apps/desktop/src/lib/settings/userPreferences/PreferencesForm.svelte
@@ -13,7 +13,6 @@
 	let snaphotLinesThreshold = project?.snapshot_lines_threshold || 20; // when undefined, the default is 20
 
 	let omitCertificateCheck = project?.omit_certificate_check;
-	let useNewLocking = project?.use_new_locking || false;
 
 	const runCommitHooks = projectRunCommitHooks(project.id);
 
@@ -27,13 +26,6 @@
 		await projectsService.updateProject(project);
 	}
 
-	async function setUseNewLocking(value: boolean) {
-		project.use_new_locking = value;
-		await projectsService.updateProject(project);
-	}
-
-	$: setUseNewLocking(useNewLocking);
-
 	async function handleOmitCertificateCheckClick(event: MouseEvent) {
 		await setOmitCertificateCheck((event.target as HTMLInputElement)?.checked);
 	}
@@ -86,15 +78,4 @@
 			/>
 		</svelte:fragment>
 	</SectionCard>
-
-	<SectionCard labelFor="useNewLocking" orientation="row">
-		<svelte:fragment slot="title">Use new experimental hunk locking algorithm</svelte:fragment>
-		<svelte:fragment slot="caption">
-			This new hunk locking algorithm is still in the testing phase but should more accurately catch
-			locks and subsequently cause fewer errors.
-		</svelte:fragment>
-		<svelte:fragment slot="actions">
-			<Toggle id="useNewLocking" bind:checked={useNewLocking} />
-		</svelte:fragment>
-	</SectionCard>
 </Section>
diff --git a/crates/gitbutler-branch-actions/src/status.rs b/crates/gitbutler-branch-actions/src/status.rs
index 31096eb61a..99b582cd4f 100644
--- a/crates/gitbutler-branch-actions/src/status.rs
+++ b/crates/gitbutler-branch-actions/src/status.rs
@@ -11,11 +11,9 @@ use crate::{
     BranchManagerExt, VirtualBranchesExt,
 };
 use anyhow::{bail, Context, Result};
-use git2::Tree;
 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_diff::{diff_files_into_hunks, Hunk, HunkHash};
 use gitbutler_hunk_dependency::{
     compute_hunk_locks, HunkDependencyOptions, HunkLock, InputCommit, InputDiff, InputFile,
     InputStack,
@@ -98,18 +96,13 @@ pub fn get_applied_status_cached(
     let vb_state = ctx.project().virtual_branches();
     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)?
-    };
+    let locks = compute_locks(
+        ctx,
+        &workspace_head,
+        &default_target.sha,
+        &base_diffs,
+        &virtual_branches,
+    )?;
 
     for branch in &mut virtual_branches {
         if let Err(e) = branch.initialize(ctx) {
@@ -332,88 +325,3 @@ fn compute_locks(
         stacks: stacks_input,
     })
 }
-
-fn compute_old_locks(
-    repository: &git2::Repository,
-    unstaged_hunks_by_path: &HashMap<PathBuf, Vec<gitbutler_diff::GitHunk>>,
-    virtual_branches: &[Stack],
-    base_tree: Tree,
-) -> Result<HashMap<HunkHash, Vec<HunkLock>>> {
-    let mut diff_opts = git2::DiffOptions::new();
-    let opts = diff_opts
-        .show_binary(true)
-        .ignore_submodules(true)
-        .context_lines(3);
-
-    let branch_path_diffs = virtual_branches
-        .iter()
-        .filter_map(|branch| {
-            let commit = repository.find_commit(branch.head()).ok()?;
-            let tree = repository
-                .find_real_tree(&commit, Default::default())
-                .ok()?;
-            let diff = repository
-                .diff_tree_to_tree(Some(&base_tree), Some(&tree), Some(opts))
-                .ok()?;
-            let hunks_by_filepath =
-                gitbutler_diff::hunks_by_filepath(Some(repository), &diff).ok()?;
-
-            Some((branch, hunks_by_filepath))
-        })
-        .collect::<Vec<_>>();
-
-    let mut workspace_hunks_by_path =
-        HashMap::<PathBuf, Vec<(gitbutler_diff::GitHunk, &Stack)>>::new();
-
-    for (branch, hunks_by_filepath) in branch_path_diffs {
-        for (path, hunks) in hunks_by_filepath {
-            workspace_hunks_by_path.entry(path).or_default().extend(
-                hunks
-                    .hunks
-                    .iter()
-                    .map(|hunk| (hunk.clone(), branch))
-                    .collect::<Vec<_>>(),
-            );
-        }
-    }
-
-    let locked_hunks = unstaged_hunks_by_path
-        .iter()
-        .filter_map(|(path, hunks)| {
-            let workspace_hunks = workspace_hunks_by_path.get(path)?;
-
-            let (unapplied_hunk, branches) = hunks.iter().find_map(|unapplied_hunk| {
-                // Find all branches that have a hunk that intersects with the unapplied hunk
-                let locked_to = workspace_hunks
-                    .iter()
-                    .filter_map(|(workspace_hunk, branch)| {
-                        if GitHunk::workspace_intersects_unapplied(workspace_hunk, unapplied_hunk) {
-                            Some(*branch)
-                        } else {
-                            None
-                        }
-                    })
-                    .collect::<Vec<_>>();
-                if locked_to.is_empty() {
-                    None
-                } else {
-                    Some((unapplied_hunk, locked_to))
-                }
-            })?;
-
-            let hash = Hunk::hash_diff(&unapplied_hunk.diff_lines);
-            let locks = branches
-                .iter()
-                .map(|b| HunkLock {
-                    branch_id: b.id,
-                    commit_id: b.head(),
-                })
-                .collect::<Vec<_>>();
-
-            // For now we're returning an array of locks to align with the original type, even though this implementation doesn't give multiple locks for the same hunk
-            Some((hash, locks))
-        })
-        .collect::<HashMap<_, _>>();
-
-    Ok(locked_hunks)
-}
diff --git a/crates/gitbutler-branch-actions/tests/virtual_branches/mod.rs b/crates/gitbutler-branch-actions/tests/virtual_branches/mod.rs
index f181eb677b..7ae5c4955e 100644
--- a/crates/gitbutler-branch-actions/tests/virtual_branches/mod.rs
+++ b/crates/gitbutler-branch-actions/tests/virtual_branches/mod.rs
@@ -30,11 +30,9 @@ impl Default for Test {
         let projects = projects::Controller::from_path(data_dir.path());
 
         let test_project = TestProject::default();
-        let mut project = projects
+        let 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,
diff --git a/crates/gitbutler-project/src/project.rs b/crates/gitbutler-project/src/project.rs
index b69499e866..c68047c03f 100644
--- a/crates/gitbutler-project/src/project.rs
+++ b/crates/gitbutler-project/src/project.rs
@@ -97,9 +97,6 @@ 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)]
diff --git a/crates/gitbutler-project/src/storage.rs b/crates/gitbutler-project/src/storage.rs
index b8abbdf768..a3fbb80f97 100644
--- a/crates/gitbutler-project/src/storage.rs
+++ b/crates/gitbutler-project/src/storage.rs
@@ -28,7 +28,6 @@ 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 {
@@ -129,10 +128,6 @@ 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)?)?;