Auto merge of #7170 - ehuss:no-more-glob, r=alexcrichton

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

Author: bors

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<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)?;
@@ -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)?;
 
@@ -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).

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,
     );
 }