diff --git a/crates/ignore/src/dir.rs b/crates/ignore/src/dir.rs index 5939f32e84..ef1e3ef78e 100644 --- a/crates/ignore/src/dir.rs +++ b/crates/ignore/src/dir.rs @@ -91,7 +91,15 @@ struct IgnoreOptions { /// Ignore is a matcher useful for recursively walking one or more directories. #[derive(Clone, Debug)] -pub(crate) struct Ignore(Arc); +pub(crate) struct Ignore { + inner: Arc, + // Parent matchers are cached independently of the path being walked, but + // matching them still needs the canonicalized path originally passed to + // `add_parents`. For example, when walking `/tmp/project/src`, parent + // matchers use `/tmp/project/src` to rewrite `foo.py` before matching it + // against ignore files from `/tmp/project` and its ancestors. + absolute_base: Option>, +} #[derive(Clone, Debug)] struct IgnoreInner { @@ -112,12 +120,9 @@ struct IgnoreInner { /// /// If this is the root directory or there are otherwise no more /// directories to match, then `parent` is `None`. - parent: Option, + parent: Option>, /// Whether this is an absolute parent matcher, as added by add_parent. is_absolute_parent: bool, - /// The absolute base path of this matcher. Populated only if parent - /// directories are added. - absolute_base: Option>, /// The directory that gitignores should be interpreted relative to. /// /// Usually this is the directory containing the gitignore file. But in @@ -152,23 +157,22 @@ struct IgnoreInner { impl Ignore { /// Return the directory path of this matcher. + #[cfg(test)] pub(crate) fn path(&self) -> &Path { - &self.0.dir + &self.inner.dir } /// Return true if this matcher has no parent. pub(crate) fn is_root(&self) -> bool { - self.0.parent.is_none() - } - - /// Returns true if this matcher was added via the `add_parents` method. - pub(crate) fn is_absolute_parent(&self) -> bool { - self.0.is_absolute_parent + self.inner.parent.is_none() } /// Return this matcher's parent, if one exists. pub(crate) fn parent(&self) -> Option { - self.0.parent.clone() + self.inner.parent.as_ref().map(|parent| Ignore { + inner: parent.clone(), + absolute_base: self.absolute_base.clone(), + }) } /// Create a new `Ignore` matcher with the parent directories of `dir`. @@ -179,10 +183,10 @@ impl Ignore { &self, path: P, ) -> (Ignore, Option) { - if !self.0.opts.parents - && !self.0.opts.git_ignore - && !self.0.opts.git_exclude - && !self.0.opts.git_global + if !self.inner.opts.parents + && !self.inner.opts.git_ignore + && !self.inner.opts.git_exclude + && !self.inner.opts.git_global { // If we never need info from parent directories, then don't do // anything. @@ -211,25 +215,30 @@ impl Ignore { let mut errs = PartialErrorBuilder::default(); let mut ig = self.clone(); for parent in parents.into_iter().rev() { - let mut compiled = self.0.compiled.write().unwrap(); + let mut compiled = self.inner.compiled.write().unwrap(); if let Some(weak) = compiled.get(parent.as_os_str()) { if let Some(prebuilt) = weak.upgrade() { - ig = Ignore(prebuilt); + ig = Ignore { + inner: prebuilt, + absolute_base: Some(absolute_base.clone()), + }; continue; } } let (mut igtmp, err) = ig.add_child_path(parent); errs.maybe_push(err); igtmp.is_absolute_parent = true; - igtmp.absolute_base = Some(absolute_base.clone()); igtmp.has_git = - if self.0.opts.require_git && self.0.opts.git_ignore { + if self.inner.opts.require_git && self.inner.opts.git_ignore { parent.join(".git").exists() || parent.join(".jj").exists() } else { false }; let ig_arc = Arc::new(igtmp); - ig = Ignore(ig_arc.clone()); + ig = Ignore { + inner: ig_arc.clone(), + absolute_base: Some(absolute_base.clone()), + }; compiled.insert( parent.as_os_str().to_os_string(), Arc::downgrade(&ig_arc), @@ -251,13 +260,19 @@ impl Ignore { dir: P, ) -> (Ignore, Option) { let (ig, err) = self.add_child_path(dir.as_ref()); - (Ignore(Arc::new(ig)), err) + ( + Ignore { + inner: Arc::new(ig), + absolute_base: self.absolute_base.clone(), + }, + err, + ) } /// Like add_child, but takes a full path and returns an IgnoreInner. fn add_child_path(&self, dir: &Path) -> (IgnoreInner, Option) { - let check_vcs_dir = self.0.opts.require_git - && (self.0.opts.git_ignore || self.0.opts.git_exclude); + let check_vcs_dir = self.inner.opts.require_git + && (self.inner.opts.git_ignore || self.inner.opts.git_exclude); let git_type = if check_vcs_dir { dir.join(".git").metadata().ok().map(|md| md.file_type()) } else { @@ -267,44 +282,45 @@ impl Ignore { check_vcs_dir && (git_type.is_some() || dir.join(".jj").exists()); let mut errs = PartialErrorBuilder::default(); - let custom_ig_matcher = if self.0.custom_ignore_filenames.is_empty() { - Gitignore::empty() - } else { - let (m, err) = create_gitignore( - &dir, - &dir, - &self.0.custom_ignore_filenames, - self.0.opts.ignore_case_insensitive, - ); - errs.maybe_push(err); - m - }; - let ig_matcher = if !self.0.opts.ignore { + let custom_ig_matcher = + if self.inner.custom_ignore_filenames.is_empty() { + Gitignore::empty() + } else { + let (m, err) = create_gitignore( + &dir, + &dir, + &self.inner.custom_ignore_filenames, + self.inner.opts.ignore_case_insensitive, + ); + errs.maybe_push(err); + m + }; + let ig_matcher = if !self.inner.opts.ignore { Gitignore::empty() } else { let (m, err) = create_gitignore( &dir, &dir, &[".ignore"], - self.0.opts.ignore_case_insensitive, + self.inner.opts.ignore_case_insensitive, ); errs.maybe_push(err); m }; - let gi_matcher = if !self.0.opts.git_ignore { + let gi_matcher = if !self.inner.opts.git_ignore { Gitignore::empty() } else { let (m, err) = create_gitignore( &dir, &dir, &[".gitignore"], - self.0.opts.ignore_case_insensitive, + self.inner.opts.ignore_case_insensitive, ); errs.maybe_push(err); m }; - let gi_exclude_matcher = if !self.0.opts.git_exclude { + let gi_exclude_matcher = if !self.inner.opts.git_exclude { Gitignore::empty() } else { match resolve_git_commondir(dir, git_type) { @@ -313,7 +329,7 @@ impl Ignore { &dir, &git_dir, &["info/exclude"], - self.0.opts.ignore_case_insensitive, + self.inner.opts.ignore_case_insensitive, ); errs.maybe_push(err); m @@ -325,36 +341,38 @@ impl Ignore { } }; let ig = IgnoreInner { - compiled: self.0.compiled.clone(), + compiled: self.inner.compiled.clone(), dir: dir.to_path_buf(), - overrides: self.0.overrides.clone(), - types: self.0.types.clone(), - parent: Some(self.clone()), + overrides: self.inner.overrides.clone(), + types: self.inner.types.clone(), + parent: Some(self.inner.clone()), is_absolute_parent: false, - absolute_base: self.0.absolute_base.clone(), global_gitignores_relative_to: self - .0 + .inner .global_gitignores_relative_to .clone(), - explicit_ignores: self.0.explicit_ignores.clone(), - custom_ignore_filenames: self.0.custom_ignore_filenames.clone(), + explicit_ignores: self.inner.explicit_ignores.clone(), + custom_ignore_filenames: self + .inner + .custom_ignore_filenames + .clone(), custom_ignore_matcher: custom_ig_matcher, ignore_matcher: ig_matcher, - git_global_matcher: self.0.git_global_matcher.clone(), + git_global_matcher: self.inner.git_global_matcher.clone(), git_ignore_matcher: gi_matcher, git_exclude_matcher: gi_exclude_matcher, has_git, - opts: self.0.opts, + opts: self.inner.opts, }; (ig, errs.into_error_option()) } /// Returns true if at least one type of ignore rule should be matched. fn has_any_ignore_rules(&self) -> bool { - let opts = self.0.opts; + let opts = self.inner.opts; let has_custom_ignore_files = - !self.0.custom_ignore_filenames.is_empty(); - let has_explicit_ignores = !self.0.explicit_ignores.is_empty(); + !self.inner.custom_ignore_filenames.is_empty(); + let has_explicit_ignores = !self.inner.explicit_ignores.is_empty(); opts.ignore || opts.git_global @@ -370,7 +388,7 @@ impl Ignore { dent: &DirEntry, ) -> Match> { let m = self.matched(dent.path(), dent.is_dir()); - if m.is_none() && self.0.opts.hidden && is_hidden(dent) { + if m.is_none() && self.inner.opts.hidden && is_hidden(dent) { return Match::Ignore(IgnoreMatch::hidden()); } m @@ -395,9 +413,9 @@ impl Ignore { // regardless of whether it's whitelist/ignore, then we quit and // return that result immediately. Overrides have the highest // precedence. - if !self.0.overrides.is_empty() { + if !self.inner.overrides.is_empty() { let mat = self - .0 + .inner .overrides .matched(path, is_dir) .map(IgnoreMatch::overrides); @@ -414,9 +432,9 @@ impl Ignore { whitelisted = mat; } } - if !self.0.types.is_empty() { + if !self.inner.types.is_empty() { let mat = - self.0.types.matched(path, is_dir).map(IgnoreMatch::types); + self.inner.types.matched(path, is_dir).map(IgnoreMatch::types); if mat.is_ignore() { return mat; } else if mat.is_whitelist() { @@ -440,37 +458,42 @@ impl Ignore { mut m_gi_exclude, mut m_explicit, ) = (Match::None, Match::None, Match::None, Match::None, Match::None); - let any_git = - !self.0.opts.require_git || self.parents().any(|ig| ig.0.has_git); + let any_git = !self.inner.opts.require_git + || self.parents().any(|ig| ig.inner.has_git); let mut saw_git = false; - for ig in self.parents().take_while(|ig| !ig.0.is_absolute_parent) { + for ig in self.parents().take_while(|ig| !ig.inner.is_absolute_parent) + { if m_custom_ignore.is_none() { - m_custom_ignore = - ig.0.custom_ignore_matcher - .matched(path, is_dir) - .map(IgnoreMatch::gitignore); + m_custom_ignore = ig + .inner + .custom_ignore_matcher + .matched(path, is_dir) + .map(IgnoreMatch::gitignore); } if m_ignore.is_none() { - m_ignore = - ig.0.ignore_matcher - .matched(path, is_dir) - .map(IgnoreMatch::gitignore); + m_ignore = ig + .inner + .ignore_matcher + .matched(path, is_dir) + .map(IgnoreMatch::gitignore); } if any_git && !saw_git && m_gi.is_none() { - m_gi = - ig.0.git_ignore_matcher - .matched(path, is_dir) - .map(IgnoreMatch::gitignore); + m_gi = ig + .inner + .git_ignore_matcher + .matched(path, is_dir) + .map(IgnoreMatch::gitignore); } if any_git && !saw_git && m_gi_exclude.is_none() { - m_gi_exclude = - ig.0.git_exclude_matcher - .matched(path, is_dir) - .map(IgnoreMatch::gitignore); + m_gi_exclude = ig + .inner + .git_exclude_matcher + .matched(path, is_dir) + .map(IgnoreMatch::gitignore); } - saw_git = saw_git || ig.0.has_git; + saw_git = saw_git || ig.inner.has_git; } - if self.0.opts.parents { + if self.inner.opts.parents { if let Some(abs_parent_path) = self.absolute_base() { // What we want to do here is take the absolute base path of // this directory and join it with the path we're searching. @@ -481,7 +504,7 @@ impl Ignore { // this crate. let path = abs_parent_path.join( self.parents() - .take_while(|ig| !ig.0.is_absolute_parent) + .take_while(|ig| !ig.inner.is_absolute_parent) .last() .map_or(path, |ig| { // This is a weird special case when ripgrep users @@ -490,56 +513,63 @@ impl Ignore { // we don't bail out now, the code below will strip // a leading `.` from `path`, which might mangle // a hidden file name! - if ig.0.dir.as_path() == Path::new(".") { + if ig.inner.dir.as_path() == Path::new(".") { return path; } - let without_dot_slash = - strip_if_is_prefix("./", ig.0.dir.as_path()); + let without_dot_slash = strip_if_is_prefix( + "./", + ig.inner.dir.as_path(), + ); let relative_base = strip_if_is_prefix(without_dot_slash, path); strip_if_is_prefix("/", relative_base) }), ); - for ig in - self.parents().skip_while(|ig| !ig.0.is_absolute_parent) + for ig in self + .parents() + .skip_while(|ig| !ig.inner.is_absolute_parent) { if m_custom_ignore.is_none() { - m_custom_ignore = - ig.0.custom_ignore_matcher - .matched(&path, is_dir) - .map(IgnoreMatch::gitignore); + m_custom_ignore = ig + .inner + .custom_ignore_matcher + .matched(&path, is_dir) + .map(IgnoreMatch::gitignore); } if m_ignore.is_none() { - m_ignore = - ig.0.ignore_matcher - .matched(&path, is_dir) - .map(IgnoreMatch::gitignore); + m_ignore = ig + .inner + .ignore_matcher + .matched(&path, is_dir) + .map(IgnoreMatch::gitignore); } if any_git && !saw_git && m_gi.is_none() { - m_gi = - ig.0.git_ignore_matcher - .matched(&path, is_dir) - .map(IgnoreMatch::gitignore); + m_gi = ig + .inner + .git_ignore_matcher + .matched(&path, is_dir) + .map(IgnoreMatch::gitignore); } if any_git && !saw_git && m_gi_exclude.is_none() { - m_gi_exclude = - ig.0.git_exclude_matcher - .matched(&path, is_dir) - .map(IgnoreMatch::gitignore); + m_gi_exclude = ig + .inner + .git_exclude_matcher + .matched(&path, is_dir) + .map(IgnoreMatch::gitignore); } - saw_git = saw_git || ig.0.has_git; + saw_git = saw_git || ig.inner.has_git; } } } - for gi in self.0.explicit_ignores.iter().rev() { + for gi in self.inner.explicit_ignores.iter().rev() { if !m_explicit.is_none() { break; } m_explicit = gi.matched(&path, is_dir).map(IgnoreMatch::gitignore); } let m_global = if any_git { - self.0 + self.inner .git_global_matcher .matched(&path, is_dir) .map(IgnoreMatch::gitignore) @@ -557,29 +587,46 @@ impl Ignore { /// Returns an iterator over parent ignore matchers, including this one. pub(crate) fn parents(&self) -> Parents<'_> { - Parents(Some(self)) + Parents(Some(IgnoreRef { inner: &self.inner })) } /// Returns the first absolute path of the first absolute parent, if /// one exists. fn absolute_base(&self) -> Option<&Path> { - self.0.absolute_base.as_ref().map(|p| &***p) + self.absolute_base.as_ref().map(|p| &***p) + } +} + +#[derive(Clone, Copy)] +pub(crate) struct IgnoreRef<'a> { + inner: &'a IgnoreInner, +} + +impl IgnoreRef<'_> { + pub(crate) fn path(&self) -> &Path { + &self.inner.dir + } + + pub(crate) fn is_absolute_parent(&self) -> bool { + self.inner.is_absolute_parent } } /// An iterator over all parents of an ignore matcher, including itself. -/// -/// The lifetime `'a` refers to the lifetime of the initial `Ignore` matcher. -pub(crate) struct Parents<'a>(Option<&'a Ignore>); +pub(crate) struct Parents<'a>(Option>); impl<'a> Iterator for Parents<'a> { - type Item = &'a Ignore; + type Item = IgnoreRef<'a>; - fn next(&mut self) -> Option<&'a Ignore> { + fn next(&mut self) -> Option> { match self.0.take() { None => None, Some(ig) => { - self.0 = ig.0.parent.as_ref(); + self.0 = ig + .inner + .parent + .as_deref() + .map(|inner| IgnoreRef { inner }); Some(ig) } } @@ -674,27 +721,29 @@ impl IgnoreBuilder { Gitignore::empty() }; - Ignore(Arc::new(IgnoreInner { - compiled: Arc::new(RwLock::new(HashMap::new())), - dir: self.dir.clone(), - overrides: self.overrides.clone(), - types: self.types.clone(), - parent: None, - is_absolute_parent: true, + Ignore { + inner: Arc::new(IgnoreInner { + compiled: Arc::new(RwLock::new(HashMap::new())), + dir: self.dir.clone(), + overrides: self.overrides.clone(), + types: self.types.clone(), + parent: None, + is_absolute_parent: true, + global_gitignores_relative_to, + explicit_ignores: Arc::new(self.explicit_ignores.clone()), + custom_ignore_filenames: Arc::new( + self.custom_ignore_filenames.clone(), + ), + custom_ignore_matcher: Gitignore::empty(), + ignore_matcher: Gitignore::empty(), + git_global_matcher: Arc::new(git_global_matcher), + git_ignore_matcher: Gitignore::empty(), + git_exclude_matcher: Gitignore::empty(), + has_git: false, + opts: self.opts, + }), absolute_base: None, - global_gitignores_relative_to, - explicit_ignores: Arc::new(self.explicit_ignores.clone()), - custom_ignore_filenames: Arc::new( - self.custom_ignore_filenames.clone(), - ), - custom_ignore_matcher: Gitignore::empty(), - ignore_matcher: Gitignore::empty(), - git_global_matcher: Arc::new(git_global_matcher), - git_ignore_matcher: Gitignore::empty(), - git_exclude_matcher: Gitignore::empty(), - has_git: false, - opts: self.opts, - })) + } } /// Set the current directory used for matching global gitignores. @@ -946,7 +995,7 @@ fn strip_if_is_prefix<'a, P: AsRef + ?Sized>( #[cfg(test)] mod tests { - use std::{io::Write, path::Path}; + use std::{io::Write, path::Path, sync::Arc}; use crate::{ Error, dir::IgnoreBuilder, gitignore::Gitignore, tests::TempDir, @@ -1258,6 +1307,29 @@ mod tests { assert!(ig2.matched("src/foo", false).is_ignore()); } + #[test] + fn absolute_parent_matchers_are_cached_across_roots() { + let td = tmpdir(); + mkdirp(td.path().join(".git")); + mkdirp(td.path().join("src/build")); + mkdirp(td.path().join("tests/build")); + wfile(td.path().join(".gitignore"), "tests/**/build/\n"); + + let ig0 = IgnoreBuilder::new().build(); + let (src_parents, err) = ig0.add_parents(td.path().join("src")); + assert!(err.is_none()); + let (src, err) = src_parents.add_child(td.path().join("src")); + assert!(err.is_none()); + let (tests_parents, err) = ig0.add_parents(td.path().join("tests")); + assert!(err.is_none()); + let (tests, err) = tests_parents.add_child(td.path().join("tests")); + assert!(err.is_none()); + + assert!(Arc::ptr_eq(&src_parents.inner, &tests_parents.inner)); + assert!(src.matched("build", true).is_none()); + assert!(tests.matched("build", true).is_ignore()); + } + #[test] fn git_info_exclude_in_linked_worktree() { let td = tmpdir();