diff options
| author | Andrew Gallant <jamslam@gmail.com> | 2016-11-09 16:45:21 -0500 |
|---|---|---|
| committer | Andrew Gallant <jamslam@gmail.com> | 2016-11-09 16:45:23 -0500 |
| commit | 2dce0dc0df3c9ade3d89dd1ed673213d76760509 (patch) | |
| tree | 5cad9fb9e824ac9858aea30ad3c7f3f2fd2e3678 | |
| parent | 2e5c3c05e822d55f3752fc0f1765f128a5e3b4e1 (diff) | |
Fix a bug with handling --ignore-file.
Namely, passing a directory to --ignore-file caused ripgrep to allocate
memory without bound.
The issue was that I got a bit overzealous with partial error
reporting. Namely, when processing a gitignore file, we should try
to use every pattern even if some patterns are invalid globs (e.g.,
a**b). In the process, I applied the same logic to I/O errors. In this
case, it manifest by attempting to read lines from a directory, which
appears to yield Results forever, where each Result is an error of the
form "you can't read from a directory silly." Since I treated it as a
partial error, ripgrep was just spinning and accruing each error in
memory, which caused the OOM killer to kick in.
Fixes #228
| -rw-r--r-- | ignore/src/gitignore.rs | 2 | ||||
| -rw-r--r-- | ignore/src/walk.rs | 2 | ||||
| -rw-r--r-- | tests/tests.rs | 7 |
3 files changed, 9 insertions, 2 deletions
diff --git a/ignore/src/gitignore.rs b/ignore/src/gitignore.rs index c44910ff..5523fa6d 100644 --- a/ignore/src/gitignore.rs +++ b/ignore/src/gitignore.rs @@ -311,7 +311,7 @@ impl GitignoreBuilder { Ok(line) => line, Err(err) => { errs.push(Error::Io(err).tagged(path, lineno)); - continue; + break; } }; if let Err(err) = self.add_line(Some(path.to_path_buf()), &line) { diff --git a/ignore/src/walk.rs b/ignore/src/walk.rs index a1ac2de5..f82b2335 100644 --- a/ignore/src/walk.rs +++ b/ignore/src/walk.rs @@ -451,7 +451,7 @@ impl WalkBuilder { pub fn add_ignore<P: AsRef<Path>>(&mut self, path: P) -> Option<Error> { let mut builder = GitignoreBuilder::new(""); let mut errs = PartialErrorBuilder::default(); - errs.maybe_push_ignore_io(builder.add(path)); + errs.maybe_push(builder.add(path)); match builder.build() { Ok(gi) => { self.ig_builder.add_ignore(gi); } Err(err) => { errs.push(err); } diff --git a/tests/tests.rs b/tests/tests.rs index 59cefb59..5c5546d3 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -893,6 +893,13 @@ clean!(regression_206, "test", ".", |wd: WorkDir, mut cmd: Command| { assert_eq!(lines, format!("{}:test\n", path("foo/bar.txt"))); }); +// See: https://github.com/BurntSushi/ripgrep/issues/228 +clean!(regression_228, "test", ".", |wd: WorkDir, mut cmd: Command| { + wd.create_dir("foo"); + cmd.arg("--ignore-file").arg("foo"); + wd.assert_err(&mut cmd); +}); + // See: https://github.com/BurntSushi/ripgrep/issues/20 sherlock!(feature_20_no_filename, "Sherlock", ".", |wd: WorkDir, mut cmd: Command| { |