diff options
author | Andrew Gallant <jamslam@gmail.com> | 2017-10-21 22:29:21 -0400 |
---|---|---|
committer | Andrew Gallant <jamslam@gmail.com> | 2017-10-21 22:40:10 -0400 |
commit | 1bf9d2925989061313b4181140dba79186df1cba (patch) | |
tree | 7694eae0590015491533f43e35d91d763cc1b1a5 | |
parent | 2a14bf22491b01cabe882bfaa116a4aea048047f (diff) |
ignore: be fastidious with file handles
This commit fixes the symlink loop checker in the parallel directory
traverser to open fewer handles at the expense of keeping handles held
open longer.
This roughly matches the corresponding change in walkdir:
https://github.com/BurntSushi/walkdir/commit/5bcc5b87ee08a58a7f2a7ef4b1d23392b55155bb
Fixes #633
-rw-r--r-- | ignore/src/dir.rs | 23 | ||||
-rw-r--r-- | ignore/src/walk.rs | 10 |
2 files changed, 27 insertions, 6 deletions
diff --git a/ignore/src/dir.rs b/ignore/src/dir.rs index 95c71848..544440d0 100644 --- a/ignore/src/dir.rs +++ b/ignore/src/dir.rs @@ -21,6 +21,7 @@ use std::sync::{Arc, RwLock}; use gitignore::{self, Gitignore, GitignoreBuilder}; use pathutil::{is_hidden, strip_prefix}; use overrides::{self, Override}; +use same_file::Handle; use types::{self, Types}; use {Error, Match, PartialErrorBuilder}; @@ -95,6 +96,9 @@ struct IgnoreInner { compiled: Arc<RwLock<HashMap<OsString, Ignore>>>, /// The path to the directory that this matcher was built from. dir: PathBuf, + /// An open handle to the directory, for checking symlink loops in the + /// parallel iterator. + handle: Arc<Option<Handle>>, /// An override matcher (default is empty). overrides: Arc<Override>, /// A file type matcher. @@ -127,11 +131,15 @@ struct IgnoreInner { impl Ignore { /// Return the directory path of this matcher. - #[allow(dead_code)] pub fn path(&self) -> &Path { &self.0.dir } + /// Return a handle to the directory of this matcher. + pub fn handle(&self) -> Option<&Handle> { + (*self.0.handle).as_ref() + } + /// Return true if this matcher has no parent. pub fn is_root(&self) -> bool { self.0.parent.is_none() @@ -238,9 +246,17 @@ impl Ignore { errs.maybe_push(err); m }; + let handle = match Handle::from_path(dir) { + Ok(handle) => Some(handle), + Err(err) => { + errs.push(Error::from(err).with_path(dir)); + None + } + }; let ig = IgnoreInner { compiled: self.0.compiled.clone(), dir: dir.to_path_buf(), + handle: Arc::new(handle), overrides: self.0.overrides.clone(), types: self.0.types.clone(), parent: Some(self.clone()), @@ -451,9 +467,14 @@ impl IgnoreBuilder { } gi }; + let handle = match Handle::from_path(&self.dir) { + Ok(handle) => Some(handle), + Err(_) => None, + }; Ignore(Arc::new(IgnoreInner { compiled: Arc::new(RwLock::new(HashMap::new())), dir: self.dir.clone(), + handle: Arc::new(handle), overrides: self.overrides.clone(), types: self.types.clone(), parent: None, diff --git a/ignore/src/walk.rs b/ignore/src/walk.rs index 8a15984d..8f0a555a 100644 --- a/ignore/src/walk.rs +++ b/ignore/src/walk.rs @@ -11,7 +11,7 @@ use std::time::Duration; use std::vec; use crossbeam::sync::MsQueue; -use same_file::is_same_file; +use same_file::Handle; use walkdir::{self, WalkDir}; use dir::{Ignore, IgnoreBuilder}; @@ -1308,11 +1308,11 @@ fn check_symlink_loop( child_path: &Path, child_depth: usize, ) -> Result<(), Error> { + let hchild = Handle::from_path(child_path).map_err(|err| { + Error::from(err).with_path(child_path).with_depth(child_depth) + })?; for ig in ig_parent.parents().take_while(|ig| !ig.is_absolute_parent()) { - let same = try!(is_same_file(ig.path(), child_path).map_err(|err| { - Error::from(err).with_path(child_path).with_depth(child_depth) - })); - if same { + if ig.handle().map_or(true, |parent| parent == &hchild) { return Err(Error::Loop { ancestor: ig.path().to_path_buf(), child: child_path.to_path_buf(), |