Skip to content

Commit

Permalink
Remove feature flag for new locking algorithm
Browse files Browse the repository at this point in the history
  • Loading branch information
@mtsgrd
mtsgrd committed Oct 28, 2024
1 parent 7a03e89 commit 0eae43e
Show file tree
Hide file tree
Showing 7 changed files with 8 additions and 138 deletions.
1 change: 0 additions & 1 deletion apps/desktop/src/lib/backend/projects.ts
View file
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
8 changes: 0 additions & 8 deletions apps/desktop/src/lib/settings/userPreferences/CommitSigningForm.svelte
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
const projectsService = getContext(ProjectsService);

Check failure on line 18 in apps/desktop/src/lib/settings/userPreferences/CommitSigningForm.svelte

View workflow job for this annotation

GitHub Actions / lint-node 

'projectsService' is assigned a value but never used. Allowed unused vars must match /^_/u
const project = getContext(Project);
let useNewLocking = project?.use_new_locking || false;
let signCommits = false;
const gitConfig = getContext(GitConfigService);
Expand Down Expand Up @@ -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;
Expand Down
19 changes: 0 additions & 19 deletions apps/desktop/src/lib/settings/userPreferences/PreferencesForm.svelte
View file
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
View file
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)
}
2 changes: 0 additions & 2 deletions crates/gitbutler-branch-actions/tests/virtual_branches/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,6 @@ impl Default for Test {
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,
Expand Down
3 changes: 0 additions & 3 deletions crates/gitbutler-project/src/project.rs
View file
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
View file
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

0 comments on commit 0eae43e

Please sign in to comment.