Fix transitive path dependencies in sdist

Author: messense

src/source_distribution.rs

@@ -1,7 +1,7 @@
 use crate::module_writer::ModuleWriter;
 use crate::{Metadata21, SDistWriter};
 use anyhow::{bail, Context, Result};
-use cargo_metadata::Metadata;
+use cargo_metadata::{Metadata, PackageId};
 use fs_err as fs;
 use std::collections::HashMap;
 use std::path::{Path, PathBuf};
@@ -10,6 +10,13 @@ use std::str;
 
 const LOCAL_DEPENDENCIES_FOLDER: &str = "local_dependencies";
 
+#[derive(Debug, Clone)]
+struct PathDependency {
+    id: PackageId,
+    name: String,
+    path: PathBuf,
+}
+
 /// We need cargo to load the local dependencies from the location where we put them in the source
 /// distribution. Since there is no cargo-backed way to replace dependencies
 /// (see https://github.com/rust-lang/cargo/issues/9170), we do a simple
@@ -19,7 +26,7 @@ const LOCAL_DEPENDENCIES_FOLDER: &str = "local_dependencies";
 /// This method is rather frail, but unfortunately I don't know a better solution.
 fn rewrite_cargo_toml(
     manifest_path: impl AsRef<Path>,
-    known_path_deps: &HashMap<String, PathBuf>,
+    known_path_deps: &HashMap<String, PathDependency>,
     root_crate: bool,
 ) -> Result<String> {
     let text = fs::read_to_string(&manifest_path).context(format!(
@@ -87,7 +94,7 @@ fn add_crate_to_source_distribution(
     writer: &mut SDistWriter,
     manifest_path: impl AsRef<Path>,
     prefix: impl AsRef<Path>,
-    known_path_deps: &HashMap<String, PathBuf>,
+    known_path_deps: &HashMap<String, PathDependency>,
     root_crate: bool,
 ) -> Result<()> {
     let output = Command::new("cargo")
@@ -157,7 +164,45 @@ fn add_crate_to_source_distribution(
     Ok(())
 }
 
-/// Creates aif source distribution, packing the root crate and all local dependencies
+/// Get path dependencies for a cargo package
+fn get_path_deps(
+    cargo_metadata: &Metadata,
+    resolve: &cargo_metadata::Resolve,
+    pkg_id: &cargo_metadata::PackageId,
+    visited: &HashMap<String, PathDependency>,
+) -> Result<HashMap<String, PathDependency>> {
+    // Parse ids in the format:
+    // on unix:    some_path_dep 0.1.0 (path+file:///home/konsti/maturin/test-crates/some_path_dep)
+    // on windows: some_path_dep 0.1.0 (path+file:///C:/konsti/maturin/test-crates/some_path_dep)
+    // This is not a good way to identify path dependencies, but I don't know a better one
+    let node = resolve
+        .nodes
+        .iter()
+        .find(|node| &node.id == pkg_id)
+        .context("Expected to get a node of dependency graph from cargo")?;
+    let path_deps = node
+        .deps
+        .iter()
+        .filter(|node| node.pkg.repr.contains("path+file://"))
+        .filter_map(|node| {
+            cargo_metadata.packages.iter().find_map(|pkg| {
+                if pkg.id.repr == node.pkg.repr && !visited.contains_key(&pkg.name) {
+                    let path_dep = PathDependency {
+                        id: pkg.id.clone(),
+                        name: pkg.name.clone(),
+                        path: PathBuf::from(&pkg.manifest_path),
+                    };
+                    Some((pkg.name.clone(), path_dep))
+                } else {
+                    None
+                }
+            })
+        })
+        .collect();
+    Ok(path_deps)
+}
+
+/// Creates a source distribution, packing the root crate and all local dependencies
 ///
 /// The source distribution format is specified in
 /// [PEP 517 under "build_sdist"](https://www.python.org/dev/peps/pep-0517/#build-sdist)
@@ -170,37 +215,24 @@ pub fn source_distribution(
     cargo_metadata: &Metadata,
     sdist_include: Option<&Vec<String>>,
 ) -> Result<PathBuf> {
-    // Parse ids in the format:
-    // on unix:    some_path_dep 0.1.0 (path+file:///home/konsti/maturin/test-crates/some_path_dep)
-    // on windows: some_path_dep 0.1.0 (path+file:///C:/konsti/maturin/test-crates/some_path_dep)
-    // This is not a good way to identify path dependencies, but I don't know a better one
     let resolve = cargo_metadata
         .resolve
         .as_ref()
         .context("Expected to get a dependency graph from cargo")?;
     let root = resolve
         .root
-        .as_ref()
+        .clone()
         .context("Expected to get a root package id of dependency graph from cargo")?;
-    let root_node = resolve
-        .nodes
-        .iter()
-        .find(|node| &node.id == root)
-        .context("Expected to get a root node of dependency graph from cargo")?;
-    let known_path_deps: HashMap<String, PathBuf> = root_node
-        .deps
-        .iter()
-        .filter(|node| node.pkg.repr.contains("path+file://"))
-        .filter_map(|node| {
-            cargo_metadata.packages.iter().find_map(|pkg| {
-                if pkg.id.repr == node.pkg.repr {
-                    Some((pkg.name.clone(), PathBuf::from(&pkg.manifest_path)))
-                } else {
-                    None
-                }
-            })
-        })
-        .collect();
+    let mut known_path_deps = HashMap::new();
+    let mut stack = vec![root];
+    while let Some(pkg_id) = stack.pop() {
+        let path_deps = get_path_deps(cargo_metadata, resolve, &pkg_id, &known_path_deps)?;
+        if path_deps.is_empty() {
+            continue;
+        }
+        stack.extend(path_deps.values().map(|dep| dep.id.clone()));
+        known_path_deps.extend(path_deps);
+    }
 
     let mut writer = SDistWriter::new(wheel_dir, metadata21)?;
     let root_dir = PathBuf::from(format!(
@@ -210,18 +242,18 @@ pub fn source_distribution(
     ));
 
     // Add local path dependencies
-    for (name, path) in known_path_deps.iter() {
+    for (name, path_dep) in known_path_deps.iter() {
         add_crate_to_source_distribution(
             &mut writer,
-            &path,
+            &path_dep.path,
             &root_dir.join(LOCAL_DEPENDENCIES_FOLDER).join(name),
             &known_path_deps,
             false,
         )
         .context(format!(
             "Failed to add local dependency {} at {} to the source distribution",
             name,
-            path.display()
+            path_dep.path.display()
         ))?;
     }
 

test-crates/lib_with_path_dep/.gitignore

@@ -0,0 +1 @@
+Cargo.lock