Don't require Cargo.toml to be in root of a git repo for path source walking.

Author: ehuss

src/cargo/sources/path.rs

@@ -177,40 +177,49 @@ impl<'cfg> PathSource<'cfg> {
         root: &Path,
         filter: &mut dyn FnMut(&Path, bool) -> CargoResult<bool>,
     ) -> Option<CargoResult<Vec<PathBuf>>> {
-        // If this package is in a Git repository, then we really do want to
-        // query the Git repository as it takes into account items such as
-        // `.gitignore`. We're not quite sure where the Git repository is,
-        // however, so we do a bit of a probe.
-        //
-        // We walk this package's path upwards and look for a sibling
-        // `Cargo.toml` and `.git` directory. If we find one then we assume that
-        // we're part of that repository.
-        let mut cur = root;
-        loop {
-            if cur.join("Cargo.toml").is_file() {
-                // If we find a Git repository next to this `Cargo.toml`, we still
-                // check to see if we are indeed part of the index. If not, then
-                // this is likely an unrelated Git repo, so keep going.
-                if let Ok(repo) = git2::Repository::open(cur) {
-                    let index = match repo.index() {
-                        Ok(index) => index,
-                        Err(err) => return Some(Err(err.into())),
-                    };
-                    let path = root.strip_prefix(cur).unwrap().join("Cargo.toml");
-                    if index.get_path(&path, 0).is_some() {
-                        return Some(self.list_files_git(pkg, &repo, filter));
-                    }
-                }
+        let repo = match git2::Repository::discover(root) {
+            Ok(repo) => repo,
+            Err(_) => return None,
+        };
+        let index = match repo.index() {
+            Ok(index) => index,
+            Err(err) => {
+                let e = anyhow::Error::new(err).context(format!(
+                    "failed to open git index at {}",
+                    repo.path().display()
+                ));
+                return Some(Err(e));
             }
-            // Don't cross submodule boundaries.
-            if cur.join(".git").is_dir() {
-                break;
+        };
+        let repo_root = match repo.workdir() {
+            Some(dir) => dir,
+            None => {
+                return Some(Err(anyhow::format_err!(
+                    "did not expect repo at {} to be bare",
+                    repo.path().display()
+                )))
             }
-            match cur.parent() {
-                Some(parent) => cur = parent,
-                None => break,
+        };
+        let repo_relative_path = match root.strip_prefix(repo_root) {
+            Ok(path) => path,
+            Err(err) => {
+                let e = anyhow::Error::new(err).context(format!(
+                    "expected git repo {} to be parent of package {}",
+                    repo.path().display(),
+                    root.display()
+                ));
+                return Some(Err(e));
             }
+        };
+        // Git requires forward-slashes.
+        let repo_safe_path = repo_relative_path
+            .join("Cargo.toml")
+            .to_string_lossy()
+            .replace('\\', "/");
+        if index.get_path(Path::new(&repo_safe_path), 0).is_some() {
+            return Some(self.list_files_git(pkg, &repo, filter));
         }
+        // Package Cargo.toml is not in git, don't use git to guide our selection.
         None
     }
 

tests/testsuite/package.rs

@@ -6,7 +6,7 @@ use cargo_test_support::{
     basic_manifest, cargo_process, git, path2url, paths, project, publish::validate_crate_contents,
     registry, symlink_supported, t,
 };
-use std::fs::{read_to_string, File};
+use std::fs::{self, read_to_string, File};
 use std::path::Path;
 
 #[cargo_test]
@@ -1691,3 +1691,56 @@ fn package_restricted_windows() {
         )
         .run();
 }
+
+#[cargo_test]
+fn finds_git_in_parent() {
+    // Test where `Cargo.toml` is not in the root of the git repo.
+    let repo_path = paths::root().join("repo");
+    fs::create_dir(&repo_path).unwrap();
+    let p = project()
+        .at("repo/foo")
+        .file("Cargo.toml", &basic_manifest("foo", "0.1.0"))
+        .file("src/lib.rs", "")
+        .build();
+    let repo = git::init(&repo_path);
+    git::add(&repo);
+    git::commit(&repo);
+    p.change_file("ignoreme", "");
+    p.change_file("ignoreme2", "");
+    p.cargo("package --list --allow-dirty")
+        .with_stdout(
+            "\
+Cargo.toml
+Cargo.toml.orig
+ignoreme
+ignoreme2
+src/lib.rs
+",
+        )
+        .run();
+
+    p.change_file(".gitignore", "ignoreme");
+    p.cargo("package --list --allow-dirty")
+        .with_stdout(
+            "\
+.gitignore
+Cargo.toml
+Cargo.toml.orig
+ignoreme2
+src/lib.rs
+",
+        )
+        .run();
+
+    fs::write(repo_path.join(".gitignore"), "ignoreme2").unwrap();
+    p.cargo("package --list --allow-dirty")
+        .with_stdout(
+            "\
+.gitignore
+Cargo.toml
+Cargo.toml.orig
+src/lib.rs
+",
+        )
+        .run();
+}

tests/testsuite/publish_lockfile.rs

@@ -469,6 +469,7 @@ fn ignore_lockfile_inner() {
             "\
 [PACKAGING] bar v0.0.1 ([..])
 [ARCHIVING] .cargo_vcs_info.json
+[ARCHIVING] .gitignore
 [ARCHIVING] Cargo.lock
 [ARCHIVING] Cargo.toml
 [ARCHIVING] Cargo.toml.orig