Skip to content

Commit 7ce1439

Browse files
committed
Do not dynamically calculate the head commit when listing virtual branches.
That way, the head is static which allows to re-use the worktree status across multiple related calls. Also, add a way to quickly get the `head_commit()` from a `git2::Repository`. Note that this may cause breakage in the app, as now it will see the actual head, not a computed one, but that should also help us to find places that didn't properly update their state.
1 parent fc63929 commit 7ce1439

File tree

8 files changed

+41
-22
lines changed

8 files changed

+41
-22
lines changed

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

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use gitbutler_branch::{
88
use gitbutler_command_context::{CommandContext, GixRepositoryExt};
99
use gitbutler_project::access::WorktreeReadPermission;
1010
use gitbutler_reference::normalize_branch_name;
11+
use gitbutler_repo::RepositoryExt;
1112
use gitbutler_serde::BStringForFrontend;
1213
use gix::object::tree::diff::Action;
1314
use gix::prelude::ObjectIdExt;
@@ -27,12 +28,7 @@ pub(crate) fn get_uncommited_files(
2728
_permission: &WorktreeReadPermission,
2829
) -> Result<Vec<RemoteBranchFile>> {
2930
let repository = context.repository();
30-
let head_commit = repository
31-
.head()
32-
.context("Failed to get head")?
33-
.peel_to_commit()
34-
.context("Failed to get head commit")?;
35-
31+
let head_commit = repository.head_commit()?;
3632
let files = gitbutler_diff::workdir(repository, &head_commit.id())
3733
.context("Failed to list uncommited files")?
3834
.into_iter()

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

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
use std::{collections::HashMap, path::PathBuf, vec};
22

3+
use crate::{
4+
conflicts::RepoConflictsExt,
5+
file::{virtual_hunks_into_virtual_files, VirtualBranchFile},
6+
hunk::{file_hunks_from_diffs, HunkLock, VirtualBranchHunk},
7+
BranchManagerExt, VirtualBranchesExt,
8+
};
39
use anyhow::{bail, Context, Result};
410
use git2::Tree;
511
use gitbutler_branch::{
@@ -10,16 +16,9 @@ use gitbutler_command_context::CommandContext;
1016
use gitbutler_diff::{diff_files_into_hunks, GitHunk, Hunk, HunkHash};
1117
use gitbutler_operating_modes::assure_open_workspace_mode;
1218
use gitbutler_project::access::WorktreeWritePermission;
19+
use gitbutler_repo::RepositoryExt;
1320
use tracing::instrument;
1421

15-
use crate::{
16-
conflicts::RepoConflictsExt,
17-
file::{virtual_hunks_into_virtual_files, VirtualBranchFile},
18-
hunk::{file_hunks_from_diffs, HunkLock, VirtualBranchHunk},
19-
integration::get_workspace_head,
20-
BranchManagerExt, VirtualBranchesExt,
21-
};
22-
2322
/// Represents the uncommitted status of the applied virtual branches in the workspace.
2423
pub struct VirtualBranchesStatus {
2524
/// A collection of branches and their associated uncommitted file changes.
@@ -37,13 +36,18 @@ pub fn get_applied_status(
3736
perm: Option<&mut WorktreeWritePermission>,
3837
) -> Result<VirtualBranchesStatus> {
3938
assure_open_workspace_mode(ctx).context("ng applied status requires open workspace mode")?;
40-
let integration_commit = get_workspace_head(ctx)?;
39+
// TODO(ST): this was `get_workspace_head()`, which is slow and ideally, we don't dynamically
40+
// calculate which should already be 'fixed' - why do we have the integration branch
41+
// if we can't assume it's in the right state? So ideally, we assure that the code
42+
// that affects the integration branch also updates it?
43+
let integration_commit_id = ctx.repository().head_commit()?.id();
4144
let mut virtual_branches = ctx
4245
.project()
4346
.virtual_branches()
4447
.list_branches_in_workspace()?;
45-
let base_file_diffs = gitbutler_diff::workdir(ctx.repository(), &integration_commit.to_owned())
46-
.context("failed to diff workdir")?;
48+
let base_file_diffs =
49+
gitbutler_diff::workdir(ctx.repository(), &integration_commit_id.to_owned())
50+
.context("failed to diff workdir")?;
4751

4852
let mut skipped_files: Vec<gitbutler_diff::FileDiff> = Vec::new();
4953
for file_diff in base_file_diffs.values() {

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -973,6 +973,7 @@ fn merge_vbranch_upstream_conflict() -> Result<()> {
973973
)?;
974974

975975
// make gb see the conflict resolution
976+
gitbutler_branch_actions::update_gitbutler_integration(&vb_state, ctx)?;
976977
let (branches, _) = list_virtual_branches(ctx, guard.write_permission())?;
977978
assert!(branches[0].conflicted);
978979

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,3 @@
1-
use gitbutler_branch::BranchCreateRequest;
2-
use gitbutler_reference::Refname;
3-
41
use super::*;
52

63
#[test]
@@ -101,6 +98,9 @@ fn conflicting() {
10198
"<<<<<<< ours\nconflict\n=======\nsecond\n>>>>>>> theirs\n"
10299
);
103100

101+
let vb_state = VirtualBranchesHandle::new(project.gb_dir());
102+
let ctx = CommandContext::open(project).unwrap();
103+
update_gitbutler_integration(&vb_state, &ctx).unwrap();
104104
let (branches, _) = controller.list_virtual_branches(project).unwrap();
105105

106106
assert_eq!(branches.len(), 1);

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
use std::{fs, path, path::PathBuf, str::FromStr};
22

3-
use gitbutler_branch::BranchCreateRequest;
4-
use gitbutler_branch_actions::VirtualBranchActions;
3+
use gitbutler_branch::{BranchCreateRequest, VirtualBranchesHandle};
4+
use gitbutler_branch_actions::{update_gitbutler_integration, VirtualBranchActions};
5+
use gitbutler_command_context::CommandContext;
56
use gitbutler_error::error::Marker;
67
use gitbutler_project::{self as projects, Project, ProjectId};
78
use gitbutler_reference::Refname;
@@ -137,6 +138,9 @@ fn resolve_conflict_flow() {
137138
.create_virtual_branch_from_branch(project, &unapplied_branch, None)
138139
.unwrap();
139140

141+
let vb_state = VirtualBranchesHandle::new(project.gb_dir());
142+
let ctx = CommandContext::open(project).unwrap();
143+
update_gitbutler_integration(&vb_state, &ctx).unwrap();
140144
let (branches, _) = controller.list_virtual_branches(project).unwrap();
141145
assert_eq!(branches.len(), 1);
142146
assert!(branches[0].active);

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -731,6 +731,9 @@ mod applied_branch {
731731
)
732732
.unwrap();
733733

734+
let vb_state = VirtualBranchesHandle::new(project.gb_dir());
735+
let ctx = CommandContext::open(project).unwrap();
736+
update_gitbutler_integration(&vb_state, &ctx).unwrap();
734737
let (branches, _) = controller.list_virtual_branches(project).unwrap();
735738
assert_eq!(branches.len(), 1);
736739
assert!(branches[0].active);

crates/gitbutler-oplog/src/oplog.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,7 @@ impl OplogExt for Project {
260260
restore_snapshot(self, snapshot_commit_id, guard.write_permission())
261261
}
262262

263+
#[instrument(level = tracing::Level::DEBUG, skip(self), err(Debug))]
263264
fn should_auto_snapshot(&self, check_if_last_snapshot_older_than: Duration) -> Result<bool> {
264265
let last_snapshot_time = OplogHandle::new(&self.gb_dir()).modified_at()?;
265266
if last_snapshot_time.elapsed()? <= check_if_last_snapshot_older_than {
@@ -711,6 +712,7 @@ fn write_conflicts_tree(
711712

712713
/// Exclude files that are larger than the limit (eg. database.sql which may never be intended to be committed)
713714
/// TODO(ST): refactor this to be path-safe and ' ' save - the returned list is space separated (!!)
715+
#[instrument(level = tracing::Level::DEBUG, skip(repo), err(Debug))]
714716
fn worktree_files_larger_than_limit_as_git2_ignore_rule(
715717
repo: &git2::Repository,
716718
worktree_dir: &Path,
@@ -765,6 +767,7 @@ fn lines_since_snapshot(project: &Project, repo: &git2::Repository) -> Result<us
765767
Ok(lines_changed)
766768
}
767769

770+
#[instrument(level = tracing::Level::DEBUG, skip(branch, repo), err(Debug))]
768771
fn branch_lines_since_snapshot(
769772
branch: &Branch,
770773
repo: &git2::Repository,

crates/gitbutler-repo/src/repository_ext.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ use tracing::instrument;
1717
///
1818
/// For now, it collects useful methods from `gitbutler-core::git::Repository`
1919
pub trait RepositoryExt {
20+
fn head_commit(&self) -> Result<git2::Commit<'_>>;
2021
fn remote_branches(&self) -> Result<Vec<RemoteRefname>>;
2122
fn remotes_as_string(&self) -> Result<Vec<String>>;
2223
/// Open a new in-memory repository and executes the provided closure using it.
@@ -72,6 +73,13 @@ pub trait RepositoryExt {
7273
}
7374

7475
impl RepositoryExt for git2::Repository {
76+
fn head_commit(&self) -> Result<git2::Commit<'_>> {
77+
self.head()
78+
.context("Failed to get head")?
79+
.peel_to_commit()
80+
.context("Failed to get head commit")
81+
}
82+
7583
fn in_memory<T, F>(&self, f: F) -> Result<T>
7684
where
7785
F: FnOnce(&git2::Repository) -> Result<T>,

0 commit comments

Comments
 (0)