Merge pull request #14 from mikerite/walker_zip_pr4

Reimplement with zip and two DirWalkers

Author: epage

Cargo.toml

@@ -10,4 +10,4 @@ readme = "README.md"
 tags = ["directory", "diff", "fs"]
 
 [dependencies]
-walkdir = "1.0.7"
+walkdir = "2.0.1"

src/lib.rs

@@ -7,7 +7,7 @@
 //!
 //! ```no_run
 //! extern crate dir_diff;
-//! 
+//!
 //! assert!(dir_diff::is_different("dir/a", "dir/b").unwrap());
 //! ```
 
@@ -16,8 +16,9 @@ extern crate walkdir;
 use std::fs::File;
 use std::io::prelude::*;
 use std::path::Path;
+use std::cmp::Ordering;
 
-use walkdir::WalkDir;
+use walkdir::{DirEntry, WalkDir};
 
 /// The various errors that can happen when diffing two directories
 #[derive(Debug)]
@@ -33,68 +34,37 @@ pub enum Error {
 ///
 /// ```no_run
 /// extern crate dir_diff;
-/// 
+///
 /// assert!(dir_diff::is_different("dir/a", "dir/b").unwrap());
 /// ```
 pub fn is_different<A: AsRef<Path>, B: AsRef<Path>>(a_base: A, b_base: B) -> Result<bool, Error> {
-    let a_base = a_base.as_ref();
-    let b_base = b_base.as_ref();
+    let mut a_walker = walk_dir(a_base);
+    let mut b_walker = walk_dir(b_base);
 
-    // We start by checking the count in the top-level directories. There's two
-    // reasons for this: first, if a is empty, but b is not, our for loop will not
-    // catch it; it will execute zero times and return true, and that's wrong!
-    // Second, if they're differnet, there's no reason to bother comparing the
-    // files; we already know they're different.
+    for (a, b) in (&mut a_walker).zip(&mut b_walker) {
+        let a = a?;
+        let b = b?;
 
-    let a_count = std::fs::read_dir(&a_base)?.count();
-    let b_count = std::fs::read_dir(&b_base)?.count();
-
-    if a_count != b_count {
-        return Ok(true);
+        if a.depth() != b.depth() || a.file_type() != b.file_type()
+            || a.file_name() != b.file_name()
+            || (a.file_type().is_file() && read_to_vec(a.path())? != read_to_vec(b.path())?)
+        {
+            return Ok(true);
+        }
     }
 
-    Ok(compare(a_base, b_base)? || compare(b_base, a_base)?)
+    Ok(!a_walker.next().is_none() || !b_walker.next().is_none())
 }
 
-fn compare(a_base: &Path, b_base: &Path) -> Result<bool, Error> {
-    // next, we walk all of the entries in a and compare them to b
-    for entry in WalkDir::new(a_base) {
-        let entry = entry?;
-        let a = entry.path();
-
-        // calculate just the part of the path relative to a
-        let no_prefix = a.strip_prefix(a_base)?;
-
-        // and then join that with b to get the path in b
-        let b = b_base.join(no_prefix);
-
-        if a.is_dir() {
-            if b.is_dir() {
-                // can't compare the contents of directories, so just continue
-                continue;
-            } else {
-                // if one is a file and one is a directory, we have a difference!
-                return Ok(true)
-            }
-        }
-
-        // file a is guaranteed to exist...
-        let a_text = read_to_vec(a)?;
-
-        // but file b is not. If we have any kind of error when loading
-        // it up, that's a positive result, not an actual error.
-        let b_text = match read_to_vec(b) {
-            Ok(contents) => contents,
-            Err(_) => return Ok(true),
-        };
-
-        if a_text != b_text {
-            return Ok(true);
-        }
-    }
+fn walk_dir<P: AsRef<Path>>(path: P) -> std::iter::Skip<walkdir::IntoIter> {
+    WalkDir::new(path)
+        .sort_by(compare_by_file_name)
+        .into_iter()
+        .skip(1)
+}
 
-    // if we made it here, we're good!
-    Ok(false)
+fn compare_by_file_name(a: &DirEntry, b: &DirEntry) -> Ordering {
+    a.file_name().cmp(b.file_name())
 }
 
 fn read_to_vec<P: AsRef<Path>>(file: P) -> Result<Vec<u8>, std::io::Error> {
@@ -122,4 +92,4 @@ impl From<walkdir::Error> for Error {
     fn from(e: walkdir::Error) -> Error {
         Error::WalkDir(e)
     }
-}
+}

tests/dirs_differ/dir1/dirA/test.txt

@@ -0,0 +1 @@
+test

tests/dirs_differ/dir2/dirB/test.txt

@@ -0,0 +1 @@
+test

tests/filedepth/asc/dir1/a/b.txt

@@ -0,0 +1 @@
+test

tests/filedepth/asc/dir2/b.txt

@@ -0,0 +1 @@
+test

tests/filedepth/desc/dir1/a.txt

@@ -0,0 +1 @@
+test

tests/filedepth/desc/dir2/b/a.txt

@@ -0,0 +1 @@
+test

tests/smoke.rs

@@ -1,5 +1,9 @@
 extern crate dir_diff;
 
+use std::path::Path;
+use std::fs::create_dir;
+use std::io::ErrorKind;
+
 #[test]
 fn easy_good() {
     assert!(!dir_diff::is_different("tests/easy/good/dir1", "tests/easy/good/dir2").unwrap());
@@ -33,4 +37,29 @@ fn oneempty() {
 #[test]
 fn reflexive() {
     assert!(dir_diff::is_different("tests/reflexive/dir1", "tests/reflexive/dir2").unwrap());
-}
+}
+
+#[test]
+fn dirs_differ() {
+    assert!(dir_diff::is_different("tests/dirs_differ/dir1", "tests/dirs_differ/dir2").unwrap());
+}
+
+fn ensure_dir<P: AsRef<Path>>(path: P) -> Result<(), std::io::Error> {
+    match create_dir(path) {
+        Err(ref err) if err.kind() == ErrorKind::AlreadyExists => Ok(()),
+        other => other,
+    }
+}
+
+#[test]
+fn filedepth() {
+    ensure_dir("tests/filedepth/asc/dir2/a").unwrap();
+    ensure_dir("tests/filedepth/desc/dir1/b").unwrap();
+
+    assert!(
+        dir_diff::is_different("tests/filedepth/asc/dir1", "tests/filedepth/asc/dir2").unwrap()
+    );
+    assert!(
+        dir_diff::is_different("tests/filedepth/desc/dir1", "tests/filedepth/desc/dir2").unwrap()
+    );
+}