Skip to content

Commit

Permalink
Auto merge of #4378 - behnam:ignore, r=alexcrichton
Browse files Browse the repository at this point in the history
[sources/path] Support leading slash in glob patterns

Background: #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 <#4377> for suggestion
to throw warning on useless/invalid patterns in these fields.
  • Loading branch information
bors committed Aug 8, 2017
2 parents fe56d43 + 5641cbd commit 641bff8
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 15 deletions.
27 changes: 16 additions & 11 deletions src/cargo/sources/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
})
};
Expand All @@ -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)
}
};

Expand Down Expand Up @@ -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()
))?;
Expand All @@ -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()
))?;
Expand All @@ -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()
))?;
Expand All @@ -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()
))?;
Expand Down
4 changes: 0 additions & 4 deletions tests/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 [..]
Expand Down Expand Up @@ -351,7 +349,6 @@ See [..]
[ARCHIVING] [..]
[ARCHIVING] [..]
[ARCHIVING] [..]
[ARCHIVING] [..]
"));

assert_that(&p.root().join("target/package/foo-0.0.1.crate"), existing_file());
Expand All @@ -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
Expand Down

0 comments on commit 641bff8

Please sign in to comment.