Start to introduce index resolution tests

Cargo.lock

@@ -2111,9 +2111,9 @@ dependencies = [
 
 [[package]]
 name = "git2"
-version = "0.18.3"
+version = "0.19.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "232e6a7bfe35766bf715e55a88b39a700596c0ccfd88cd3680b4cdb40d66ef70"
+checksum = "b903b73e45dc0c6c596f2d37eccece7c1c8bb6e4407b001096387c63d0d93724"
 dependencies = [
  "bitflags 2.6.0",
  "libc",
@@ -4785,9 +4785,9 @@ checksum = "d8adc4bb1803a324070e64a98ae98f38934d91957a99cfb3a43dcbc01bc56439"
 
 [[package]]
 name = "libgit2-sys"
-version = "0.16.2+1.7.2"
+version = "0.17.0+1.8.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "ee4126d8b4ee5c9d9ea891dd875cfdc1e9d0950437179104b183d7d8a74d24e8"
+checksum = "10472326a8a6477c3c20a64547b0059e4b0d086869eee31e6d7da728a8eb7224"
 dependencies = [
  "cc",
  "libc",

Cargo.toml

@@ -37,8 +37,9 @@ resolver = "2"
 [workspace.dependencies]
 bstr = "1.10.0"
 # Add the `tracing` or `tracing-detail` features to see more of gitoxide in the logs. Useful to see which programs it invokes.
-gix = { git = "https://github.com/Byron/gitoxide", rev = "72daa46bad9d397ef2cc48a3cffda23f414ccd8a", default-features = false, features = [] }
+gix = { git = "https://github.com/Byron/gitoxide", rev = "72daa46bad9d397ef2cc48a3cffda23f414ccd8a", default-features = false, features = [
-git2 = { version = "0.18.3", features = [
+] }
+git2 = { version = "0.19.0", features = [
     "vendored-openssl",
     "vendored-libgit2",
 ] }
@@ -93,4 +94,4 @@ debug = true      # Enable debug symbols, for profiling
 [profile.bench]
 codegen-units = 256
 lto = false
-opt-level = 3
+opt-level = 3

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

@@ -478,44 +478,12 @@ fn compute_resolutions(
 
 #[cfg(test)]
 mod test {
-    use std::fs;
-
     use gitbutler_branch::BranchOwnershipClaims;
-    use tempfile::tempdir;
+    use gitbutler_testsupport::testing_repository::TestingRepository;
     use uuid::Uuid;
 
     use super::*;
 
-    fn commit_file<'a>(
-        repository: &'a git2::Repository,
-        parent: Option<&git2::Commit>,
-        files: &[(&str, &str)],
-    ) -> git2::Commit<'a> {
-        for (file_name, contents) in files {
-            fs::write(repository.path().join("..").join(file_name), contents).unwrap();
-        }
-        let mut index = repository.index().unwrap();
-        // Make sure we're not having weird cached state
-        index.read(true).unwrap();
-        index
-            .add_all(["*"], git2::IndexAddOption::DEFAULT, None)
-            .unwrap();
-
-        let signature = git2::Signature::now("Caleb", "caleb@gitbutler.com").unwrap();
-        let commit = repository
-            .commit(
-                None,
-                &signature,
-                &signature,
-                "Committee",
-                &repository.find_tree(index.write_tree().unwrap()).unwrap(),
-                parent.map(|c| vec![c]).unwrap_or_default().as_slice(),
-            )
-            .unwrap();
-
-        repository.find_commit(commit).unwrap()
-    }
-
     fn make_branch(head: git2::Oid, tree: git2::Oid) -> Branch {
         Branch {
             id: Uuid::new_v4().into(),
@@ -540,16 +508,15 @@ mod test {
 
     #[test]
     fn test_up_to_date_if_head_commits_equivalent() {
-        let tempdir = tempdir().unwrap();
+        let test_repository = TestingRepository::open();
-        let repository = git2::Repository::init(tempdir.path()).unwrap();
+        let initial_commit = test_repository.commit_tree(None, &[("foo.txt", "bar")]);
-        let initial_commit = commit_file(&repository, None, &[("foo.txt", "bar")]);
+        let head_commit = test_repository.commit_tree(Some(&initial_commit), &[("foo.txt", "baz")]);
-        let head_commit = commit_file(&repository, Some(&initial_commit), &[("foo.txt", "baz")]);
 
         let context = UpstreamIntegrationContext {
             _permission: None,
             old_target: head_commit.clone(),
             new_target: head_commit,
-            repository: &repository,
+            repository: &test_repository.repository,
             virtual_branches_in_workspace: vec![],
             target_branch_name: "main".to_string(),
         };
@@ -562,17 +529,16 @@ mod test {
 
     #[test]
     fn test_updates_required_if_new_head_ahead() {
-        let tempdir = tempdir().unwrap();
+        let test_repository = TestingRepository::open();
-        let repository = git2::Repository::init(tempdir.path()).unwrap();
+        let initial_commit = test_repository.commit_tree(None, &[("foo.txt", "bar")]);
-        let initial_commit = commit_file(&repository, None, &[("foo.txt", "bar")]);
+        let old_target = test_repository.commit_tree(Some(&initial_commit), &[("foo.txt", "baz")]);
-        let old_target = commit_file(&repository, Some(&initial_commit), &[("foo.txt", "baz")]);
+        let new_target = test_repository.commit_tree(Some(&old_target), &[("foo.txt", "qux")]);
-        let new_target = commit_file(&repository, Some(&old_target), &[("foo.txt", "qux")]);
 
         let context = UpstreamIntegrationContext {
             _permission: None,
             old_target,
             new_target,
-            repository: &repository,
+            repository: &test_repository.repository,
             virtual_branches_in_workspace: vec![],
             target_branch_name: "main".to_string(),
         };
@@ -585,11 +551,10 @@ mod test {
 
     #[test]
     fn test_empty_branch() {
-        let tempdir = tempdir().unwrap();
+        let test_repository = TestingRepository::open();
-        let repository = git2::Repository::init(tempdir.path()).unwrap();
+        let initial_commit = test_repository.commit_tree(None, &[("foo.txt", "bar")]);
-        let initial_commit = commit_file(&repository, None, &[("foo.txt", "bar")]);
+        let old_target = test_repository.commit_tree(Some(&initial_commit), &[("foo.txt", "baz")]);
-        let old_target = commit_file(&repository, Some(&initial_commit), &[("foo.txt", "baz")]);
+        let new_target = test_repository.commit_tree(Some(&old_target), &[("foo.txt", "qux")]);
-        let new_target = commit_file(&repository, Some(&old_target), &[("foo.txt", "qux")]);
 
         let branch = make_branch(old_target.id(), old_target.tree_id());
 
@@ -597,7 +562,7 @@ mod test {
             _permission: None,
             old_target,
             new_target,
-            repository: &repository,
+            repository: &test_repository.repository,
             virtual_branches_in_workspace: vec![branch.clone()],
             target_branch_name: "main".to_string(),
         };
@@ -610,14 +575,11 @@ mod test {
 
     #[test]
     fn test_conflicted_head_branch() {
-        let tempdir = tempdir().unwrap();
+        let test_repository = TestingRepository::open();
-        let repository = git2::Repository::init(tempdir.path()).unwrap();
+        let initial_commit = test_repository.commit_tree(None, &[("foo.txt", "bar")]);
-        let initial_commit = commit_file(&repository, None, &[("foo.txt", "bar")]);
+        let old_target = test_repository.commit_tree(Some(&initial_commit), &[("foo.txt", "baz")]);
-        // Create refs/heads/master
+        let branch_head = test_repository.commit_tree(Some(&old_target), &[("foo.txt", "fux")]);
-        repository.branch("master", &initial_commit, false).unwrap();
+        let new_target = test_repository.commit_tree(Some(&old_target), &[("foo.txt", "qux")]);
-        let old_target = commit_file(&repository, Some(&initial_commit), &[("foo.txt", "baz")]);
-        let branch_head = commit_file(&repository, Some(&old_target), &[("foo.txt", "fux")]);
-        let new_target = commit_file(&repository, Some(&old_target), &[("foo.txt", "qux")]);
 
         let branch = make_branch(branch_head.id(), branch_head.tree_id());
 
@@ -625,7 +587,7 @@ mod test {
             _permission: None,
             old_target,
             new_target: new_target.clone(),
-            repository: &repository,
+            repository: &test_repository.repository,
             virtual_branches_in_workspace: vec![branch.clone()],
             target_branch_name: "main".to_string(),
         };
@@ -655,11 +617,12 @@ mod test {
             panic!("Should be variant UpdatedObjects")
         };
 
-        let head_commit = repository.find_commit(head).unwrap();
+        let head_commit = test_repository.repository.find_commit(head).unwrap();
         assert_eq!(head_commit.parent(0).unwrap().id(), new_target.id());
         assert!(head_commit.is_conflicted());
 
-        let head_tree = repository
+        let head_tree = test_repository
+            .repository
             .find_real_tree(&head_commit, Default::default())
             .unwrap();
         assert_eq!(head_tree.id(), tree)
@@ -667,12 +630,11 @@ mod test {
 
     #[test]
     fn test_conflicted_tree_branch() {
-        let tempdir = tempdir().unwrap();
+        let test_repository = TestingRepository::open();
-        let repository = git2::Repository::init(tempdir.path()).unwrap();
+        let initial_commit = test_repository.commit_tree(None, &[("foo.txt", "bar")]);
-        let initial_commit = commit_file(&repository, None, &[("foo.txt", "bar")]);
+        let old_target = test_repository.commit_tree(Some(&initial_commit), &[("foo.txt", "baz")]);
-        let old_target = commit_file(&repository, Some(&initial_commit), &[("foo.txt", "baz")]);
+        let branch_head = test_repository.commit_tree(Some(&old_target), &[("foo.txt", "fux")]);
-        let branch_head = commit_file(&repository, Some(&old_target), &[("foo.txt", "fux")]);
+        let new_target = test_repository.commit_tree(Some(&old_target), &[("foo.txt", "qux")]);
-        let new_target = commit_file(&repository, Some(&old_target), &[("foo.txt", "qux")]);
 
         let branch = make_branch(old_target.id(), branch_head.tree_id());
 
@@ -680,7 +642,7 @@ mod test {
             _permission: None,
             old_target,
             new_target,
-            repository: &repository,
+            repository: &test_repository.repository,
             virtual_branches_in_workspace: vec![branch.clone()],
             target_branch_name: "main".to_string(),
         };
@@ -698,13 +660,12 @@ mod test {
 
     #[test]
     fn test_conflicted_head_and_tree_branch() {
-        let tempdir = tempdir().unwrap();
+        let test_repository = TestingRepository::open();
-        let repository = git2::Repository::init(tempdir.path()).unwrap();
+        let initial_commit = test_repository.commit_tree(None, &[("foo.txt", "bar")]);
-        let initial_commit = commit_file(&repository, None, &[("foo.txt", "bar")]);
+        let old_target = test_repository.commit_tree(Some(&initial_commit), &[("foo.txt", "baz")]);
-        let old_target = commit_file(&repository, Some(&initial_commit), &[("foo.txt", "baz")]);
+        let branch_head = test_repository.commit_tree(Some(&old_target), &[("foo.txt", "fux")]);
-        let branch_head = commit_file(&repository, Some(&old_target), &[("foo.txt", "fux")]);
+        let branch_tree = test_repository.commit_tree(Some(&old_target), &[("foo.txt", "bax")]);
-        let branch_tree = commit_file(&repository, Some(&old_target), &[("foo.txt", "bax")]);
+        let new_target = test_repository.commit_tree(Some(&old_target), &[("foo.txt", "qux")]);
-        let new_target = commit_file(&repository, Some(&old_target), &[("foo.txt", "qux")]);
 
         let branch = make_branch(branch_head.id(), branch_tree.tree_id());
 
@@ -712,7 +673,7 @@ mod test {
             _permission: None,
             old_target,
             new_target,
-            repository: &repository,
+            repository: &test_repository.repository,
             virtual_branches_in_workspace: vec![branch.clone()],
             target_branch_name: "main".to_string(),
         };
@@ -730,11 +691,10 @@ mod test {
 
     #[test]
     fn test_integrated() {
-        let tempdir = tempdir().unwrap();
+        let test_repository = TestingRepository::open();
-        let repository = git2::Repository::init(tempdir.path()).unwrap();
+        let initial_commit = test_repository.commit_tree(None, &[("foo.txt", "bar")]);
-        let initial_commit = commit_file(&repository, None, &[("foo.txt", "bar")]);
+        let old_target = test_repository.commit_tree(Some(&initial_commit), &[("foo.txt", "baz")]);
-        let old_target = commit_file(&repository, Some(&initial_commit), &[("foo.txt", "baz")]);
+        let new_target = test_repository.commit_tree(Some(&old_target), &[("foo.txt", "qux")]);
-        let new_target = commit_file(&repository, Some(&old_target), &[("foo.txt", "qux")]);
 
         let branch = make_branch(new_target.id(), new_target.tree_id());
 
@@ -742,7 +702,7 @@ mod test {
             _permission: None,
             old_target,
             new_target,
-            repository: &repository,
+            repository: &test_repository.repository,
             virtual_branches_in_workspace: vec![branch.clone()],
             target_branch_name: "main".to_string(),
         };
@@ -755,25 +715,17 @@ mod test {
 
     #[test]
     fn test_integrated_commit_with_uncommited_changes() {
-        let tempdir = tempdir().unwrap();
+        let test_repository = TestingRepository::open();
-        let repository = git2::Repository::init(tempdir.path()).unwrap();
         let initial_commit =
-            commit_file(&repository, None, &[("foo.txt", "bar"), ("bar.txt", "bar")]);
+            test_repository.commit_tree(None, &[("foo.txt", "bar"), ("bar.txt", "bar")]);
-        let old_target = commit_file(
+        let old_target = test_repository.commit_tree(
-            &repository,
             Some(&initial_commit),
             &[("foo.txt", "baz"), ("bar.txt", "bar")],
         );
-        let new_target = commit_file(
+        let new_target = test_repository
-            &repository,
+            .commit_tree(Some(&old_target), &[("foo.txt", "qux"), ("bar.txt", "bar")]);
-            Some(&old_target),
+        let tree = test_repository
-            &[("foo.txt", "qux"), ("bar.txt", "bar")],
+            .commit_tree(Some(&old_target), &[("foo.txt", "baz"), ("bar.txt", "qux")]);
-        );
-        let tree = commit_file(
-            &repository,
-            Some(&old_target),
-            &[("foo.txt", "baz"), ("bar.txt", "qux")],
-        );
 
         let branch = make_branch(new_target.id(), tree.tree_id());
 
@@ -781,7 +733,7 @@ mod test {
             _permission: None,
             old_target,
             new_target,
-            repository: &repository,
+            repository: &test_repository.repository,
             virtual_branches_in_workspace: vec![branch.clone()],
             target_branch_name: "main".to_string(),
         };
@@ -794,31 +746,23 @@ mod test {
 
     #[test]
     fn test_safly_updatable() {
-        let tempdir = tempdir().unwrap();
+        let test_repository = TestingRepository::open();
-        let repository = git2::Repository::init(tempdir.path()).unwrap();
+        let initial_commit =
-        let initial_commit = commit_file(
+            test_repository.commit_tree(None, &[("files-one.txt", "foo"), ("file-two.txt", "foo")]);
-            &repository,
+        let old_target = test_repository.commit_tree(
-            None,
-            &[("files-one.txt", "foo"), ("file-two.txt", "foo")],
-        );
-        let old_target = commit_file(
-            &repository,
             Some(&initial_commit),
             &[("file-one.txt", "bar"), ("file-two.txt", "foo")],
         );
-        let new_target = commit_file(
+        let new_target = test_repository.commit_tree(
-            &repository,
             Some(&old_target),
             &[("file-one.txt", "baz"), ("file-two.txt", "foo")],
         );
 
-        let branch_head = commit_file(
+        let branch_head = test_repository.commit_tree(
-            &repository,
             Some(&old_target),
             &[("file-one.txt", "bar"), ("file-two.txt", "bar")],
         );
-        let branch_tree = commit_file(
+        let branch_tree = test_repository.commit_tree(
-            &repository,
             Some(&branch_head),
             &[("file-one.txt", "bar"), ("file-two.txt", "baz")],
         );
@@ -829,7 +773,7 @@ mod test {
             _permission: None,
             old_target,
             new_target,
-            repository: &repository,
+            repository: &test_repository.repository,
             virtual_branches_in_workspace: vec![branch.clone()],
             target_branch_name: "main".to_string(),
         };

crates/gitbutler-repo/src/rebase.rs

@@ -343,7 +343,7 @@ pub fn gitbutler_merge_commits<'repository>(
 /// in the commit that is getting cherry picked in favor of what came before it
 fn resolve_index(
     repository: &git2::Repository,
-    cherrypick_index: &mut git2::Index,
+    index: &mut git2::Index,
 ) -> Result<Vec<PathBuf>, anyhow::Error> {
     fn bytes_to_path(path: &[u8]) -> Result<PathBuf> {
         let path = std::str::from_utf8(path)?;
@@ -354,9 +354,9 @@ fn resolve_index(
 
     // Set the index on an in-memory repository
     let in_memory_repository = repository.in_memory_repo()?;
-    in_memory_repository.set_index(cherrypick_index)?;
+    in_memory_repository.set_index(index)?;
 
-    let index_conflicts = cherrypick_index.conflicts()?.flatten().collect::<Vec<_>>();
+    let index_conflicts = index.conflicts()?.flatten().collect::<Vec<_>>();
 
     for mut conflict in index_conflicts {
         // There may be a case when there is an ancestor in the index without
@@ -364,19 +364,20 @@ fn resolve_index(
         // getting renamed and modified in the two commits.
         if let Some(ancestor) = &conflict.ancestor {
             let path = bytes_to_path(&ancestor.path)?;
-            cherrypick_index.remove_path(&path)?;
+            index.remove_path(&path)?;
         }
 
         if let (Some(their), None) = (&conflict.their, &conflict.our) {
             // Their (the commit we're rebasing)'s change gets dropped
             let their_path = bytes_to_path(&their.path)?;
-            cherrypick_index.remove_path(&their_path)?;
+            index.remove_path(&their_path)?;
 
             conflicted_files.push(their_path);
         } else if let (None, Some(our)) = (&conflict.their, &mut conflict.our) {
             // Our (the commit we're rebasing onto)'s gets kept
             let blob = repository.find_blob(our.id)?;
-            cherrypick_index.add_frombuffer(our, blob.content())?;
+            our.flags = 0; // For some unknown reason we need to set flags to 0
+            index.add_frombuffer(our, blob.content())?;
 
             let our_path = bytes_to_path(&our.path)?;
             conflicted_files.push(our_path);
@@ -386,8 +387,9 @@ fn resolve_index(
             let their_path = bytes_to_path(&their.path)?;
             let blob = repository.find_blob(our.id)?;
 
-            cherrypick_index.remove_path(&their_path)?;
+            index.remove_path(&their_path)?;
-            cherrypick_index.add_frombuffer(our, blob.content())?;
+            our.flags = 0; // For some unknown reason we need to set flags to 0
+            index.add_frombuffer(our, blob.content())?;
 
             let our_path = bytes_to_path(&our.path)?;
             conflicted_files.push(our_path);
@@ -396,3 +398,51 @@ fn resolve_index(
 
     Ok(conflicted_files)
 }
+
+#[cfg(test)]
+mod test {
+    #[cfg(test)]
+    mod resolve_index {
+        use gitbutler_testsupport::testing_repository::TestingRepository;
+
+        use crate::rebase::resolve_index;
+
+        #[test]
+        fn test_same_file_twice() {
+            let test_repository = TestingRepository::open();
+
+            // Make some commits
+            let a = test_repository.commit_tree(None, &[("foo.txt", "a")]);
+            let b = test_repository.commit_tree(None, &[("foo.txt", "b")]);
+            let c = test_repository.commit_tree(None, &[("foo.txt", "c")]);
+            test_repository.commit_tree(None, &[("foo.txt", "asdfasdf")]);
+
+            // Merge the index
+            let mut index: git2::Index = test_repository
+                .repository
+                .merge_trees(
+                    &a.tree().unwrap(), // Base
+                    &b.tree().unwrap(), // Ours
+                    &c.tree().unwrap(), // Theirs
+                    None,
+                )
+                .unwrap();
+
+            assert!(index.has_conflicts());
+
+            // Call our index resolution function
+            resolve_index(&test_repository.repository, &mut index).unwrap();
+
+            // Ensure there are no conflicts
+            assert!(!index.has_conflicts());
+
+            let tree = index.write_tree_to(&test_repository.repository).unwrap();
+            let tree: git2::Tree = test_repository.repository.find_tree(tree).unwrap();
+
+            let blob = tree.get_name("foo.txt").unwrap().id(); // We fail here to get the entry because the tree is empty
+            let blob: git2::Blob = test_repository.repository.find_blob(blob).unwrap();
+
+            assert_eq!(blob.content(), b"b") // expect b"b", using x as a test inverse
+        }
+    }
+}

crates/gitbutler-testsupport/src/lib.rs

@@ -7,6 +7,8 @@ pub use test_project::TestProject;
 mod suite;
 pub use suite::*;
 
+pub mod testing_repository;
+
 pub mod paths {
     use tempfile::TempDir;
 

crates/gitbutler-testsupport/src/testing_repository.rs

@@ -0,0 +1,69 @@
+use std::fs;
+
+use tempfile::{tempdir, TempDir};
+
+pub struct TestingRepository {
+    pub repository: git2::Repository,
+    pub tempdir: TempDir,
+}
+
+impl TestingRepository {
+    pub fn open() -> Self {
+        let tempdir = tempdir().unwrap();
+        let repository = git2::Repository::init(tempdir.path()).unwrap();
+
+        Self {
+            tempdir,
+            repository,
+        }
+    }
+
+    pub fn commit_tree<'a>(
+        &'a self,
+        parent: Option<&git2::Commit<'a>>,
+        files: &[(&str, &str)],
+    ) -> git2::Commit<'a> {
+        // Remove everything other than the .git folder
+        for entry in fs::read_dir(self.tempdir.path()).unwrap() {
+            let entry = entry.unwrap();
+            if entry.file_name() != ".git" {
+                let path = entry.path();
+                if path.is_dir() {
+                    fs::remove_dir_all(path).unwrap();
+                } else {
+                    fs::remove_file(path).unwrap();
+                }
+            }
+        }
+        // Write any files
+        for (file_name, contents) in files {
+            fs::write(self.tempdir.path().join(file_name), contents).unwrap();
+        }
+
+        // Update the index
+        let mut index = self.repository.index().unwrap();
+        // Make sure we're not having weird cached state
+        index.read(true).unwrap();
+        index
+            .add_all(["*"], git2::IndexAddOption::DEFAULT, None)
+            .unwrap();
+
+        let signature = git2::Signature::now("Caleb", "caleb@gitbutler.com").unwrap();
+        let commit = self
+            .repository
+            .commit(
+                None,
+                &signature,
+                &signature,
+                "Committee",
+                &self
+                    .repository
+                    .find_tree(index.write_tree().unwrap())
+                    .unwrap(),
+                parent.map(|c| vec![c]).unwrap_or_default().as_slice(),
+            )
+            .unwrap();
+
+        self.repository.find_commit(commit).unwrap()
+    }
+}