diff options
author | Andrew Gallant <jamslam@gmail.com> | 2019-01-27 10:45:09 -0500 |
---|---|---|
committer | Andrew Gallant <jamslam@gmail.com> | 2019-01-27 12:11:52 -0500 |
commit | f158a42a715fb6470aa2924182cbf8e1e0fd9448 (patch) | |
tree | 1a87cc84ad8f9851ecfe2edde77c6dd96eac8e25 | |
parent | 5724391d3956dbc68c005d4e6f8f8e4718a052bd (diff) |
ignore: correctly detect hidden files on Windows
This commit fixes a bug where ripgrep only treated files beginning with
a `.` as hidden. On Windows, we continue this tradition, but
additionally check whether a file has the special Windows "hidden"
attribute set. If so, we treat it as a hidden file.
In order to make this work without an additional stat call, we had to
rearrange some of the plumbing from the directory traverser.
Fixes #1154
-rw-r--r-- | Cargo.lock | 14 | ||||
-rw-r--r-- | ignore/Cargo.toml | 2 | ||||
-rw-r--r-- | ignore/src/dir.rs | 18 | ||||
-rw-r--r-- | ignore/src/pathutil.rs | 48 | ||||
-rw-r--r-- | ignore/src/walk.rs | 42 |
5 files changed, 83 insertions, 41 deletions
@@ -170,7 +170,7 @@ dependencies = [ "regex 1.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "same-file 1.0.4 (registry+https://github.com/rust-lang/crates.io-index)", "termcolor 1.0.4 (registry+https://github.com/rust-lang/crates.io-index)", - "winapi-util 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)", + "winapi-util 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] @@ -244,7 +244,7 @@ dependencies = [ "tempfile 3.0.5 (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.7 (registry+https://github.com/rust-lang/crates.io-index)", - "winapi-util 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)", + "winapi-util 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] @@ -537,7 +537,7 @@ name = "same-file" version = "1.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ - "winapi-util 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)", + "winapi-util 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] @@ -703,7 +703,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ "same-file 1.0.4 (registry+https://github.com/rust-lang/crates.io-index)", "winapi 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)", - "winapi-util 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)", + "winapi-util 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] @@ -722,7 +722,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" [[package]] name = "winapi-util" -version = "0.1.1" +version = "0.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ "winapi 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)", @@ -739,7 +739,7 @@ version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ "winapi 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)", - "winapi-util 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)", + "winapi-util 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", ] [metadata] @@ -820,6 +820,6 @@ dependencies = [ "checksum walkdir 2.2.7 (registry+https://github.com/rust-lang/crates.io-index)" = "9d9d7ed3431229a144296213105a390676cc49c9b6a72bd19f3176c98e129fa1" "checksum winapi 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)" = "92c1eb33641e276cfa214a0522acad57be5c56b10cb348b3c5117db75f3ac4b0" "checksum winapi-i686-pc-windows-gnu 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "ac3b87c63620426dd9b991e5ce0329eff545bccbbb34f3be09ff6fb6ab51b7b6" -"checksum winapi-util 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "afc5508759c5bf4285e61feb862b6083c8480aec864fa17a81fdec6f69b461ab" +"checksum winapi-util 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "7168bab6e1daee33b4557efd0e95d5ca70a03706d39fa5f3fe7a236f584b03c9" "checksum winapi-x86_64-pc-windows-gnu 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" "checksum wincolor 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)" = "561ed901ae465d6185fa7864d63fbd5720d0ef718366c9a4dc83cf6170d7e9ba" diff --git a/ignore/Cargo.toml b/ignore/Cargo.toml index 0d4b4a7e..9d5e13d9 100644 --- a/ignore/Cargo.toml +++ b/ignore/Cargo.toml @@ -29,7 +29,7 @@ thread_local = "0.3.6" walkdir = "2.2.7" [target.'cfg(windows)'.dependencies.winapi-util] -version = "0.1.1" +version = "0.1.2" [dev-dependencies] tempfile = "3.0.5" diff --git a/ignore/src/dir.rs b/ignore/src/dir.rs index 3ba0ede8..be2b8351 100644 --- a/ignore/src/dir.rs +++ b/ignore/src/dir.rs @@ -22,6 +22,7 @@ use gitignore::{self, Gitignore, GitignoreBuilder}; use pathutil::{is_hidden, strip_prefix}; use overrides::{self, Override}; use types::{self, Types}; +use walk::DirEntry; use {Error, Match, PartialErrorBuilder}; /// IgnoreMatch represents information about where a match came from when using @@ -306,11 +307,23 @@ impl Ignore { || has_explicit_ignores } + /// Like `matched`, but works with a directory entry instead. + pub fn matched_dir_entry<'a>( + &'a self, + dent: &DirEntry, + ) -> Match<IgnoreMatch<'a>> { + let m = self.matched(dent.path(), dent.is_dir()); + if m.is_none() && self.0.opts.hidden && is_hidden(dent) { + return Match::Ignore(IgnoreMatch::hidden()); + } + m + } + /// Returns a match indicating whether the given file path should be /// ignored or not. /// /// The match contains information about its origin. - pub fn matched<'a, P: AsRef<Path>>( + fn matched<'a, P: AsRef<Path>>( &'a self, path: P, is_dir: bool, @@ -351,9 +364,6 @@ impl Ignore { whitelisted = mat; } } - if whitelisted.is_none() && self.0.opts.hidden && is_hidden(path) { - return Match::Ignore(IgnoreMatch::hidden()); - } whitelisted } diff --git a/ignore/src/pathutil.rs b/ignore/src/pathutil.rs index bfd43de3..fbbc0f89 100644 --- a/ignore/src/pathutil.rs +++ b/ignore/src/pathutil.rs @@ -1,22 +1,56 @@ use std::ffi::OsStr; use std::path::Path; -/// Returns true if and only if this file path is considered to be hidden. +use walk::DirEntry; + +/// Returns true if and only if this entry is considered to be hidden. +/// +/// This only returns true if the base name of the path starts with a `.`. +/// +/// On Unix, this implements a more optimized check. #[cfg(unix)] -pub fn is_hidden<P: AsRef<Path>>(path: P) -> bool { +pub fn is_hidden(dent: &DirEntry) -> bool { use std::os::unix::ffi::OsStrExt; - if let Some(name) = file_name(path.as_ref()) { + if let Some(name) = file_name(dent.path()) { name.as_bytes().get(0) == Some(&b'.') } else { false } } -/// Returns true if and only if this file path is considered to be hidden. -#[cfg(not(unix))] -pub fn is_hidden<P: AsRef<Path>>(path: P) -> bool { - if let Some(name) = file_name(path.as_ref()) { +/// Returns true if and only if this entry is considered to be hidden. +/// +/// On Windows, this returns true if one of the following is true: +/// +/// * The base name of the path starts with a `.`. +/// * The file attributes have the `HIDDEN` property set. +#[cfg(windows)] +pub fn is_hidden(dent: &DirEntry) -> bool { + use std::os::windows::fs::MetadataExt; + use winapi_util::file; + + // This looks like we're doing an extra stat call, but on Windows, the + // directory traverser reuses the metadata retrieved from each directory + // entry and stores it on the DirEntry itself. So this is "free." + if let Ok(md) = dent.metadata() { + if file::is_hidden(md.file_attributes() as u64) { + return true; + } + } + if let Some(name) = file_name(dent.path()) { + name.to_str().map(|s| s.starts_with(".")).unwrap_or(false) + } else { + false + } +} + +/// Returns true if and only if this entry is considered to be hidden. +/// +/// This only returns true if the base name of the path starts with a `.`. +#[cfg(not(any(unix, windows)))] +pub fn is_hidden(dent: &DirEntry) -> bool { + if let Some(name) = file_name(dent.path()) { name.to_str().map(|s| s.starts_with(".")).unwrap_or(false) } else { false diff --git a/ignore/src/walk.rs b/ignore/src/walk.rs index ae1f58ba..72459220 100644 --- a/ignore/src/walk.rs +++ b/ignore/src/walk.rs @@ -99,7 +99,7 @@ impl DirEntry { } /// Returns true if and only if this entry points to a directory. - fn is_dir(&self) -> bool { + pub(crate) fn is_dir(&self) -> bool { self.dent.is_dir() } @@ -883,16 +883,17 @@ impl Walk { return Ok(true); } } - let is_dir = ent.file_type().map_or(false, |ft| ft.is_dir()); - let max_size = self.max_filesize; - let should_skip_path = skip_path(&self.ig, ent.path(), is_dir); - let should_skip_filesize = if !is_dir && max_size.is_some() { - skip_filesize(max_size.unwrap(), ent.path(), &ent.metadata().ok()) - } else { - false - }; - - Ok(should_skip_path || should_skip_filesize) + if should_skip_entry(&self.ig, ent) { + return Ok(true); + } + if self.max_filesize.is_some() && !ent.is_dir() { + return Ok(skip_filesize( + self.max_filesize.unwrap(), + ent.path(), + &ent.metadata().ok(), + )); + } + Ok(false) } } @@ -1420,13 +1421,11 @@ impl Worker { return WalkState::Continue; } } - let is_dir = dent.is_dir(); - let max_size = self.max_filesize; - let should_skip_path = skip_path(ig, dent.path(), is_dir); + let should_skip_path = should_skip_entry(ig, &dent); let should_skip_filesize = - if !is_dir && max_size.is_some() { + if self.max_filesize.is_some() && !dent.is_dir() { skip_filesize( - max_size.unwrap(), + self.max_filesize.unwrap(), dent.path(), &dent.metadata().ok(), ) @@ -1609,17 +1608,16 @@ fn skip_filesize( } } -fn skip_path( +fn should_skip_entry( ig: &Ignore, - path: &Path, - is_dir: bool, + dent: &DirEntry, ) -> bool { - let m = ig.matched(path, is_dir); + let m = ig.matched_dir_entry(dent); if m.is_ignore() { - debug!("ignoring {}: {:?}", path.display(), m); + debug!("ignoring {}: {:?}", dent.path().display(), m); true } else if m.is_whitelist() { - debug!("whitelisting {}: {:?}", path.display(), m); + debug!("whitelisting {}: {:?}", dent.path().display(), m); false } else { false |