rust/ripgrep
1
0
Fork 0
mirror of https://github.com/BurntSushi/ripgrep.git synced 2025-05-13 21:26:27 +02:00
Code Issues Releases Activity

Fix handling of absolute patterns in parent gitignore files.

Browse Source 
If a gitignore file in a *parent* directory is used, then it must be
matched relative to the directory it's in. ripgrep wasn't actually
adhering to this rule. Consider an example:

  .gitignore
  src
    llvm
      foo

Where `.gitignore` contains `/llvm/` and `foo` contains `test`. When
running `rg test` at the top-level directory, `foo` is correctly searched.
If you `cd` into `src` and re-run the same search, `foo` is ignored because
the `/llvm/` pattern is interpreted with respect to the current working
directory, which is wrong. The problem is that the path of `llvm` is
`./llvm`, which makes it look like it should match.

We fix this by rebuilding the directory path of each file when traversing
gitignores in parent directories. This does come with a small performance
hit.

Fixes #25.
This commit is contained in:
Andrew Gallant 2016-09-24 18:40:50 -04:00
parent 56fe93d343
commit 346bad7dfc
2 changed files with 66 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

69
src/ignore.rs
View File

@ -14,12 +14,13 @@ of `IgnoreDir`s for use during directory traversal.
*/
use std::error::Error as StdError;
use std::ffi::OsString;
use std::fmt;
use std::io;
use std::path::{Path, PathBuf};
use gitignore::{self, Gitignore, GitignoreBuilder, Match, Pattern};
use pathutil::is_hidden;
use pathutil::{file_name, is_hidden};
use types::Types;
const IGNORE_NAMES: &'static [&'static str] = &[
@ -78,7 +79,9 @@ impl From<gitignore::Error> for Error {
pub struct Ignore {
/// A stack of ignore patterns at each directory level of traversal.
/// A directory that contributes no ignore patterns is `None`.
stack: Vec<Option<IgnoreDir>>,
stack: Vec<IgnoreDir>,
/// A stack of parent directories above the root of the current search.
parent_stack: Vec<IgnoreDir>,
/// A set of override globs that are always checked first. A match (whether
/// it's whitelist or blacklist) trumps anything in stack.
overrides: Overrides,
@ -96,6 +99,7 @@ impl Ignore {
pub fn new() -> Ignore {
Ignore {
stack: vec![],
parent_stack: vec![],
overrides: Overrides::new(None),
types: Types::empty(),
ignore_hidden: true,
@ -142,7 +146,7 @@ impl Ignore {
let mut ignore_dir_results = vec![];
while let Some(parent) = path.parent() {
if self.no_ignore {
ignore_dir_results.push(Ok(None));
ignore_dir_results.push(Ok(IgnoreDir::empty(parent)));
} else {
if saw_git {
ignore_names.retain(|&name| name != ".gitignore");
@ -157,7 +161,7 @@ impl Ignore {
}
for ignore_dir_result in ignore_dir_results.into_iter().rev() {
try!(self.push_ignore_dir(ignore_dir_result));
self.parent_stack.push(try!(ignore_dir_result));
}
Ok(())
}
@ -168,7 +172,7 @@ impl Ignore {
/// stack (and therefore should be popped).
pub fn push<P: AsRef<Path>>(&mut self, path: P) -> Result<(), Error> {
if self.no_ignore {
self.stack.push(None);
self.stack.push(IgnoreDir::empty(path));
return Ok(());
}
self.push_ignore_dir(IgnoreDir::new(path))
@ -179,7 +183,7 @@ impl Ignore {
/// If the result given contains an error, then it is returned.
pub fn push_ignore_dir(
&mut self,
result: Result<Option<IgnoreDir>, Error>,
result: Result<IgnoreDir, Error>,
) -> Result<(), Error> {
match result {
Ok(id) => {
@ -188,7 +192,7 @@ impl Ignore {
}
Err(err) => {
// Don't leave the stack in an inconsistent state.
self.stack.push(None);
self.stack.push(IgnoreDir::empty("error"));
Err(err)
}
}
@ -213,9 +217,8 @@ impl Ignore {
return true;
}
if !self.no_ignore {
for id in self.stack.iter().rev().filter_map(|id| id.as_ref()) {
for id in self.stack.iter().rev() {
let mat = id.matched(path, is_dir);
// println!("{}, {:?}, {:?}, {:?}\n", path.display(), is_dir, mat, id);
if let Some(is_ignored) = self.ignore_match(path, mat) {
if is_ignored {
return true;
@ -225,6 +228,21 @@ impl Ignore {
break;
}
}
let mut path = path.to_path_buf();
for id in self.parent_stack.iter().rev() {
if let Some(ref dirname) = id.name {
path = Path::new(dirname).join(path);
}
let mat = id.matched(&*path, is_dir);
if let Some(is_ignored) = self.ignore_match(&*path, mat) {
if is_ignored {
return true;
}
// If this path is whitelisted by an ignore, then
// fallthrough and let the file type matcher have a say.
break;
}
}
}
let mat = self.types.matched(path, is_dir);
if let Some(is_ignored) = self.ignore_match(path, mat) {
@ -262,6 +280,8 @@ impl Ignore {
pub struct IgnoreDir {
/// The path to this directory as given.
path: PathBuf,
/// The directory name, if one exists.
name: Option<OsString>,
/// A single accumulation of glob patterns for this directory, matched
/// using gitignore semantics.
///
@ -278,10 +298,19 @@ impl IgnoreDir {
///
/// If no ignore glob patterns could be found in the directory then `None`
/// is returned.
pub fn new<P: AsRef<Path>>(path: P) -> Result<Option<IgnoreDir>, Error> {
pub fn new<P: AsRef<Path>>(path: P) -> Result<IgnoreDir, Error> {
IgnoreDir::with_ignore_names(path, IGNORE_NAMES.iter())
}
/// Create a new IgnoreDir that never matches anything with the given path.
pub fn empty<P: AsRef<Path>>(path: P) -> IgnoreDir {
IgnoreDir {
path: path.as_ref().to_path_buf(),
name: file_name(path.as_ref()).map(|s| s.to_os_string()),
gi: None,
}
}
/// Create a new matcher for the given directory using only the ignore
/// patterns found in the file names given.
///
@ -294,12 +323,9 @@ impl IgnoreDir {
pub fn with_ignore_names<P: AsRef<Path>, S, I>(
path: P,
names: I,
) -> Result<Option<IgnoreDir>, Error>
) -> Result<IgnoreDir, Error>
where P: AsRef<Path>, S: AsRef<str>, I: Iterator<Item=S> {
let mut id = IgnoreDir {
path: path.as_ref().to_path_buf(),
gi: None,
};
let mut id = IgnoreDir::empty(path);
let mut ok = false;
let mut builder = GitignoreBuilder::new(&id.path);
// The ordering here is important. Later globs have higher precedence.
@ -307,11 +333,10 @@ impl IgnoreDir {
ok = builder.add_path(id.path.join(name.as_ref())).is_ok() || ok;
}
if !ok {
Ok(None)
} else {
id.gi = Some(try!(builder.build()));
Ok(Some(id))
return Ok(id);
}
id.gi = Some(try!(builder.build()));
Ok(id)
}
/// Returns true if and only if the given file path should be ignored
@ -393,6 +418,9 @@ mod tests {
let gi = builder.build().unwrap();
let id = IgnoreDir {
path: Path::new($root).to_path_buf(),
name: Path::new($root).file_name().map(|s| {
s.to_os_string()
}),
gi: Some(gi),
};
assert!(id.matched($path, false).is_ignored());
@ -410,6 +438,9 @@ mod tests {
let gi = builder.build().unwrap();
let id = IgnoreDir {
path: Path::new($root).to_path_buf(),
name: Path::new($root).file_name().map(|s| {
s.to_os_string()
}),
gi: Some(gi),
};
assert!(!id.matched($path, false).is_ignored());

16
tests/tests.rs
View File

@ -609,6 +609,22 @@ clean!(regression_16, "xyz", ".", |wd: WorkDir, mut cmd: Command| {
wd.assert_err(&mut cmd);
});
// See: https://github.com/BurntSushi/ripgrep/issues/25
clean!(regression_25, "test", ".", |wd: WorkDir, mut cmd: Command| {
wd.create(".gitignore", "/llvm/");
wd.create_dir("src/llvm");
wd.create("src/llvm/foo", "test");
let lines: String = wd.stdout(&mut cmd);
let expected = "src/llvm/foo:test\n";
assert_eq!(lines, expected);
cmd.current_dir(wd.path().join("src"));
let lines: String = wd.stdout(&mut cmd);
let expected = "llvm/foo:test\n";
assert_eq!(lines, expected);
});
// See: https://github.com/BurntSushi/ripgrep/issues/49
clean!(regression_49, "xyz", ".", |wd: WorkDir, mut cmd: Command| {
wd.create(".gitignore", "foo/bar");