From eacf7aeb42d7ba54c305664773e77eb592b51b99 Mon Sep 17 00:00:00 2001 From: Bernardo Date: Sat, 19 Jan 2019 22:28:51 +0100 Subject: ignore check event dir for ignore, cleanup tests --- crates/ra_vfs/src/watcher.rs | 61 +++++++++++++++----- crates/ra_vfs/tests/vfs.rs | 131 +++++++++++++++++++++++++++---------------- 2 files changed, 129 insertions(+), 63 deletions(-) diff --git a/crates/ra_vfs/src/watcher.rs b/crates/ra_vfs/src/watcher.rs index 9d552f886..f0ef9bc0e 100644 --- a/crates/ra_vfs/src/watcher.rs +++ b/crates/ra_vfs/src/watcher.rs @@ -1,7 +1,7 @@ use crate::io; use crossbeam_channel::Sender; use drop_bomb::DropBomb; -use ignore; +use ignore::{gitignore::Gitignore, Walk}; use notify::{DebouncedEvent, RecommendedWatcher, RecursiveMode, Watcher as NotifyWatcher}; use parking_lot::Mutex; use std::{ @@ -40,8 +40,11 @@ fn handle_change_event( sender.send(io::Task::HandleChange(WatcherChange::Rescan))?; } DebouncedEvent::Create(path) => { - if path.is_dir() { - watch_recursive(watcher, &path); + // we have to check if `path` is ignored because Walk iterator doesn't check it + // also childs are only ignored if they match a pattern + // (see `matched` vs `matched_path_or_any_parents` in `Gitignore`) + if path.is_dir() && !should_ignore_dir(&path) { + watch_recursive(watcher, &path, Some(sender)); } sender.send(io::Task::HandleChange(WatcherChange::Create(path)))?; } @@ -63,15 +66,18 @@ fn handle_change_event( Ok(()) } -fn watch_one(watcher: &mut RecommendedWatcher, path: &Path) { - match watcher.watch(path, RecursiveMode::NonRecursive) { - Ok(()) => log::debug!("watching \"{}\"", path.display()), - Err(e) => log::warn!("could not watch \"{}\": {}", path.display(), e), +fn watch_one(watcher: &mut RecommendedWatcher, dir: &Path) { + match watcher.watch(dir, RecursiveMode::NonRecursive) { + Ok(()) => log::debug!("watching \"{}\"", dir.display()), + Err(e) => log::warn!("could not watch \"{}\": {}", dir.display(), e), } } -fn watch_recursive(watcher: &Arc>>, path: &Path) { - log::debug!("watch_recursive \"{}\"", path.display()); +fn watch_recursive( + watcher: &Arc>>, + dir: &Path, + sender: Option<&Sender>, +) { let mut watcher = watcher.lock(); let mut watcher = match *watcher { Some(ref mut watcher) => watcher, @@ -80,20 +86,47 @@ fn watch_recursive(watcher: &Arc>>, path: &Path return; } }; - // TODO it seems path itself isn't checked against ignores - // check if path should be ignored before walking it - for res in ignore::Walk::new(path) { + for res in Walk::new(dir) { match res { Ok(entry) => { if entry.path().is_dir() { watch_one(&mut watcher, entry.path()); } + if let Some(sender) = sender { + // emit as create because we haven't seen it yet + if let Err(e) = sender.send(io::Task::HandleChange(WatcherChange::Create( + entry.path().to_path_buf(), + ))) { + log::warn!("watcher error: {}", e) + } + } } Err(e) => log::warn!("watcher error: {}", e), } } } +fn should_ignore_dir(dir: &Path) -> bool { + let mut parent = dir; + loop { + parent = match parent.parent() { + Some(p) => p, + None => break, + }; + let gitignore = parent.join(".gitignore"); + if gitignore.exists() { + let gitignore = Gitignore::new(gitignore).0; + if gitignore.matched_path_or_any_parents(dir, true).is_ignore() { + log::debug!("ignored {}", dir.display()); + return true; + } + } + } + false +} + +const WATCHER_DELAY: Duration = Duration::from_millis(250); + impl Watcher { pub(crate) fn start( output_sender: Sender, @@ -101,7 +134,7 @@ impl Watcher { let (input_sender, input_receiver) = mpsc::channel(); let watcher = Arc::new(Mutex::new(Some(notify::watcher( input_sender, - Duration::from_millis(250), + WATCHER_DELAY, )?))); let w = watcher.clone(); let thread = thread::spawn(move || { @@ -119,7 +152,7 @@ impl Watcher { } pub fn watch(&mut self, root: impl AsRef) { - watch_recursive(&self.watcher, root.as_ref()); + watch_recursive(&self.watcher, root.as_ref(), None); } pub fn shutdown(mut self) -> thread::Result<()> { diff --git a/crates/ra_vfs/tests/vfs.rs b/crates/ra_vfs/tests/vfs.rs index 8266a0bd5..71b25a5c9 100644 --- a/crates/ra_vfs/tests/vfs.rs +++ b/crates/ra_vfs/tests/vfs.rs @@ -11,9 +11,21 @@ fn process_tasks(vfs: &mut Vfs, num_tasks: u32) { } } +macro_rules! assert_match { + ($x:expr, $pat:pat) => { + assert_match!($x, $pat, assert!(true)) + }; + ($x:expr, $pat:pat, $assert:expr) => { + match $x { + $pat => $assert, + x => assert!(false, "Expected {}, got {:?}", stringify!($pat), x), + }; + }; +} + #[test] fn test_vfs_works() -> std::io::Result<()> { - // Logger::with_str("debug").start().unwrap(); + // Logger::with_str("vfs=debug,ra_vfs=debug").start().unwrap(); let files = [ ("a/foo.rs", "hello"), @@ -21,13 +33,16 @@ fn test_vfs_works() -> std::io::Result<()> { ("a/b/baz.rs", "nested hello"), ]; - let dir = tempdir()?; + let dir = tempdir().unwrap(); for (path, text) in files.iter() { let file_path = dir.path().join(path); - fs::create_dir_all(file_path.parent().unwrap())?; + fs::create_dir_all(file_path.parent().unwrap()).unwrap(); fs::write(file_path, text)? } + let gitignore = dir.path().join("a/.gitignore"); + fs::write(gitignore, "/target").unwrap(); + let a_root = dir.path().join("a"); let b_root = dir.path().join("a/b"); @@ -62,79 +77,97 @@ fn test_vfs_works() -> std::io::Result<()> { } fs::write(&dir.path().join("a/b/baz.rs"), "quux").unwrap(); - // 2 tasks per watcher change, first for HandleChange then for LoadChange + // 2 tasks per change, HandleChange and then LoadChange process_tasks(&mut vfs, 2); - match vfs.commit_changes().as_slice() { - [VfsChange::ChangeFile { text, .. }] => assert_eq!(text.as_str(), "quux"), - xs => panic!("unexpected changes {:?}", xs), - } + assert_match!( + vfs.commit_changes().as_slice(), + [VfsChange::ChangeFile { text, .. }], + assert_eq!(text.as_str(), "quux") + ); vfs.change_file_overlay(&dir.path().join("a/b/baz.rs"), "m".to_string()); - match vfs.commit_changes().as_slice() { - [VfsChange::ChangeFile { text, .. }] => assert_eq!(text.as_str(), "m"), - xs => panic!("unexpected changes {:?}", xs), - } + assert_match!( + vfs.commit_changes().as_slice(), + [VfsChange::ChangeFile { text, .. }], + assert_eq!(text.as_str(), "m") + ); // removing overlay restores data on disk vfs.remove_file_overlay(&dir.path().join("a/b/baz.rs")); - match vfs.commit_changes().as_slice() { - [VfsChange::ChangeFile { text, .. }] => assert_eq!(text.as_str(), "quux"), - xs => panic!("unexpected changes {:?}", xs), - } + assert_match!( + vfs.commit_changes().as_slice(), + [VfsChange::ChangeFile { text, .. }], + assert_eq!(text.as_str(), "quux") + ); vfs.add_file_overlay(&dir.path().join("a/b/spam.rs"), "spam".to_string()); - match vfs.commit_changes().as_slice() { - [VfsChange::AddFile { text, path, .. }] => { + assert_match!( + vfs.commit_changes().as_slice(), + [VfsChange::AddFile { text, path, .. }], + { assert_eq!(text.as_str(), "spam"); assert_eq!(path, "spam.rs"); } - xs => panic!("unexpected changes {:?}", xs), - } + ); vfs.remove_file_overlay(&dir.path().join("a/b/spam.rs")); - match vfs.commit_changes().as_slice() { - [VfsChange::RemoveFile { path, .. }] => assert_eq!(path, "spam.rs"), - xs => panic!("unexpected changes {:?}", xs), - } - - fs::write(&dir.path().join("a/new.rs"), "new hello").unwrap(); - process_tasks(&mut vfs, 2); - match vfs.commit_changes().as_slice() { - [VfsChange::AddFile { text, path, .. }] => { + assert_match!( + vfs.commit_changes().as_slice(), + [VfsChange::RemoveFile { path, .. }], + assert_eq!(path, "spam.rs") + ); + + fs::create_dir_all(dir.path().join("a/c")).unwrap(); + fs::write(dir.path().join("a/c/new.rs"), "new hello").unwrap(); + process_tasks(&mut vfs, 4); + assert_match!( + vfs.commit_changes().as_slice(), + [VfsChange::AddFile { text, path, .. }], + { assert_eq!(text.as_str(), "new hello"); - assert_eq!(path, "new.rs"); + assert_eq!(path, "c/new.rs"); } - xs => panic!("unexpected changes {:?}", xs), - } + ); - fs::rename(&dir.path().join("a/new.rs"), &dir.path().join("a/new1.rs")).unwrap(); + fs::rename( + &dir.path().join("a/c/new.rs"), + &dir.path().join("a/c/new1.rs"), + ) + .unwrap(); process_tasks(&mut vfs, 4); - match vfs.commit_changes().as_slice() { + assert_match!( + vfs.commit_changes().as_slice(), [VfsChange::RemoveFile { path: removed_path, .. }, VfsChange::AddFile { text, path: added_path, .. - }] => { - assert_eq!(removed_path, "new.rs"); - assert_eq!(added_path, "new1.rs"); + }], + { + assert_eq!(removed_path, "c/new.rs"); + assert_eq!(added_path, "c/new1.rs"); assert_eq!(text.as_str(), "new hello"); } - xs => panic!("unexpected changes {:?}", xs), - } + ); - fs::remove_file(&dir.path().join("a/new1.rs")).unwrap(); + fs::remove_file(&dir.path().join("a/c/new1.rs")).unwrap(); process_tasks(&mut vfs, 2); - match vfs.commit_changes().as_slice() { - [VfsChange::RemoveFile { path, .. }] => assert_eq!(path, "new1.rs"), - xs => panic!("unexpected changes {:?}", xs), - } - - match vfs.task_receiver().try_recv() { - Err(crossbeam_channel::TryRecvError::Empty) => (), - res => panic!("unexpected {:?}", res), - } + assert_match!( + vfs.commit_changes().as_slice(), + [VfsChange::RemoveFile { path, .. }], + assert_eq!(path, "c/new1.rs") + ); + + fs::create_dir_all(dir.path().join("a/target")).unwrap(); + // should be ignored + fs::write(&dir.path().join("a/target/new.rs"), "ignore me").unwrap(); + process_tasks(&mut vfs, 1); // 1 task because no LoadChange will happen, just HandleChange for dir creation + + assert_match!( + vfs.task_receiver().try_recv(), + Err(crossbeam_channel::TryRecvError::Empty) + ); vfs.shutdown().unwrap(); Ok(()) -- cgit v1.2.3