From 46ac9ff5e3cf070584d8167150655d091d47e3c2 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sat, 18 Jul 2020 16:40:10 +0200 Subject: Simplify exclusion logic --- Cargo.lock | 30 ----------- crates/rust-analyzer/Cargo.toml | 1 - crates/vfs-notify/Cargo.toml | 1 - crates/vfs-notify/src/include.rs | 42 ---------------- crates/vfs-notify/src/lib.rs | 104 ++++++++++++++++++--------------------- crates/vfs/src/loader.rs | 82 ++++++++++++++++++++++++++---- xtask/tests/tidy.rs | 1 - 7 files changed, 120 insertions(+), 141 deletions(-) delete mode 100644 crates/vfs-notify/src/include.rs diff --git a/Cargo.lock b/Cargo.lock index d341d1054..c58c3035c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -103,15 +103,6 @@ version = "1.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cf1de2fe8c75bc145a2f577add951f8134889b4795d47466a54a5c846d691693" -[[package]] -name = "bstr" -version = "0.2.13" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "31accafdb70df7871592c058eca3985b71104e15ac32f64706022c58867da931" -dependencies = [ - "memchr", -] - [[package]] name = "byteorder" version = "1.3.4" @@ -403,12 +394,6 @@ dependencies = [ "serde_json", ] -[[package]] -name = "fnv" -version = "1.0.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3f9eec918d3f24069decb9af1554cad7c880e2da24a9afd88aca000531ab82c1" - [[package]] name = "fs_extra" version = "1.1.0" @@ -473,19 +458,6 @@ version = "0.22.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "aaf91faf136cb47367fa430cd46e37a788775e7fa104f8b4bcb3861dc389b724" -[[package]] -name = "globset" -version = "0.4.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7ad1da430bd7281dde2576f44c84cc3f0f7b475e7202cd503042dff01a8c8120" -dependencies = [ - "aho-corasick", - "bstr", - "fnv", - "log", - "regex", -] - [[package]] name = "goblin" version = "0.2.3" @@ -1494,7 +1466,6 @@ dependencies = [ "env_logger", "expect", "flycheck", - "globset", "itertools", "jod-thread", "log", @@ -2003,7 +1974,6 @@ name = "vfs-notify" version = "0.1.0" dependencies = [ "crossbeam-channel", - "globset", "jod-thread", "log", "notify", diff --git a/crates/rust-analyzer/Cargo.toml b/crates/rust-analyzer/Cargo.toml index 3ed2a689e..5eb2d0bb7 100644 --- a/crates/rust-analyzer/Cargo.toml +++ b/crates/rust-analyzer/Cargo.toml @@ -17,7 +17,6 @@ path = "src/bin/main.rs" anyhow = "1.0.26" crossbeam-channel = "0.4.0" env_logger = { version = "0.7.1", default-features = false } -globset = "0.4.4" itertools = "0.9.0" jod-thread = "0.1.0" log = "0.4.8" diff --git a/crates/vfs-notify/Cargo.toml b/crates/vfs-notify/Cargo.toml index 95c56ffa6..fce7bae3a 100644 --- a/crates/vfs-notify/Cargo.toml +++ b/crates/vfs-notify/Cargo.toml @@ -13,7 +13,6 @@ log = "0.4.8" rustc-hash = "1.0" jod-thread = "0.1.0" walkdir = "2.3.1" -globset = "0.4.5" crossbeam-channel = "0.4.0" notify = "5.0.0-pre.3" diff --git a/crates/vfs-notify/src/include.rs b/crates/vfs-notify/src/include.rs deleted file mode 100644 index 59da3d77a..000000000 --- a/crates/vfs-notify/src/include.rs +++ /dev/null @@ -1,42 +0,0 @@ -//! See `Include`. -use std::convert::TryFrom; - -use globset::{Glob, GlobSet, GlobSetBuilder}; -use paths::{RelPath, RelPathBuf}; - -/// `Include` is the opposite of .gitignore. -/// -/// It describes the set of files inside some directory. -/// -/// The current implementation is very limited, it allows including file globs -/// and recursively excluding directories. -#[derive(Debug, Clone)] -pub(crate) struct Include { - include_files: GlobSet, - exclude_dirs: Vec, -} - -impl Include { - pub(crate) fn new(include: Vec) -> Include { - let mut include_files = GlobSetBuilder::new(); - let mut exclude_dirs = Vec::new(); - - for glob in include { - if glob.starts_with("!/") { - if let Ok(path) = RelPathBuf::try_from(&glob["!/".len()..]) { - exclude_dirs.push(path) - } - } else { - include_files.add(Glob::new(&glob).unwrap()); - } - } - let include_files = include_files.build().unwrap(); - Include { include_files, exclude_dirs } - } - pub(crate) fn include_file(&self, path: &RelPath) -> bool { - self.include_files.is_match(path) - } - pub(crate) fn exclude_dir(&self, path: &RelPath) -> bool { - self.exclude_dirs.iter().any(|excluded| path.starts_with(excluded)) - } -} diff --git a/crates/vfs-notify/src/lib.rs b/crates/vfs-notify/src/lib.rs index 6aaa53f63..e1e36612a 100644 --- a/crates/vfs-notify/src/lib.rs +++ b/crates/vfs-notify/src/lib.rs @@ -6,9 +6,7 @@ //! //! Hopefully, one day a reliable file watching/walking crate appears on //! crates.io, and we can reduce this to trivial glue code. -mod include; - -use std::convert::{TryFrom, TryInto}; +use std::convert::TryFrom; use crossbeam_channel::{never, select, unbounded, Receiver, Sender}; use notify::{RecommendedWatcher, RecursiveMode, Watcher}; @@ -16,8 +14,6 @@ use paths::{AbsPath, AbsPathBuf}; use vfs::loader; use walkdir::WalkDir; -use crate::include::Include; - #[derive(Debug)] pub struct NotifyHandle { // Relative order of fields below is significant. @@ -53,7 +49,7 @@ type NotifyEvent = notify::Result; struct NotifyActor { sender: loader::Sender, - config: Vec<(AbsPathBuf, Include, bool)>, + watched_entries: Vec, // Drop order is significant. watcher: Option<(RecommendedWatcher, Receiver)>, } @@ -66,7 +62,7 @@ enum Event { impl NotifyActor { fn new(sender: loader::Sender) -> NotifyActor { - NotifyActor { sender, config: Vec::new(), watcher: None } + NotifyActor { sender, watched_entries: Vec::new(), watcher: None } } fn next_event(&self, receiver: &Receiver) -> Option { let watcher_receiver = self.watcher.as_ref().map(|(_, receiver)| receiver); @@ -93,15 +89,17 @@ impl NotifyActor { let n_total = config.load.len(); self.send(loader::Message::Progress { n_total, n_done: 0 }); - self.config.clear(); + self.watched_entries.clear(); for (i, entry) in config.load.into_iter().enumerate() { let watch = config.watch.contains(&i); + if watch { + self.watched_entries.push(entry.clone()) + } let files = self.load_entry(entry, watch); self.send(loader::Message::Loaded { files }); self.send(loader::Message::Progress { n_total, n_done: i + 1 }); } - self.config.sort_by(|x, y| x.0.cmp(&y.0)); } Message::Invalidate(path) => { let contents = read(path.as_path()); @@ -116,34 +114,27 @@ impl NotifyActor { .into_iter() .map(|path| AbsPathBuf::try_from(path).unwrap()) .filter_map(|path| { - let is_dir = path.is_dir(); - let is_file = path.is_file(); - - let config_idx = - match self.config.binary_search_by(|it| it.0.cmp(&path)) { - Ok(it) => it, - Err(it) => it.saturating_sub(1), - }; - let include = self.config.get(config_idx).and_then(|it| { - let rel_path = path.strip_prefix(&it.0)?; - Some((rel_path, &it.1)) - }); - - if let Some((rel_path, include)) = include { - if is_dir && include.exclude_dir(&rel_path) - || is_file && !include.include_file(&rel_path) - { - return None; - } + if path.is_dir() + && self + .watched_entries + .iter() + .any(|entry| entry.contains_dir(&path)) + { + self.watch(path); + return None; } - if is_dir { - self.watch(path); + if !path.is_file() { return None; } - if !is_file { + if !self + .watched_entries + .iter() + .any(|entry| entry.contains_file(&path)) + { return None; } + let contents = read(&path); Some((path, contents)) }) @@ -170,43 +161,42 @@ impl NotifyActor { (file, contents) }) .collect::>(), - loader::Entry::Directory { path, include } => { - let include = Include::new(include); - self.config.push((path.clone(), include.clone(), watch)); - - let files = WalkDir::new(&path) - .into_iter() - .filter_entry(|entry| { - let abs_path: &AbsPath = entry.path().try_into().unwrap(); - match abs_path.strip_prefix(&path) { - Some(rel_path) => { - !(entry.file_type().is_dir() && include.exclude_dir(rel_path)) - } - None => false, + loader::Entry::Directories(dirs) => { + let mut res = Vec::new(); + + for root in dirs.include.iter() { + let walkdir = WalkDir::new(root).into_iter().filter_entry(|entry| { + if !entry.file_type().is_dir() { + return true; } - }) - .filter_map(|entry| entry.ok()) - .filter_map(|entry| { + let path = AbsPath::assert(entry.path()); + root == path + || dirs.exclude.iter().chain(&dirs.include).all(|it| it != path) + }); + + let files = walkdir.filter_map(|it| it.ok()).filter_map(|entry| { let is_dir = entry.file_type().is_dir(); let is_file = entry.file_type().is_file(); - let abs_path = AbsPathBuf::try_from(entry.into_path()).unwrap(); + let abs_path = AbsPathBuf::assert(entry.into_path()); if is_dir && watch { self.watch(abs_path.clone()); } - let rel_path = abs_path.strip_prefix(&path)?; - if is_file && include.include_file(&rel_path) { - Some(abs_path) - } else { - None + if !is_file { + return None; + } + let ext = abs_path.extension().unwrap_or_default(); + if dirs.extensions.iter().all(|it| it.as_str() != ext) { + return None; } + Some(abs_path) }); - files - .map(|file| { + res.extend(files.map(|file| { let contents = read(file.as_path()); (file, contents) - }) - .collect() + })); + } + res } } } diff --git a/crates/vfs/src/loader.rs b/crates/vfs/src/loader.rs index 6de2e5b3f..9c6e4b6a7 100644 --- a/crates/vfs/src/loader.rs +++ b/crates/vfs/src/loader.rs @@ -3,10 +3,25 @@ use std::fmt; use paths::{AbsPath, AbsPathBuf}; -#[derive(Debug)] +#[derive(Debug, Clone)] pub enum Entry { Files(Vec), - Directory { path: AbsPathBuf, include: Vec }, + Directories(Directories), +} + +/// Specifies a set of files on the file system. +/// +/// A file is included if: +/// * it has included extension +/// * it is under an `include` path +/// * it is not under `exclude` path +/// +/// If many include/exclude paths match, the longest one wins. +#[derive(Debug, Clone)] +pub struct Directories { + pub extensions: Vec, + pub include: Vec, + pub exclude: Vec, } #[derive(Debug)] @@ -33,21 +48,70 @@ pub trait Handle: fmt::Debug { impl Entry { pub fn rs_files_recursively(base: AbsPathBuf) -> Entry { - Entry::Directory { path: base, include: globs(&["*.rs", "!/.git/"]) } + Entry::Directories(dirs(base, &[".git"])) } pub fn local_cargo_package(base: AbsPathBuf) -> Entry { - Entry::Directory { path: base, include: globs(&["*.rs", "!/target/", "!/.git/"]) } + Entry::Directories(dirs(base, &[".git", "target"])) } pub fn cargo_package_dependency(base: AbsPathBuf) -> Entry { - Entry::Directory { - path: base, - include: globs(&["*.rs", "!/tests/", "!/examples/", "!/benches/", "!/.git/"]), + Entry::Directories(dirs(base, &[".git", "/tests", "/examples", "/benches"])) + } + + pub fn contains_file(&self, path: &AbsPath) -> bool { + match self { + Entry::Files(files) => files.iter().any(|it| it == path), + Entry::Directories(dirs) => dirs.contains_file(path), + } + } + pub fn contains_dir(&self, path: &AbsPath) -> bool { + match self { + Entry::Files(_) => false, + Entry::Directories(dirs) => dirs.contains_dir(path), + } + } +} + +impl Directories { + pub fn contains_file(&self, path: &AbsPath) -> bool { + let ext = path.extension().unwrap_or_default(); + if self.extensions.iter().all(|it| it.as_str() != ext) { + return false; + } + self.includes_path(path) + } + pub fn contains_dir(&self, path: &AbsPath) -> bool { + self.includes_path(path) + } + fn includes_path(&self, path: &AbsPath) -> bool { + let mut include = None; + for incl in &self.include { + if is_prefix(incl, path) { + include = Some(match include { + Some(prev) if is_prefix(incl, prev) => prev, + _ => incl, + }) + } + } + let include = match include { + Some(it) => it, + None => return false, + }; + for excl in &self.exclude { + if is_prefix(excl, path) && is_prefix(include, excl) { + return false; + } + } + return true; + + fn is_prefix(short: &AbsPath, long: &AbsPath) -> bool { + long.strip_prefix(short).is_some() } } } -fn globs(globs: &[&str]) -> Vec { - globs.iter().map(|it| it.to_string()).collect() +fn dirs(base: AbsPathBuf, exclude: &[&str]) -> Directories { + let exclude = exclude.iter().map(|it| base.join(it)).collect::>(); + Directories { extensions: vec!["rs".to_string()], include: vec![base], exclude } } impl fmt::Debug for Message { diff --git a/xtask/tests/tidy.rs b/xtask/tests/tidy.rs index 72088e414..ca14e8ac1 100644 --- a/xtask/tests/tidy.rs +++ b/xtask/tests/tidy.rs @@ -54,7 +54,6 @@ fn check_licenses() { let expected = " 0BSD OR MIT OR Apache-2.0 Apache-2.0 -Apache-2.0 / MIT Apache-2.0 OR BSL-1.0 Apache-2.0 OR MIT Apache-2.0 WITH LLVM-exception OR Apache-2.0 OR MIT -- cgit v1.2.3