From 24d2850247458c800babfc8685bc93193eb13e50 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Tue, 23 Jul 2019 13:49:17 -0700 Subject: [PATCH] Remove include/exclude glob warning. --- src/cargo/sources/path.rs | 117 +------------------------------------ tests/testsuite/package.rs | 43 ++------------ 2 files changed, 6 insertions(+), 154 deletions(-) diff --git a/src/cargo/sources/path.rs b/src/cargo/sources/path.rs index 571a0a2ce18..af6d458c345 100644 --- a/src/cargo/sources/path.rs +++ b/src/cargo/sources/path.rs @@ -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}; @@ -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> { 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::, _>>(); - - let glob_include = pkg - .manifest() - .include() - .iter() - .map(|p| glob_parse(p)) - .collect::, _>>(); - - // 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)?; @@ -201,8 +129,6 @@ impl<'cfg> PathSource<'cfg> { } }; - // Matching to paths. - let mut filter = |path: &Path| -> CargoResult { let relative_path = path.strip_prefix(root)?; @@ -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 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 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 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 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). diff --git a/tests/testsuite/package.rs b/tests/testsuite/package.rs index affc22add32..92be2b2eee2 100644 --- a/tests/testsuite/package.rs +++ b/tests/testsuite/package.rs @@ -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 @@ -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!( @@ -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(); } @@ -1230,7 +1209,6 @@ fn package_include_ignore_only() { src/abc2.rs\n\ src/lib.rs\n\ ", - false, ) } @@ -1246,7 +1224,6 @@ fn gitignore_patterns() { foo\n\ x/foo/y\n\ ", - true, ); include_exclude_test( @@ -1256,7 +1233,6 @@ fn gitignore_patterns() { "Cargo.toml\n\ foo\n\ ", - false, ); include_exclude_test( @@ -1269,7 +1245,6 @@ fn gitignore_patterns() { foo\n\ src/lib.rs\n\ ", - true, ); include_exclude_test( @@ -1292,7 +1267,6 @@ fn gitignore_patterns() { other\n\ src/lib.rs\n\ ", - false, ); include_exclude_test( @@ -1302,7 +1276,6 @@ fn gitignore_patterns() { "Cargo.toml\n\ a/foo/bar\n\ ", - false, ); include_exclude_test( @@ -1312,7 +1285,6 @@ fn gitignore_patterns() { "Cargo.toml\n\ foo/x/y/z\n\ ", - false, ); include_exclude_test( @@ -1324,7 +1296,6 @@ fn gitignore_patterns() { a/x/b\n\ a/x/y/b\n\ ", - false, ); } @@ -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 @@ -1352,7 +1322,6 @@ fn gitignore_negate() { "Cargo.toml\n\ src/lib.rs\n\ ", - false, ); include_exclude_test( @@ -1362,7 +1331,6 @@ fn gitignore_negate() { "Cargo.toml\n\ src/lib.rs\n\ ", - false, ); include_exclude_test( @@ -1372,6 +1340,5 @@ fn gitignore_negate() { "Cargo.toml\n\ foo.rs\n\ ", - false, ); }