remove git2 verion of the diff algorithm (#16)

Author: Byron

src/index/diff/mod.rs

@@ -1,11 +1,9 @@
-use crate::{Change, CrateVersion, Index};
+use crate::{Change, Index};
 use git_repository as git;
 use git_repository::prelude::{FindExt, ObjectIdExt, TreeIterExt};
 use git_repository::refs::transaction::PreviousValue;
 use std::convert::TryFrom;
 
-static LINE_ADDED_INDICATOR: char = '+';
-
 mod delegate;
 use delegate::Delegate;
 
@@ -90,82 +88,6 @@ impl Index {
         from: impl Into<git::hash::ObjectId>,
         to: impl Into<git::hash::ObjectId>,
     ) -> Result<Vec<Change>, Error> {
-        let repo = git2::Repository::open(self.repo.git_dir())?;
-        let from = git2::Oid::from_bytes(from.into().as_slice())?;
-        let to = git2::Oid::from_bytes(to.into().as_slice())?;
-        fn into_tree<'a>(
-            repo: &'a git2::Repository,
-            obj: &git2::Object<'_>,
-        ) -> Result<git2::Tree<'a>, git2::Error> {
-            repo.find_tree(match obj.kind() {
-                Some(git2::ObjectType::Commit) => obj
-                    .as_commit()
-                    .expect("object of kind commit yields commit")
-                    .tree_id(),
-                _ =>
-                /* let it possibly fail later */
-                {
-                    obj.id()
-                }
-            })
-        }
-        let from = repo.find_object(from, None)?;
-        let to = repo.find_object(to, None)?;
-        let diff = repo.diff_tree_to_tree(
-            Some(&into_tree(&repo, &from)?),
-            Some(&into_tree(&repo, &to)?),
-            None,
-        )?;
-        let mut changes: Vec<Change> = Vec::new();
-        let mut deletes: Vec<String> = Vec::new();
-        diff.foreach(
-            &mut |delta, _| {
-                if delta.status() == git2::Delta::Deleted {
-                    if let Some(path) = delta.new_file().path() {
-                        if let Some(file_name) = path.file_name() {
-                            deletes.push(file_name.to_string_lossy().to_string());
-                        }
-                    }
-                }
-                true
-            },
-            None,
-            None,
-            Some(&mut |delta, _hunk, diffline| {
-                if diffline.origin() != LINE_ADDED_INDICATOR {
-                    return true;
-                }
-                if !matches!(delta.status(), git2::Delta::Added | git2::Delta::Modified) {
-                    return true;
-                }
-
-                if let Ok(crate_version) =
-                    serde_json::from_slice::<CrateVersion>(diffline.content())
-                {
-                    if crate_version.yanked {
-                        changes.push(Change::Yanked(crate_version));
-                    } else {
-                        changes.push(Change::Added(crate_version));
-                    }
-                }
-                true
-            }),
-        )?;
-
-        changes.extend(deletes.iter().map(|krate| Change::Deleted {
-            name: krate.clone(),
-        }));
-        Ok(changes)
-    }
-
-    /// Similar to `changes()`, but requires `from` and `to` objects to be provided. They may point
-    /// to either `Commit`s or `Tree`s.
-    pub fn changes_between_commits2(
-        &mut self,
-        from: impl Into<git::hash::ObjectId>,
-        to: impl Into<git::hash::ObjectId>,
-    ) -> Result<Vec<Change>, Error> {
-        self.repo.object_cache_size_if_unset(4 * 1024 * 1024);
         let into_tree = |id: git::hash::ObjectId| -> Result<git::Tree<'_>, Error> {
             Ok(id
                 .attach(&self.repo)

src/index/init.rs

@@ -86,8 +86,10 @@ impl Index {
             }
         })?;
 
+        let mut repo = git::open(repo.path())?.apply_environment();
+        repo.object_cache_size_if_unset(4 * 1024 * 1024);
         Ok(Index {
-            repo: git::open(repo.path())?,
+            repo,
             branch_name: "master",
             seen_ref_name: LAST_SEEN_REFNAME,
         })

tests/index/changes_between_commits.rs

@@ -19,23 +19,6 @@ fn addition() -> crate::Result {
     Ok(())
 }
 
-#[test]
-fn addition2() -> crate::Result {
-    let changes = changes2(index_ro()?, ":/initial commit")?;
-    assert_eq!(changes.len(), 3228);
-    assert!(matches!(
-        changes
-            .first()
-            .and_then(|c| c.added().map(|v| v.name.as_str())),
-        Some("gi-get-artifact")
-    ));
-    assert!(matches!(
-        changes.last().expect("present"),
-        Change::Added(CrateVersion {name, ..}) if name == "gizmo"
-    ));
-    Ok(())
-}
-
 #[test]
 fn deletion() -> crate::Result {
     let changes = changes(index_ro()?, "@~326")?;
@@ -44,14 +27,6 @@ fn deletion() -> crate::Result {
     Ok(())
 }
 
-#[test]
-fn deletion2() -> crate::Result {
-    let changes = changes2(index_ro()?, "@~326")?;
-    assert_eq!(changes.len(), 1);
-    assert_eq!(changes.first().and_then(|c| c.deleted()), Some("girl"));
-    Ok(())
-}
-
 #[test]
 fn new_version() -> crate::Result {
     let changes = changes(index_ro()?, ":/Updating crate `git-repository#0.22.1`")?;
@@ -65,19 +40,6 @@ fn new_version() -> crate::Result {
     Ok(())
 }
 
-#[test]
-fn new_version2() -> crate::Result {
-    let changes = changes2(index_ro()?, ":/Updating crate `git-repository#0.22.1`")?;
-    assert_eq!(changes.len(), 1);
-    assert_eq!(
-        changes
-            .first()
-            .and_then(|c| c.added().map(|v| v.name.as_str())),
-        Some("git-repository")
-    );
-    Ok(())
-}
-
 #[test]
 fn yanked() -> crate::Result {
     let changes = changes(index_ro()?, ":/Yanking crate `github_release_rs#0.1.0`")?;
@@ -91,19 +53,6 @@ fn yanked() -> crate::Result {
     Ok(())
 }
 
-#[test]
-fn yanked2() -> crate::Result {
-    let changes = changes2(index_ro()?, ":/Yanking crate `github_release_rs#0.1.0`")?;
-    assert_eq!(changes.len(), 1);
-    assert_eq!(
-        changes
-            .first()
-            .and_then(|c| c.yanked().map(|v| v.name.as_str())),
-        Some("github_release_rs")
-    );
-    Ok(())
-}
-
 #[test]
 fn unyanked_crates_recognized_as_added() -> crate::Result {
     let changes = changes(index_ro()?, ":/Unyanking crate `git2mail#0.3.2`")?;
@@ -117,19 +66,6 @@ fn unyanked_crates_recognized_as_added() -> crate::Result {
     Ok(())
 }
 
-#[test]
-fn unyanked_crates_recognized_as_added2() -> crate::Result {
-    let changes = changes2(index_ro()?, ":/Unyanking crate `git2mail#0.3.2`")?;
-    assert_eq!(changes.len(), 1);
-    assert_eq!(
-        changes
-            .first()
-            .and_then(|c| c.added().map(|v| v.name.as_str())),
-        Some("git2mail")
-    );
-    Ok(())
-}
-
 #[test]
 fn normalization() -> crate::Result {
     let changes = changes(index_ro()?, ":/normalize")?;
@@ -141,17 +77,6 @@ fn normalization() -> crate::Result {
     Ok(())
 }
 
-#[test]
-fn normalization2() -> crate::Result {
-    let changes = changes2(index_ro()?, ":/normalize")?;
-    assert_eq!(
-        changes.len(),
-        2356, // should be 0
-        "normalization changes the representation, but the data itself stays the same, BUT we can't do it yet"
-    );
-    Ok(())
-}
-
 fn changes(mut index: Index, revspec: &str) -> crate::Result<Vec<Change>> {
     let (prev, current) = {
         let repo = index.repository_mut();
@@ -176,28 +101,3 @@ fn changes(mut index: Index, revspec: &str) -> crate::Result<Vec<Change>> {
     };
     Ok(index.changes_between_commits(prev, current)?)
 }
-
-fn changes2(mut index: Index, revspec: &str) -> crate::Result<Vec<Change>> {
-    let (prev, current) = {
-        let repo = index.repository_mut();
-        repo.object_cache_size_if_unset(4 * 1024 * 1024);
-        let commit = repo.rev_parse(revspec)?.single().unwrap();
-        let ancestor_tree = commit
-            .object()?
-            .into_commit()
-            .parent_ids()
-            .next()
-            .and_then(|parent| {
-                parent
-                    .object()
-                    .ok()?
-                    .into_commit()
-                    .tree_id()
-                    .ok()
-                    .map(|id| id.detach())
-            })
-            .unwrap_or_else(|| git::hash::ObjectId::empty_tree(repo.object_hash()));
-        (ancestor_tree, commit.detach())
-    };
-    Ok(index.changes_between_commits2(prev, current)?)
-}