Use read-only worktree diff

The worktree diff is essentially a 'git status' with a given tree,
and that can be expressed with a diff of the workdir, the index and a tree.

This comes at the expense of not being able to control which file sizes are diffed.

Author: Byron

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

@@ -759,7 +759,7 @@ fn commit_id_can_be_generated_or_specified() -> Result<()> {
 #[test]
 fn merge_vbranch_upstream_clean_rebase() -> Result<()> {
     let suite = Suite::default();
-    let Case { ctx, project, .. } = &suite.new_case();
+    let Case { ctx, project, .. } = &mut suite.new_case();
 
     // create a commit and set the target
     let file_path = Path::new("test.txt");
@@ -829,8 +829,14 @@ fn merge_vbranch_upstream_clean_rebase() -> Result<()> {
 
     // create the branch
     let (branches, _) = list_virtual_branches(ctx, guard.write_permission())?;
+    assert_eq!(branches.len(), 1);
     let branch1 = &branches[0];
-    assert_eq!(branch1.files.len(), 1);
+    assert_eq!(
+        branch1.files.len(),
+        1 + 1,
+        "'test' (modified compared to index) and 'test2' (untracked).\
+        This is actually correct when looking at the git repository"
+    );
     assert_eq!(branch1.commits.len(), 1);
     // assert_eq!(branch1.upstream.as_ref().unwrap().commits.len(), 1);
 

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

@@ -61,15 +61,20 @@ fn conflicting() {
 
         let (branches, _) = controller.list_virtual_branches(project).unwrap();
         assert_eq!(branches.len(), 1);
-        assert!(branches[0].base_current);
-        assert!(branches[0].active);
+        let branch = &branches[0];
+        assert_eq!(
+            branch.name, "Virtual branch",
+            "the auto-created branch gets the default name"
+        );
+        assert!(branch.base_current);
+        assert!(branch.active);
         assert_eq!(
-            branches[0].files[0].hunks[0].diff,
+            branch.files[0].hunks[0].diff,
             "@@ -1 +1 @@\n-first\n\\ No newline at end of file\n+conflict\n\\ No newline at end of file\n"
         );
 
         let unapplied_branch = controller
-            .convert_to_real_branch(project, branches[0].id)
+            .convert_to_real_branch(project, branch.id)
             .unwrap();
 
         Refname::from_str(&unapplied_branch).unwrap()
@@ -100,7 +105,6 @@ fn conflicting() {
 
         assert_eq!(branches.len(), 1);
         let branch = &branches[0];
-        // assert!(!branch.base_current);
         assert!(branch.conflicted);
         assert_eq!(
             branch.files[0].hunks[0].diff,

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

@@ -50,9 +50,8 @@ impl Test {
     /// Consume this instance and keep the temp directory that held the local repository, returning it.
     /// Best used inside a `dbg!(test.debug_local_repo())`
     #[allow(dead_code)]
-    pub fn debug_local_repo(mut self) -> PathBuf {
-        let repo = std::mem::take(&mut self.repository);
-        repo.debug_local_repo()
+    pub fn debug_local_repo(&mut self) -> Option<PathBuf> {
+        self.repository.debug_local_repo()
     }
 }
 

crates/gitbutler-diff/src/diff.rs

@@ -1,9 +1,4 @@
-use std::{
-    borrow::Cow,
-    collections::HashMap,
-    path::{Path, PathBuf},
-    str,
-};
+use std::{borrow::Cow, collections::HashMap, path::PathBuf, str};
 
 use anyhow::{Context, Result};
 use bstr::{BStr, BString, ByteSlice, ByteVec};
@@ -133,34 +128,6 @@ pub fn workdir(repo: &git2::Repository, commit_oid: &git2::Oid) -> Result<DiffBy
         .context("failed to find commit")?;
     let old_tree = repo.find_real_tree(&commit, Default::default())?;
 
-    let mut workdir_index = repo.index()?;
-
-    let mut skipped_files = HashMap::new();
-    let cb = &mut |path: &Path, _matched_spec: &[u8]| -> i32 {
-        let file_size = std::fs::metadata(path).map(|m| m.len()).unwrap_or(0);
-        if file_size > 50_000_000 {
-            skipped_files.insert(
-                path.to_path_buf(),
-                FileDiff {
-                    old_path: None,
-                    new_path: None,
-                    hunks: Vec::new(),
-                    skipped: true,
-                    binary: true,
-                    old_size_bytes: 0,
-                    new_size_bytes: 0,
-                },
-            );
-            1 //skips the entry
-        } else {
-            0
-        }
-    };
-    workdir_index.add_all(["."], git2::IndexAddOption::DEFAULT, Some(cb))?;
-    let workdir_tree_id = workdir_index.write_tree()?;
-
-    let new_tree = repo.find_tree(workdir_tree_id)?;
-
     let mut diff_opts = git2::DiffOptions::new();
     diff_opts
         .recurse_untracked_dirs(true)
@@ -170,14 +137,27 @@ pub fn workdir(repo: &git2::Repository, commit_oid: &git2::Oid) -> Result<DiffBy
         .ignore_submodules(true)
         .context_lines(3);
 
-    let diff = repo.diff_tree_to_tree(Some(&old_tree), Some(&new_tree), Some(&mut diff_opts))?;
-    let diff_files = hunks_by_filepath(Some(repo), &diff);
-    diff_files.map(|mut df| {
-        for (key, value) in skipped_files {
-            df.insert(key, value);
-        }
-        df
-    })
+    let mut index = repo.index()?;
+    // Just a hack to resolve conflicts, which don't get diffed.
+    // Diffed conflicts are something we need though.
+    // For now, it seems easiest to resolve by adding the path forcefully,
+    // which will create objects for the diffs at least.
+    let paths_to_add: Vec<_> = index
+        .conflicts()?
+        .filter_map(Result::ok)
+        .filter_map(|c| {
+            c.our
+                .or(c.their)
+                .or(c.ancestor)
+                .and_then(|c| c.path.into_string().ok())
+        })
+        .collect();
+    for conflict_path_to_resolve in paths_to_add {
+        index.add_path(conflict_path_to_resolve.as_ref())?;
+    }
+    let diff = repo.diff_tree_to_workdir_with_index(Some(&old_tree), Some(&mut diff_opts))?;
+    // TODO(ST): bring back support for skipped (large) files.
+    hunks_by_filepath(Some(repo), &diff)
 }
 
 pub fn trees(
@@ -194,9 +174,10 @@ pub fn trees(
         .context_lines(3)
         .show_untracked_content(true);
 
+    // This is not a content-based diff, but also considers modification times apparently,
+    // maybe related to racy-git. This is why empty diffs have ot be filtered.
     let diff =
         repository.diff_tree_to_tree(Some(old_tree), Some(new_tree), Some(&mut diff_opts))?;
-
     hunks_by_filepath(None, &diff)
 }
 

crates/gitbutler-testsupport/src/suite.rs

@@ -92,7 +92,7 @@ pub struct Case {
     pub ctx: CommandContext,
     pub credentials: Helper,
     /// The directory containing the `ctx`
-    project_tmp: Option<TempDir>,
+    pub project_tmp: Option<TempDir>,
 }
 
 impl Drop for Case {

crates/gitbutler-testsupport/src/test_project.rs

@@ -19,8 +19,8 @@ pub struct TestProject {
 impl Drop for TestProject {
     fn drop(&mut self) {
         if std::env::var_os(VAR_NO_CLEANUP).is_some() {
-            let _ = self.local_tmp.take().unwrap().into_path();
-            let _ = self.remote_tmp.take().unwrap().into_path();
+            let _ = self.local_tmp.take().map(|tmp| tmp.into_path());
+            let _ = self.remote_tmp.take().map(|tmp| tmp.into_path());
         }
     }
 }
@@ -83,10 +83,11 @@ impl Default for TestProject {
 }
 
 impl TestProject {
-    /// Consume this instance and keep the temp directory that held the local repository, returning it.
+    /// Take the tmp directory holding the local repository and make sure it won't be deleted,
+    /// returning a path to it.
     /// Best used inside a `dbg!(test_project.debug_local_repo())`
-    pub fn debug_local_repo(mut self) -> PathBuf {
-        self.local_tmp.take().unwrap().into_path()
+    pub fn debug_local_repo(&mut self) -> Option<PathBuf> {
+        self.local_tmp.take().map(|tmp| tmp.into_path())
     }
     pub fn path(&self) -> &std::path::Path {
         self.local_repository.workdir().unwrap()