Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove feature flag for new locking algorithm #5331

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion apps/desktop/src/lib/backend/projects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -15,10 +15,8 @@
import { invoke } from '@tauri-apps/api/tauri';
import { onMount } from 'svelte';

const projectsService = getContext(ProjectsService);
const project = getContext(Project);

let useNewLocking = project?.use_new_locking || false;
let signCommits = false;

const gitConfig = getContext(GitConfigService);
Expand Down Expand Up @@ -77,13 +75,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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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);
}
Expand Down Expand Up @@ -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>
108 changes: 8 additions & 100 deletions crates/gitbutler-branch-actions/src/status.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
}
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 0 additions & 3 deletions crates/gitbutler-project/src/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
5 changes: 0 additions & 5 deletions crates/gitbutler-project/src/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)?)?;

Expand Down
Loading