ignore: fix false positive in path_is_symlink

This commit fixes a bug where the first path always reported itself as
as symlink via `path_is_symlink`.

Part of this fix includes updating walkdir to 2.2.1, which also includes
a corresponding bug fix.

Fixes #984

CHANGELOG.md

@@ -66,6 +66,8 @@ Bug fixes:
   Color escape sequences are no longer emitted for empty matches.
 * [BUG #940](https://github.com/BurntSushi/ripgrep/issues/940):
   Context from the `--passthru` flag should not impact process exit status.
+* [BUG #984](https://github.com/BurntSushi/ripgrep/issues/984):
+  Fixes bug in `ignore` crate where first path was always treated as a symlink.
 * [BUG #1013](https://github.com/BurntSushi/ripgrep/issues/1013):
   Add compile time and runtime CPU features to `--version` output.
 

Cargo.lock

@@ -184,7 +184,7 @@ dependencies = [
  "grep-regex 0.1.0",
  "grep-searcher 0.1.0",
  "termcolor 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)",
- "walkdir 2.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "walkdir 2.2.1 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
 [[package]]
@@ -257,7 +257,7 @@ dependencies = [
  "same-file 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "tempdir 0.3.7 (registry+https://github.com/rust-lang/crates.io-index)",
  "thread_local 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)",
- "walkdir 2.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "walkdir 2.2.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "winapi 0.3.5 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
@@ -655,7 +655,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 
 [[package]]
 name = "walkdir"
-version = "2.2.0"
+version = "2.2.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
  "same-file 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -761,7 +761,7 @@ dependencies = [
 "checksum utf8-ranges 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "662fab6525a98beff2921d7f61a39e7d59e0b425ebc7d0d9e66d316e55124122"
 "checksum version_check 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)" = "7716c242968ee87e5542f8021178248f267f295a5c4803beae8b8b7fd9bc6051"
 "checksum void 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)" = "6a02e4885ed3bc0f2de90ea6dd45ebcbb66dacffe03547fadbb0eeae2770887d"
-"checksum walkdir 2.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "f1b768ba943161a9226ccd59b26bcd901e5d60e6061f4fcad3034784e0c7372b"
+"checksum walkdir 2.2.1 (registry+https://github.com/rust-lang/crates.io-index)" = "99363cca565f12a1a4bfb0cd5b966afb2989117ce9f1c18add0ecbb87f46718c"
 "checksum winapi 0.3.5 (registry+https://github.com/rust-lang/crates.io-index)" = "773ef9dcc5f24b7d850d0ff101e542ff24c3b090a9768e03ff889fdef41f00fd"
 "checksum winapi-i686-pc-windows-gnu 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "ac3b87c63620426dd9b991e5ce0329eff545bccbbb34f3be09ff6fb6ab51b7b6"
 "checksum winapi-x86_64-pc-windows-gnu 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f"

grep/Cargo.toml

@@ -22,7 +22,7 @@ grep-searcher = { version = "0.1.0", path = "../grep-searcher" }
 [dev-dependencies]
 atty = "0.2.11"
 termcolor = "1"
-walkdir = "2.2.0"
+walkdir = "2.2.1"
 
 [features]
 avx-accel = ["grep-searcher/avx-accel"]

ignore/Cargo.toml

@@ -26,7 +26,7 @@ memchr = "2"
 regex = "1"
 same-file = "1"
 thread_local = "0.3.2"
-walkdir = "2.2.0"
+walkdir = "2.2.1"
 
 [target.'cfg(windows)'.dependencies.winapi]
 version = "0.3"

ignore/src/walk.rs

@@ -359,21 +359,29 @@ impl DirEntryRaw {
     }
 
     #[cfg(not(unix))]
-    fn from_link(depth: usize, pb: PathBuf) -> Result<DirEntryRaw, Error> {
+    fn from_path(
+        depth: usize,
+        pb: PathBuf,
+        link: bool,
+    ) -> Result<DirEntryRaw, Error> {
         let md = fs::metadata(&pb).map_err(|err| {
             Error::Io(err).with_path(&pb)
         })?;
         Ok(DirEntryRaw {
             path: pb,
             ty: md.file_type(),
-            follow_link: true,
+            follow_link: link,
             depth: depth,
             metadata: md,
         })
     }
 
     #[cfg(unix)]
-    fn from_link(depth: usize, pb: PathBuf) -> Result<DirEntryRaw, Error> {
+    fn from_path(
+        depth: usize,
+        pb: PathBuf,
+        link: bool,
+    ) -> Result<DirEntryRaw, Error> {
         use std::os::unix::fs::MetadataExt;
 
         let md = fs::metadata(&pb).map_err(|err| {
@@ -382,7 +390,7 @@ impl DirEntryRaw {
         Ok(DirEntryRaw {
             path: pb,
             ty: md.file_type(),
-            follow_link: true,
+            follow_link: link,
             depth: depth,
             ino: md.ino(),
         })
@@ -957,7 +965,7 @@ impl WalkParallel {
                 if path == Path::new("-") {
                     DirEntry::new_stdin()
                 } else {
-                    match DirEntryRaw::from_link(0, path) {
+                    match DirEntryRaw::from_path(0, path, false) {
                         Ok(dent) => DirEntry::new_raw(dent, None),
                         Err(err) => {
                             if f(Err(err)).is_quit() {
@@ -1209,7 +1217,7 @@ impl Worker {
         let is_symlink = dent.file_type().map_or(false, |ft| ft.is_symlink());
         if self.follow_links && is_symlink {
             let path = dent.path().to_path_buf();
-            dent = match DirEntryRaw::from_link(depth, path) {
+            dent = match DirEntryRaw::from_path(depth, path, true) {
                 Ok(dent) => DirEntry::new_raw(dent, None),
                 Err(err) => {
                     return (self.f)(Err(err));
@@ -1426,7 +1434,7 @@ mod tests {
 
     use tempdir::TempDir;
 
-    use super::{WalkBuilder, WalkState};
+    use super::{DirEntry, WalkBuilder, WalkState};
 
     fn wfile<P: AsRef<Path>>(path: P, contents: &str) {
         let mut file = File::create(path).unwrap();
@@ -1477,28 +1485,32 @@ mod tests {
         prefix: &Path,
         builder: &WalkBuilder,
     ) -> Vec<String> {
-        let paths = Arc::new(Mutex::new(vec![]));
+        let mut paths = vec![];
-        let prefix = Arc::new(prefix.to_path_buf());
+        for dent in walk_collect_entries_parallel(builder) {
+            let path = dent.path().strip_prefix(prefix).unwrap();
+            if path.as_os_str().is_empty() {
+                continue;
+            }
+            paths.push(normal_path(path.to_str().unwrap()));
+        }
+        paths.sort();
+        paths
+    }
+
+    fn walk_collect_entries_parallel(builder: &WalkBuilder) -> Vec<DirEntry> {
+        let dents = Arc::new(Mutex::new(vec![]));
         builder.build_parallel().run(|| {
-            let paths = paths.clone();
+            let dents = dents.clone();
-            let prefix = prefix.clone();
             Box::new(move |result| {
-                let dent = match result {
+                if let Ok(dent) = result {
-                    Err(_) => return WalkState::Continue,
+                    dents.lock().unwrap().push(dent);
-                    Ok(dent) => dent,
-                };
-                let path = dent.path().strip_prefix(&**prefix).unwrap();
-                if path.as_os_str().is_empty() {
-                    return WalkState::Continue;
                 }
-                let mut paths = paths.lock().unwrap();
-                paths.push(normal_path(path.to_str().unwrap()));
                 WalkState::Continue
             })
         });
-        let mut paths = paths.lock().unwrap();
+
-        paths.sort();
+        let dents = dents.lock().unwrap();
-        paths.to_vec()
+        dents.to_vec()
     }
 
     fn mkpaths(paths: &[&str]) -> Vec<String> {
@@ -1693,6 +1705,27 @@ mod tests {
         ]);
     }
 
+    #[cfg(unix)] // because symlinks on windows are weird
+    #[test]
+    fn first_path_not_symlink() {
+        let td = TempDir::new("walk-test-").unwrap();
+        mkdirp(td.path().join("foo"));
+
+        let dents = WalkBuilder::new(td.path().join("foo"))
+            .build()
+            .into_iter()
+            .collect::<Result<Vec<_>, _>>()
+            .unwrap();
+        assert_eq!(1, dents.len());
+        assert!(!dents[0].path_is_symlink());
+
+        let dents = walk_collect_entries_parallel(
+            &WalkBuilder::new(td.path().join("foo")),
+        );
+        assert_eq!(1, dents.len());
+        assert!(!dents[0].path_is_symlink());
+    }
+
     #[cfg(unix)] // because symlinks on windows are weird
     #[test]
     fn symlink_loop() {