[sources/path] Support leading slash in glob patterns
authorBehnam Esfahbod <behnam@zwnj.org>
Mon, 7 Aug 2017 19:48:12 +0000 (12:48 -0700)
committerBehnam Esfahbod <behnam@zwnj.org>
Mon, 7 Aug 2017 19:57:48 +0000 (12:57 -0700)
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 <https://github.com/rust-lang/cargo/issues/4377> for suggestion
to throw warning on useless/invalid patterns in these fields.

src/cargo/sources/path.rs
tests/package.rs

index 317133f0b70d4f433a3846c0e479adfa083def53..76b274b7a825b22d5aa0bd0f0805ab93d1254c3b 100644 (file)
@@ -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::<Result<Vec<_>, _>>()?;
 
         let glob_should_package = |relative_path: &Path| -> bool {
+            fn glob_match(patterns: &Vec<Pattern>, 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()
                             ))?;
index 1a01ac0cdb890e04e618d764b6f4b7186ba87bda..334fd7a526bb874933fb712beb715633e327e0ad 100644 (file)
@@ -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