Skip to content

fix: normalize path separators for --full-path glob matches on Windows #2089

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
petrroll wants to merge 1 commit into
base: master
Choose a base branch
from
+65 −30
Open

fix: normalize path separators for --full-path glob matches on Windows #2089

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
- Fire the "search pattern contains a path separator" diagnostic for any pattern containing `/`, not just patterns that happen to name an existing directory. Preserves the legacy Windows behaviour that also flags native `\` separators when the pattern resolves to a real directory. See #1873.
- Also fire the "search pattern contains a path separator" diagnostic for `--and` patterns, not only the primary positional pattern. `--and` patterns are matched against the file name just like the primary pattern, so a path separator in them silently returned zero results. See #1873.
- Fix bug where passing "-" as a directory argument didn't actually search that directory, see #849 (@Sean-Kenneth-Doherty).
- Fix full-path glob patterns containing path separators on Windows, see #2067 (@petrroll).

# 10.4.2

Expand Down
4 changes: 4 additions & 0 deletions src/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ pub struct Config {
/// Populated when `--full-path` is set; `None` means search by filename only.
pub full_path_base: Option<PathBuf>,

/// Whether to normalize native path separators before applying the search regex.
#[cfg(windows)]
pub normalize_path_separators: bool,

/// Whether to ignore hidden files and directories (or not).
pub ignore_hidden: bool,

Expand Down
2 changes: 2 additions & 0 deletions src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,8 @@ fn construct_config(mut opts: Opts, pattern_regexps: &[String]) -> Result<Config
Ok(Config {
case_sensitive,
full_path_base,
#[cfg(windows)]
normalize_path_separators: opts.glob && opts.full_path,
ignore_hidden: !(opts.hidden || opts.rg_alias_ignore()),
read_fdignore: !(opts.no_ignore || opts.rg_alias_ignore()),
read_vcsignore: !(opts.no_ignore || opts.rg_alias_ignore() || opts.no_ignore_vcs),
Expand Down
16 changes: 15 additions & 1 deletion src/walk.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -516,9 +516,23 @@ impl WorkerState {

let search_str = search_str_for_entry(entry_path, config.full_path_base.as_deref());

#[cfg_attr(not(windows), allow(unused_mut))]
let mut search_bytes = filesystem::osstr_to_bytes(search_str.as_ref());

// globset canonicalizes separators in glob patterns to '/', so the
// candidate path has to be canonicalized the same way.
#[cfg(windows)]
if config.normalize_path_separators {
for byte in search_bytes.to_mut() {
if *byte == b'\\' {
*byte = b'/';
}
}
}

if !patterns
.iter()
.all(|pat| pat.is_match(&filesystem::osstr_to_bytes(search_str.as_ref())))
.all(|pattern| pattern.is_match(&search_bytes))
{
return WalkState::Continue;
}
Expand Down
72 changes: 43 additions & 29 deletions tests/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -599,7 +599,6 @@ fn test_glob_searches() {
}

/// Glob-based searches (--glob) in combination with full path searches (--full-path)
#[cfg(not(windows))] // TODO: make this work on Windows
#[test]
fn test_full_path_glob_searches() {
let te = TestEnv::new(DEFAULT_DIRS, DEFAULT_FILES);
Expand All @@ -620,6 +619,35 @@ fn test_full_path_glob_searches() {
&["--glob", "--full-path", "**/one/*/*/*.foo"],
" one/two/three/d.foo",
);

// Every --and pattern is matched against the same full-path candidate.
te.assert_output(
&["--glob", "--full-path", "**/one/**", "--and", "**/*.foo"],
"one/b.foo
one/two/c.foo
one/two/three/d.foo",
);

#[cfg(windows)]
{
// globset accepts native separators in patterns and canonicalizes them
// to '/', so they need the same candidate normalization.
te.assert_output(
&["--glob", "--full-path", r"**\one\**\*.foo"],
"one/b.foo
one/two/c.foo
one/two/three/d.foo",
);

// --regex overrides --glob. Native-separator regexes must continue to
// receive the unmodified Windows path.
te.assert_output(
&["--glob", "--regex", "--full-path", r"\\one\\.*\.foo$"],
"one/b.foo
one/two/c.foo
one/two/three/d.foo",
);
}
}

#[test]
Expand Down Expand Up @@ -1273,8 +1301,7 @@ fn test_absolute_path() {
{abs_path}/one/two/three/
{abs_path}/one/two/three/d.foo
{abs_path}/one/two/three/directory_foo/
{abs_path}/symlink",
abs_path = &abs_path
{abs_path}/symlink"
),
);

Expand All @@ -1286,8 +1313,7 @@ fn test_absolute_path() {
{abs_path}/one/two/c.foo
{abs_path}/one/two/C.Foo2
{abs_path}/one/two/three/d.foo
{abs_path}/one/two/three/directory_foo/",
abs_path = &abs_path
{abs_path}/one/two/three/directory_foo/"
),
);
}
Expand All @@ -1305,8 +1331,7 @@ fn test_implicit_absolute_path() {
{abs_path}/one/two/c.foo
{abs_path}/one/two/C.Foo2
{abs_path}/one/two/three/d.foo
{abs_path}/one/two/three/directory_foo/",
abs_path = &abs_path
{abs_path}/one/two/three/directory_foo/"
),
);
}
Expand All @@ -1325,8 +1350,7 @@ fn test_normalized_absolute_path() {
{abs_path}/one/two/c.foo
{abs_path}/one/two/C.Foo2
{abs_path}/one/two/three/d.foo
{abs_path}/one/two/three/directory_foo/",
abs_path = &abs_path
{abs_path}/one/two/three/directory_foo/"
),
);
}
Expand Down Expand Up @@ -1560,7 +1584,7 @@ fn test_symlink_as_root() {
{dir}/one/two/three/d.foo
{dir}/one/two/three/directory_foo/
{dir}/symlink",
dir = &parent_parent
dir = parent_parent
),
);
}
Expand All @@ -1579,9 +1603,7 @@ fn test_symlink_and_absolute_path() {
{abs_path}/{expected_path}/C.Foo2
{abs_path}/{expected_path}/three/
{abs_path}/{expected_path}/three/d.foo
{abs_path}/{expected_path}/three/directory_foo/",
abs_path = &abs_path,
expected_path = expected_path
{abs_path}/{expected_path}/three/directory_foo/"
),
);
}
Expand All @@ -1597,8 +1619,7 @@ fn test_symlink_as_absolute_root() {
{abs_path}/symlink/C.Foo2
{abs_path}/symlink/three/
{abs_path}/symlink/three/d.foo
{abs_path}/symlink/three/directory_foo/",
abs_path = &abs_path
{abs_path}/symlink/three/directory_foo/"
),
);
}
Expand All @@ -1621,9 +1642,7 @@ fn test_symlink_and_full_path() {
&format!(
"{abs_path}/{expected_path}/three/
{abs_path}/{expected_path}/three/d.foo
{abs_path}/{expected_path}/three/directory_foo/",
abs_path = &abs_path,
expected_path = expected_path
{abs_path}/{expected_path}/three/directory_foo/"
),
);
}
Expand All @@ -1642,8 +1661,7 @@ fn test_symlink_and_full_path_abs_path() {
&format!(
"{abs_path}/symlink/three/
{abs_path}/symlink/three/d.foo
{abs_path}/symlink/three/directory_foo/",
abs_path = &abs_path
{abs_path}/symlink/three/directory_foo/"
),
);
}
Expand Down Expand Up @@ -1771,8 +1789,7 @@ fn test_exec() {
{abs_path}/one/two/C.Foo2
{abs_path}/one/two/c.foo
{abs_path}/one/two/three/d.foo
{abs_path}/one/two/three/directory_foo",
abs_path = &abs_path
{abs_path}/one/two/three/directory_foo"
),
);

Expand Down Expand Up @@ -1870,8 +1887,7 @@ fn test_exec_multi() {
test C.Foo2
test c.foo
test d.foo
test directory_foo",
abs_path = &abs_path
test directory_foo"
),
);

Expand Down Expand Up @@ -1926,8 +1942,7 @@ fn test_exec_batch() {
te.assert_output(
&["--absolute-path", "foo", "--exec-batch", "echo"],
&format!(
"{abs_path}/a.foo {abs_path}/one/b.foo {abs_path}/one/two/C.Foo2 {abs_path}/one/two/c.foo {abs_path}/one/two/three/d.foo {abs_path}/one/two/three/directory_foo",
abs_path = &abs_path
"{abs_path}/a.foo {abs_path}/one/b.foo {abs_path}/one/two/C.Foo2 {abs_path}/one/two/c.foo {abs_path}/one/two/three/d.foo {abs_path}/one/two/three/directory_foo"
),
);

Expand Down Expand Up @@ -2539,7 +2554,7 @@ fn test_base_directory() {

// Ignore base directory when absolute path is used
let (te, abs_path) = get_test_env_with_abs_path(DEFAULT_DIRS, DEFAULT_FILES);
let abs_base_dir = &format!("{abs_path}/one/two/", abs_path = &abs_path);
let abs_base_dir = &format!("{abs_path}/one/two/");
te.assert_output(
&["--base-directory", abs_base_dir, "foo", &abs_path],
&format!(
Expand All @@ -2548,8 +2563,7 @@ fn test_base_directory() {
{abs_path}/one/two/c.foo
{abs_path}/one/two/C.Foo2
{abs_path}/one/two/three/d.foo
{abs_path}/one/two/three/directory_foo/",
abs_path = &abs_path
{abs_path}/one/two/three/directory_foo/"
),
);
}
Expand Down