From 5641cbde4b21fc4c2e39d484448f0d35b3ca4bc0 Mon Sep 17 00:00:00 2001 From: Behnam Esfahbod Date: Mon, 7 Aug 2017 12:48:12 -0700 Subject: [PATCH] [sources/path] Support leading slash in glob patterns Background: https://github.com/rust-lang/cargo/issues/4268 This diff takes us to **Stage 1.1** of the migration plan by allowing glob patterns to include a leading slash, so that glob patterns can be updated, if needed, to start with a slash, closer to the future behavior with gitignore-like matching. Why is this stage needed? It's common to have `package.include` set like this: ``` include = ["src/**"] ``` In old interpretation, this would only include all files under the `src` directory under the package root. With the new interpretation, this would match any path with some directory called `src`, even if it's not directly under the package root. After this patch, package owners can start marking glob patters with a leading slash to fix the warning thrown, if any. One thing to notice here is that there are no extra matchings, but, if a manifest has already a pattern with a leading slash, this would silently start matching it with the paths. I believe this is fine, since the old behavior would have been for the pattern to not match anything, therefore the item was useless. See also for suggestion to throw warning on useless/invalid patterns in these fields. --- src/cargo/sources/path.rs | 27 ++++++++++++++++----------- tests/package.rs | 4 ---- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/src/cargo/sources/path.rs b/src/cargo/sources/path.rs index 317133f0b70..76b274b7a82 100644 --- a/src/cargo/sources/path.rs +++ b/src/cargo/sources/path.rs @@ -110,7 +110,12 @@ impl<'cfg> PathSource<'cfg> { // glob-like matching rules let glob_parse = |p: &String| { - Pattern::new(p).map_err(|e| { + let pattern: &str = if p.starts_with('/') { + &p[1..p.len()] + } else { + &p + }; + Pattern::new(pattern).map_err(|e| { CargoError::from(format!("could not parse glob pattern `{}`: {}", p, e)) }) }; @@ -128,15 +133,15 @@ impl<'cfg> PathSource<'cfg> { .collect::, _>>()?; let glob_should_package = |relative_path: &Path| -> bool { + fn glob_match(patterns: &Vec, relative_path: &Path) -> bool { + patterns.iter().any(|pattern| pattern.matches_path(relative_path)) + } + // include and exclude options are mutually exclusive. if no_include_option { - !glob_exclude.iter().any(|pattern| { - pattern.matches_path(relative_path) - }) + !glob_match(&glob_exclude, relative_path) } else { - glob_include.iter().any(|pattern| { - pattern.matches_path(relative_path) - }) + glob_match(&glob_include, relative_path) } }; @@ -197,7 +202,7 @@ impl<'cfg> PathSource<'cfg> { .shell() .warn(format!( "Pattern matching for Cargo's include/exclude fields is changing and \ - file `{}` WILL be excluded in the next Cargo version.\n\ + file `{}` WILL be excluded in a future Cargo version.\n\ See https://github.com/rust-lang/cargo/issues/4268 for more info", relative_path.display() ))?; @@ -206,7 +211,7 @@ impl<'cfg> PathSource<'cfg> { .shell() .warn(format!( "Pattern matching for Cargo's include/exclude fields is changing and \ - file `{}` WILL NOT be included in the next Cargo version.\n\ + file `{}` WILL NOT be included in a future Cargo version.\n\ See https://github.com/rust-lang/cargo/issues/4268 for more info", relative_path.display() ))?; @@ -217,7 +222,7 @@ impl<'cfg> PathSource<'cfg> { .shell() .warn(format!( "Pattern matching for Cargo's include/exclude fields is changing and \ - file `{}` WILL NOT be excluded in the next Cargo version.\n\ + file `{}` WILL NOT be excluded in a future Cargo version.\n\ See https://github.com/rust-lang/cargo/issues/4268 for more info", relative_path.display() ))?; @@ -226,7 +231,7 @@ impl<'cfg> PathSource<'cfg> { .shell() .warn(format!( "Pattern matching for Cargo's include/exclude fields is changing and \ - file `{}` WILL be included in the next Cargo version.\n\ + file `{}` WILL be included in a future Cargo version.\n\ See https://github.com/rust-lang/cargo/issues/4268 for more info", relative_path.display() ))?; diff --git a/tests/package.rs b/tests/package.rs index 1a01ac0cdb8..334fd7a526b 100644 --- a/tests/package.rs +++ b/tests/package.rs @@ -321,8 +321,6 @@ See [..] See [..] [WARNING] [..] file `dir_root_3[/]some_dir[/]file` WILL be excluded [..] See [..] -[WARNING] [..] file `file_root_2` WILL be excluded [..] -See [..] [WARNING] [..] file `some_dir[/]dir_deep_1[/]some_dir[/]file` WILL be excluded [..] See [..] [WARNING] [..] file `some_dir[/]dir_deep_3[/]some_dir[/]file` WILL be excluded [..] @@ -351,7 +349,6 @@ See [..] [ARCHIVING] [..] [ARCHIVING] [..] [ARCHIVING] [..] -[ARCHIVING] [..] ")); assert_that(&p.root().join("target/package/foo-0.0.1.crate"), existing_file()); @@ -362,7 +359,6 @@ Cargo.toml dir_root_1[/]some_dir[/]file dir_root_2[/]some_dir[/]file dir_root_3[/]some_dir[/]file -file_root_2 file_root_3 file_root_4 file_root_5