Reuse the worktree-diff during 'recalulate-everything'

It's used for emission of changed files, but also for listing
branch information.

Make sure we only run the worktree-status once to save time.

This also unifies the acquisition of the `HEAD^{commit}`,
which one day will be a natural feature once `gix::Repository`
is used.

Author: Byron

Cargo.lock

@@ -1,8 +1,22 @@
+use super::r#virtual as branch;
+use crate::branch::get_uncommited_files_raw;
+use crate::{
+    base::{
+        get_base_branch_data, set_base_branch, set_target_push_remote, update_base_branch,
+        BaseBranch,
+    },
+    branch::get_uncommited_files,
+    branch_manager::BranchManagerExt,
+    file::RemoteBranchFile,
+    remote::{get_branch_data, list_remote_branches, RemoteBranch, RemoteBranchData},
+    VirtualBranchesExt,
+};
 use anyhow::{Context, Result};
 use gitbutler_branch::{
     BranchCreateRequest, BranchId, BranchOwnershipClaims, BranchUpdateRequest, ChangeReference,
 };
 use gitbutler_command_context::CommandContext;
+use gitbutler_diff::DiffByPathMap;
 use gitbutler_operating_modes::assure_open_workspace_mode;
 use gitbutler_oplog::{
     entry::{OperationKind, SnapshotDetails},
@@ -13,19 +27,6 @@ use gitbutler_reference::{ReferenceName, Refname, RemoteRefname};
 use gitbutler_repo::{credentials::Helper, RepoActionsExt, RepositoryExt};
 use tracing::instrument;
 
-use super::r#virtual as branch;
-use crate::{
-    base::{
-        get_base_branch_data, set_base_branch, set_target_push_remote, update_base_branch,
-        BaseBranch,
-    },
-    branch::get_uncommited_files,
-    branch_manager::BranchManagerExt,
-    file::RemoteBranchFile,
-    remote::{get_branch_data, list_remote_branches, RemoteBranch, RemoteBranchData},
-    VirtualBranchesExt,
-};
-
 #[derive(Clone, Copy, Default)]
 pub struct VirtualBranchActions;
 
@@ -81,6 +82,24 @@ impl VirtualBranchActions {
             .map_err(Into::into)
     }
 
+    pub fn list_virtual_branches_cached(
+        &self,
+        project: &Project,
+        worktree_changes: Option<DiffByPathMap>,
+    ) -> Result<(Vec<branch::VirtualBranch>, Vec<gitbutler_diff::FileDiff>)> {
+        let ctx = open_with_verify(project)?;
+
+        assure_open_workspace_mode(&ctx)
+            .context("Listing virtual branches requires open workspace mode")?;
+
+        branch::list_virtual_branches_cached(
+            &ctx,
+            project.exclusive_worktree_access().write_permission(),
+            worktree_changes,
+        )
+        .map_err(Into::into)
+    }
+
     pub fn create_virtual_branch(
         &self,
         project: &Project,
@@ -569,11 +588,17 @@ impl VirtualBranchActions {
 
     pub fn get_uncommited_files(&self, project: &Project) -> Result<Vec<RemoteBranchFile>> {
         let context = CommandContext::open(project)?;
-
         let guard = project.exclusive_worktree_access();
-
         get_uncommited_files(&context, guard.read_permission())
     }
+
+    /// Like [`get_uncommited_files()`], but returns a type that can be re-used with
+    /// [`crate::list_virtual_branches()`].
+    pub fn get_uncommited_files_reusable(&self, project: &Project) -> Result<DiffByPathMap> {
+        let context = CommandContext::open(project)?;
+        let guard = project.exclusive_worktree_access();
+        get_uncommited_files_raw(&context, guard.read_permission())
+    }
 }
 
 fn open_with_verify(project: &Project) -> Result<CommandContext> {

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

@@ -6,6 +6,7 @@ use gitbutler_branch::{
     Branch as GitButlerBranch, BranchId, BranchIdentity, ReferenceExtGix, Target,
 };
 use gitbutler_command_context::{CommandContext, GixRepositoryExt};
+use gitbutler_diff::DiffByPathMap;
 use gitbutler_project::access::WorktreeReadPermission;
 use gitbutler_reference::normalize_branch_name;
 use gitbutler_repo::RepositoryExt;
@@ -23,14 +24,21 @@ use std::{
     vec,
 };
 
-pub(crate) fn get_uncommited_files(
+pub(crate) fn get_uncommited_files_raw(
     context: &CommandContext,
     _permission: &WorktreeReadPermission,
-) -> Result<Vec<RemoteBranchFile>> {
+) -> Result<DiffByPathMap> {
     let repository = context.repository();
     let head_commit = repository.head_commit()?;
-    let files = gitbutler_diff::workdir(repository, &head_commit.id())
-        .context("Failed to list uncommited files")?
+    Ok(gitbutler_diff::workdir(repository, &head_commit.id())
+        .context("Failed to list uncommited files")?)
+}
+
+pub(crate) fn get_uncommited_files(
+    context: &CommandContext,
+    _permission: &WorktreeReadPermission,
+) -> Result<Vec<RemoteBranchFile>> {
+    let files = get_uncommited_files_raw(context, _permission)?
         .into_iter()
         .map(|(path, file)| {
             let binary = file.hunks.iter().any(|h| h.binary);

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

@@ -27,13 +27,24 @@ pub struct VirtualBranchesStatus {
     pub skipped_files: Vec<gitbutler_diff::FileDiff>,
 }
 
+pub fn get_applied_status(
+    ctx: &CommandContext,
+    perm: Option<&mut WorktreeWritePermission>,
+) -> Result<VirtualBranchesStatus> {
+    get_applied_status_cached(ctx, perm, None)
+}
+
 /// Returns branches and their associated file changes, in addition to a list
 /// of skipped files.
+/// `worktree_changes` are all changed files against the current `HEAD^{tree}` and index
+/// against the current working tree directory, and it's used to avoid double-computing
+/// this expensive information.
 // TODO(kv): make this side effect free
-#[instrument(level = tracing::Level::DEBUG, skip(ctx, perm))]
-pub fn get_applied_status(
+#[instrument(level = tracing::Level::DEBUG, skip(ctx, perm, worktree_changes))]
+pub fn get_applied_status_cached(
     ctx: &CommandContext,
     perm: Option<&mut WorktreeWritePermission>,
+    worktree_changes: Option<gitbutler_diff::DiffByPathMap>,
 ) -> Result<VirtualBranchesStatus> {
     assure_open_workspace_mode(ctx).context("ng applied status requires open workspace mode")?;
     // TODO(ST): this was `get_workspace_head()`, which is slow and ideally, we don't dynamically
@@ -45,9 +56,10 @@ pub fn get_applied_status(
         .project()
         .virtual_branches()
         .list_branches_in_workspace()?;
-    let base_file_diffs =
+    let base_file_diffs = worktree_changes.map(Ok).unwrap_or_else(|| {
         gitbutler_diff::workdir(ctx.repository(), &integration_commit_id.to_owned())
-            .context("failed to diff workdir")?;
+            .context("failed to diff workdir")
+    })?;
 
     let mut skipped_files: Vec<gitbutler_diff::FileDiff> = Vec::new();
     for file_diff in base_file_diffs.values() {

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

@@ -13,7 +13,7 @@ use crate::{
     hunk::VirtualBranchHunk,
     integration::get_workspace_head,
     remote::{branch_to_remote_branch, RemoteBranch},
-    status::get_applied_status,
+    status::{get_applied_status, get_applied_status_cached},
     Get, VirtualBranchesExt,
 };
 use anyhow::{anyhow, bail, Context, Result};
@@ -254,13 +254,23 @@ fn resolve_old_applied_state(
 
     Ok(())
 }
-
-#[instrument(level = tracing::Level::DEBUG, skip(ctx, perm))]
 pub fn list_virtual_branches(
+    ctx: &CommandContext,
+    perm: &mut WorktreeWritePermission,
+) -> Result<(Vec<VirtualBranch>, Vec<gitbutler_diff::FileDiff>)> {
+    list_virtual_branches_cached(ctx, perm, None)
+}
+
+/// `worktree_changes` are all changed files against the current `HEAD^{tree}` and index
+/// against the current working tree directory, and it's used to avoid double-computing
+/// this expensive information.
+#[instrument(level = tracing::Level::DEBUG, skip(ctx, perm, worktree_changes))]
+pub fn list_virtual_branches_cached(
     ctx: &CommandContext,
     // TODO(ST): this should really only shared access, but there is some internals
     //           that conditionally write things.
     perm: &mut WorktreeWritePermission,
+    worktree_changes: Option<gitbutler_diff::DiffByPathMap>,
 ) -> Result<(Vec<VirtualBranch>, Vec<gitbutler_diff::FileDiff>)> {
     assure_open_workspace_mode(ctx)
         .context("Listing virtual branches requires open workspace mode")?;
@@ -274,7 +284,7 @@ pub fn list_virtual_branches(
         .get_default_target()
         .context("failed to get default target")?;
 
-    let status = get_applied_status(ctx, Some(perm))?;
+    let status = get_applied_status_cached(ctx, Some(perm), worktree_changes)?;
     let max_selected_for_changes = status
         .branches
         .iter()

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

@@ -2,7 +2,7 @@ mod diff;
 mod hunk;
 pub mod write;
 pub use diff::{
-    diff_files_into_hunks, hunks_by_filepath, reverse_hunk, trees, workdir, ChangeType, FileDiff,
-    GitHunk,
+    diff_files_into_hunks, hunks_by_filepath, reverse_hunk, trees, workdir, ChangeType,
+    DiffByPathMap, FileDiff, GitHunk,
 };
 pub use hunk::{Hunk, HunkHash};

crates/gitbutler-diff/src/lib.rs

@@ -24,6 +24,7 @@ gix = { workspace = true, features = [
 ] }
 gitbutler-command-context.workspace = true
 gitbutler-project.workspace = true
+gitbutler-diff.workspace = true
 gitbutler-user.workspace = true
 gitbutler-reference.workspace = true
 gitbutler-error.workspace = true

crates/gitbutler-watcher/Cargo.toml

@@ -1,8 +1,10 @@
 use std::{path::PathBuf, sync::Arc};
 
+use super::{events, Change};
 use anyhow::{Context, Result};
-use gitbutler_branch_actions::{VirtualBranchActions, VirtualBranches};
+use gitbutler_branch_actions::{RemoteBranchFile, VirtualBranchActions, VirtualBranches};
 use gitbutler_command_context::CommandContext;
+use gitbutler_diff::DiffByPathMap;
 use gitbutler_error::error::Marker;
 use gitbutler_operating_modes::{
     in_open_workspace_mode, in_outside_workspace_mode, operating_mode,
@@ -18,8 +20,6 @@ use gitbutler_sync::cloud::{push_oplog, push_repo};
 use gitbutler_user as users;
 use tracing::instrument;
 
-use super::{events, Change};
-
 /// A type that contains enough state to make decisions based on changes in the filesystem, which themselves
 /// may trigger [Changes](Change)
 // NOTE: This is `Clone` as each incoming event is spawned onto a thread for processing.
@@ -71,7 +71,7 @@ impl Handler {
 
             // This is only produced at the end of mutating Tauri commands to trigger a fresh state being served to the UI.
             events::InternalEvent::CalculateVirtualBranches(project_id) => self
-                .calculate_virtual_branches(project_id)
+                .calculate_virtual_branches(project_id, None)
                 .context("failed to handle virtual branch event"),
         }
     }
@@ -90,8 +90,12 @@ impl Handler {
         CommandContext::open(&project).context("Failed to create a command context")
     }
 
-    #[instrument(skip(self, project_id))]
-    fn calculate_virtual_branches(&self, project_id: ProjectId) -> Result<()> {
+    #[instrument(skip(self, project_id, worktree_changes))]
+    fn calculate_virtual_branches(
+        &self,
+        project_id: ProjectId,
+        worktree_changes: Option<DiffByPathMap>,
+    ) -> Result<()> {
         let ctx = self.open_command_context(project_id)?;
         // Skip if we're not on the open workspace mode
         if !in_open_workspace_mode(&ctx) {
@@ -102,7 +106,7 @@ impl Handler {
             .projects
             .get(project_id)
             .context("failed to get project")?;
-        match VirtualBranchActions.list_virtual_branches(&project) {
+        match VirtualBranchActions.list_virtual_branches_cached(&project, worktree_changes) {
             Ok((branches, skipped_files)) => self.emit_app_event(Change::VirtualBranches {
                 project_id: project.id,
                 virtual_branches: VirtualBranches {
@@ -126,26 +130,36 @@ impl Handler {
     fn recalculate_everything(&self, paths: Vec<PathBuf>, project_id: ProjectId) -> Result<()> {
         let ctx = self.open_command_context(project_id)?;
 
-        self.emit_uncommited_files(ctx.project());
+        let worktree_changes = self.emit_uncommited_files(ctx.project()).ok();
 
         if in_open_workspace_mode(&ctx) {
             self.maybe_create_snapshot(project_id).ok();
-            self.calculate_virtual_branches(project_id)?;
+            self.calculate_virtual_branches(project_id, worktree_changes)?;
         }
 
         Ok(())
     }
 
     /// Try to emit uncommited files. Swollow errors if they arrise.
-    fn emit_uncommited_files(&self, project: &Project) {
-        let Ok(files) = VirtualBranchActions.get_uncommited_files(project) else {
-            return;
-        };
+    fn emit_uncommited_files(&self, project: &Project) -> Result<DiffByPathMap> {
+        let files = VirtualBranchActions.get_uncommited_files_reusable(project)?;
 
         let _ = self.emit_app_event(Change::UncommitedFiles {
             project_id: project.id,
-            files,
+            files: files
+                .clone()
+                .into_iter()
+                .map(|(path, file)| {
+                    let binary = file.hunks.iter().any(|h| h.binary);
+                    RemoteBranchFile {
+                        path,
+                        hunks: file.hunks,
+                        binary,
+                    }
+                })
+                .collect(),
         });
+        Ok(files)
     }
 
     fn maybe_create_snapshot(&self, project_id: ProjectId) -> anyhow::Result<()> {