Auto merge of #9186 - weihanglo:issue-9054, r=alexcrichton

Respect Cargo.toml `[package.exclude]` even not in a git repo.

May resolves #9054

This bug (or feature?) has been lingering for a while. #7680 fixed the `cargo package` part but `cargo vendor` is still affected by the heuristic rule of ignoring dotfiles. ~~I propose to drop the rule and include dotfiles by default even if the package is not under git-controlled~~. See below.

## Updated: Changes Summary

`cargo vendor` vendors dependencies without git-controlled but `cargo package` often runs under a VCS like git. [These lines](https://github.com/rust-lang/cargo/blob/1ca930b/src/cargo/sources/path.rs#L161-L168) are where they diverges: `fn list_files_walk_except_dot_files_and_dirs` builds [its own ignore instance], which cannot merge with other filter rules from `[package.exclude]`. This causes some patterns to not work as expected, such as re-including file after ignoring dotfiles `[.*, !negated_file]`.

To make re-include (negate) rule works, this patch adds the excluding dotfiles rule directly into the `package.exclude` ignore instance if **_no include option nor git repo exists_**. Other old behaviors should not change in this patch.

[its own ignore instance]: https://github.com/rust-lang/cargo/blob/1ca930b6/src/cargo/sources/path.rs#L364-L366

Author: bors

src/cargo/sources/path.rs

@@ -108,8 +108,17 @@ impl<'cfg> PathSource<'cfg> {
     fn _list_files(&self, pkg: &Package) -> CargoResult<Vec<PathBuf>> {
         let root = pkg.root();
         let no_include_option = pkg.manifest().include().is_empty();
+        let git_repo = if no_include_option {
+            self.discover_git_repo(root)?
+        } else {
+            None
+        };
 
         let mut exclude_builder = GitignoreBuilder::new(root);
+        if no_include_option && git_repo.is_none() {
+            // no include option and not git repo discovered (see rust-lang/cargo#7183).
+            exclude_builder.add_line(None, ".*")?;
+        }
         for rule in pkg.manifest().exclude() {
             exclude_builder.add_line(None, rule)?;
         }
@@ -161,23 +170,16 @@ impl<'cfg> PathSource<'cfg> {
 
         // Attempt Git-prepopulate only if no `include` (see rust-lang/cargo#4135).
         if no_include_option {
-            if let Some(result) = self.discover_git_and_list_files(pkg, root, &mut filter)? {
-                return Ok(result);
+            if let Some(repo) = git_repo {
+                return self.list_files_git(pkg, &repo, &mut filter);
             }
-            // no include option and not git repo discovered (see rust-lang/cargo#7183).
-            return self.list_files_walk_except_dot_files_and_dirs(pkg, &mut filter);
         }
         self.list_files_walk(pkg, &mut filter)
     }
 
-    // Returns `Some(_)` if found sibling `Cargo.toml` and `.git` directory;
-    // otherwise, caller should fall back on full file list.
-    fn discover_git_and_list_files(
-        &self,
-        pkg: &Package,
-        root: &Path,
-        filter: &mut dyn FnMut(&Path, bool) -> CargoResult<bool>,
-    ) -> CargoResult<Option<Vec<PathBuf>>> {
+    /// Returns `Some(git2::Repository)` if found sibling `Cargo.toml` and `.git`
+    /// directory; otherwise, caller should fall back on full file list.
+    fn discover_git_repo(&self, root: &Path) -> CargoResult<Option<git2::Repository>> {
         let repo = match git2::Repository::discover(root) {
             Ok(repo) => repo,
             Err(e) => {
@@ -212,7 +214,7 @@ impl<'cfg> PathSource<'cfg> {
         };
         let manifest_path = repo_relative_path.join("Cargo.toml");
         if index.get_path(&manifest_path, 0).is_some() {
-            return Ok(Some(self.list_files_git(pkg, &repo, filter)?));
+            return Ok(Some(repo));
         }
         // Package Cargo.toml is not in git, don't use git to guide our selection.
         Ok(None)
@@ -356,27 +358,6 @@ impl<'cfg> PathSource<'cfg> {
         }
     }
 
-    fn list_files_walk_except_dot_files_and_dirs(
-        &self,
-        pkg: &Package,
-        filter: &mut dyn FnMut(&Path, bool) -> CargoResult<bool>,
-    ) -> CargoResult<Vec<PathBuf>> {
-        let root = pkg.root();
-        let mut exclude_dot_files_dir_builder = GitignoreBuilder::new(root);
-        exclude_dot_files_dir_builder.add_line(None, ".*")?;
-        let ignore_dot_files_and_dirs = exclude_dot_files_dir_builder.build()?;
-
-        let mut filter_ignore_dot_files_and_dirs =
-            |path: &Path, is_dir: bool| -> CargoResult<bool> {
-                let relative_path = path.strip_prefix(root)?;
-                match ignore_dot_files_and_dirs.matched_path_or_any_parents(relative_path, is_dir) {
-                    Match::Ignore(_) => Ok(false),
-                    _ => filter(path, is_dir),
-                }
-            };
-        self.list_files_walk(pkg, &mut filter_ignore_dot_files_and_dirs)
-    }
-
     fn list_files_walk(
         &self,
         pkg: &Package,

tests/testsuite/vendor.rs

@@ -50,6 +50,50 @@ fn add_vendor_config(p: &Project) {
     );
 }
 
+#[cargo_test]
+fn package_exclude() {
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+                [package]
+                name = "foo"
+                version = "0.1.0"
+
+                [dependencies]
+                bar = "0.1.0"
+            "#,
+        )
+        .file("src/lib.rs", "")
+        .build();
+
+    Package::new("bar", "0.1.0")
+        .file(
+            "Cargo.toml",
+            r#"
+                [package]
+                name = "bar"
+                version = "0.1.0"
+                exclude = [".*", "!.include", "!.dotdir/include"]
+            "#,
+        )
+        .file("src/lib.rs", "")
+        .file(".exclude", "")
+        .file(".include", "")
+        .file(".dotdir/exclude", "")
+        .file(".dotdir/include", "")
+        .publish();
+
+    p.cargo("vendor --respect-source-config").run();
+    let csum = dbg!(p.read_file("vendor/bar/.cargo-checksum.json"));
+    assert!(csum.contains(".include"));
+    assert!(!csum.contains(".exclude"));
+    assert!(!csum.contains(".dotdir/exclude"));
+    // Gitignore doesn't re-include a file in an excluded parent directory,
+    // even if negating it explicitly.
+    assert!(!csum.contains(".dotdir/include"));
+}
+
 #[cargo_test]
 fn two_versions() {
     let p = project()