Skip to content

Commit

Permalink
Auto merge of #7170 - ehuss:no-more-glob, r=alexcrichton
Browse files Browse the repository at this point in the history 
Remove include/exclude glob warning.

This removes the warning when a package include/exclude pattern produces different results from glob vs gitignore.

The pre-warnings were added in 1.21 (released 2017-10-12) #4270.
The migration was done in 1.36 (released 2019-07-04) #6924.
This will remove the warnings in 1.38 (to be released 2019-09-26).

Closes #4268
  • Loading branch information
@bors
bors committed Jul 23, 2019
2 parents 767ecea + 24d2850 commit d0f8284
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 154 deletions.
117 changes: 1 addition & 116 deletions src/cargo/sources/path.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use std::fs;
use std::path::{Path, PathBuf};

use filetime::FileTime;
use glob::Pattern;
use ignore::gitignore::GitignoreBuilder;
use ignore::Match;
use log::{trace, warn};
Expand Down Expand Up @@ -93,81 +92,10 @@ impl<'cfg> PathSource<'cfg> {
/// The basic assumption of this method is that all files in the directory
/// are relevant for building this package, but it also contains logic to
/// use other methods like .gitignore to filter the list of files.
///
/// ## Pattern matching strategy
///
/// Migrating from a glob-like pattern matching (using `glob` crate) to a
/// gitignore-like pattern matching (using `ignore` crate). The migration
/// stages are:
///
/// 1) Only warn users about the future change iff their matching rules are
/// affected.
///
/// 2) Switch to the new strategy and update documents. Still keep warning
/// affected users. (CURRENT STAGE)
///
/// 3) Drop the old strategy and no more warnings.
///
/// See rust-lang/cargo#4268 for more info.
pub fn list_files(&self, pkg: &Package) -> CargoResult<Vec<PathBuf>> {
let root = pkg.root();
let no_include_option = pkg.manifest().include().is_empty();

// Glob-like matching rules.

let glob_parse = |p: &String| {
let pattern: &str = if p.starts_with('/') {
&p[1..p.len()]
} else {
p
};
Pattern::new(pattern)
};

let glob_exclude = pkg
.manifest()
.exclude()
.iter()
.map(|p| glob_parse(p))
.collect::<Result<Vec<_>, _>>();

let glob_include = pkg
.manifest()
.include()
.iter()
.map(|p| glob_parse(p))
.collect::<Result<Vec<_>, _>>();

// Don't warn if using a negate pattern, since those weren't ever
// previously supported.
let has_negate = pkg
.manifest()
.exclude()
.iter()
.chain(pkg.manifest().include().iter())
.any(|p| p.starts_with('!'));
// Don't warn about glob mismatch if it doesn't parse.
let glob_is_valid = glob_exclude.is_ok() && glob_include.is_ok() && !has_negate;
let glob_exclude = glob_exclude.unwrap_or_else(|_| Vec::new());
let glob_include = glob_include.unwrap_or_else(|_| Vec::new());

let glob_should_package = |relative_path: &Path| -> bool {
fn glob_match(patterns: &[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_match(&glob_exclude, relative_path)
} else {
glob_match(&glob_include, relative_path)
}
};

// Ignore-like matching rules.

let mut exclude_builder = GitignoreBuilder::new(root);
for rule in pkg.manifest().exclude() {
exclude_builder.add_line(None, rule)?;
Expand Down Expand Up @@ -201,8 +129,6 @@ impl<'cfg> PathSource<'cfg> {
}
};

// Matching to paths.

let mut filter = |path: &Path| -> CargoResult<bool> {
let relative_path = path.strip_prefix(root)?;

Expand All @@ -213,48 +139,7 @@ impl<'cfg> PathSource<'cfg> {
return Ok(true);
}

let glob_should_package = glob_should_package(relative_path);
let ignore_should_package = ignore_should_package(relative_path)?;

if glob_is_valid && glob_should_package != ignore_should_package {
if glob_should_package {
if no_include_option {
self.config.shell().warn(format!(
"Pattern matching for Cargo's include/exclude fields has changed and \
file `{}` is now excluded.\n\
See <https://github.com/rust-lang/cargo/issues/4268> for more \
information.",
relative_path.display()
))?;
} else {
self.config.shell().warn(format!(
"Pattern matching for Cargo's include/exclude fields has changed and \
file `{}` is no longer included.\n\
See <https://github.com/rust-lang/cargo/issues/4268> for more \
information.",
relative_path.display()
))?;
}
} else if no_include_option {
self.config.shell().warn(format!(
"Pattern matching for Cargo's include/exclude fields has changed and \
file `{}` is NOT excluded.\n\
See <https://github.com/rust-lang/cargo/issues/4268> for more \
information.",
relative_path.display()
))?;
} else {
self.config.shell().warn(format!(
"Pattern matching for Cargo's include/exclude fields has changed and \
file `{}` is now included.\n\
See <https://github.com/rust-lang/cargo/issues/4268> for more \
information.",
relative_path.display()
))?;
}
}

Ok(ignore_should_package)
ignore_should_package(relative_path)
};

// Attempt Git-prepopulate only if no `include` (see rust-lang/cargo#4135).
Expand Down
43 changes: 5 additions & 38 deletions tests/testsuite/package.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -367,18 +367,6 @@ fn exclude() {
"\
[WARNING] manifest has no description[..]
See https://doc.rust-lang.org/cargo/reference/manifest.html#package-metadata for more info.
[WARNING] [..] file `dir_root_1/some_dir/file` is now excluded.
See [..]
[WARNING] [..] file `dir_root_2/some_dir/file` is now excluded.
See [..]
[WARNING] [..] file `dir_root_3/some_dir/file` is now excluded.
See [..]
[WARNING] [..] file `some_dir/dir_deep_1/some_dir/file` is now excluded.
See [..]
[WARNING] [..] file `some_dir/dir_deep_3/some_dir/file` is now excluded.
See [..]
[WARNING] [..] file `some_dir/file_deep_1` is now excluded.
See [..]
[PACKAGING] foo v0.0.1 ([..])
[ARCHIVING] Cargo.toml
[ARCHIVING] file_root_3
Expand Down Expand Up @@ -1172,13 +1160,7 @@ fn include_cargo_toml_implicit() {
.run();
}

fn include_exclude_test(
include: &str,
exclude: &str,
files: &[&str],
expected: &str,
has_warnings: bool,
) {
fn include_exclude_test(include: &str, exclude: &str, files: &[&str], expected: &str) {
let mut pb = project().file(
"Cargo.toml",
&format!(
Expand All @@ -1203,13 +1185,10 @@ fn include_exclude_test(
}
let p = pb.build();

let mut e = p.cargo("package --list");
if has_warnings {
e.with_stderr_contains("[..]");
} else {
e.with_stderr("");
}
e.with_stdout(expected).run();
p.cargo("package --list")
.with_stderr("")
.with_stdout(expected)
.run();
p.root().rm_rf();
}

Expand All @@ -1230,7 +1209,6 @@ fn package_include_ignore_only() {
src/abc2.rs\n\
src/lib.rs\n\
",
false,
)
}

Expand All @@ -1246,7 +1224,6 @@ fn gitignore_patterns() {
foo\n\
x/foo/y\n\
",
true,
);

include_exclude_test(
Expand All @@ -1256,7 +1233,6 @@ fn gitignore_patterns() {
"Cargo.toml\n\
foo\n\
",
false,
);

include_exclude_test(
Expand All @@ -1269,7 +1245,6 @@ fn gitignore_patterns() {
foo\n\
src/lib.rs\n\
",
true,
);

include_exclude_test(
Expand All @@ -1292,7 +1267,6 @@ fn gitignore_patterns() {
other\n\
src/lib.rs\n\
",
false,
);

include_exclude_test(
Expand All @@ -1302,7 +1276,6 @@ fn gitignore_patterns() {
"Cargo.toml\n\
a/foo/bar\n\
",
false,
);

include_exclude_test(
Expand All @@ -1312,7 +1285,6 @@ fn gitignore_patterns() {
"Cargo.toml\n\
foo/x/y/z\n\
",
false,
);

include_exclude_test(
Expand All @@ -1324,7 +1296,6 @@ fn gitignore_patterns() {
a/x/b\n\
a/x/y/b\n\
",
false,
);
}

Expand All @@ -1338,7 +1309,6 @@ fn gitignore_negate() {
Cargo.toml\n\
src/lib.rs\n\
",
false,
);

// NOTE: This is unusual compared to git. Git treats `src/` as a
Expand All @@ -1352,7 +1322,6 @@ fn gitignore_negate() {
"Cargo.toml\n\
src/lib.rs\n\
",
false,
);

include_exclude_test(
Expand All @@ -1362,7 +1331,6 @@ fn gitignore_negate() {
"Cargo.toml\n\
src/lib.rs\n\
",
false,
);

include_exclude_test(
Expand All @@ -1372,6 +1340,5 @@ fn gitignore_negate() {
"Cargo.toml\n\
foo.rs\n\
",
false,
);
}

0 comments on commit d0f8284

Please sign in to comment.