maturin fails if a local dependency is not released on crates.io

[ritchie46]

Bug Description

I am not entirely certain, but I believe it is related to a subdependency not being published to crates.io.

If I comment out this line and try to build a source distribution with maturin it fails.

The dependency is not required for the python bindings and is not yet released on crates.io.

This only happens if we try to build a source distro.

Your Python version (python -V)

Python 3.9.12

Your pip version (pip -V)

pip 21.2.4

What bindings you're using

pyo3

Does cargo build work?

• Yes, it works

If on windows, have you checked that you aren't accidentally using unix path (those with the forward slash /)?

• Yes

Steps to Reproduce

Comment out this this line on: pola-rs/polars@c8cfc99

and then run: $ maturin build --sdist.

📦 Including license file "LICENSE"
🍹 Building a mixed python/rust project
🔗 Found pyo3 bindings with abi3 support for Python ≥ 3.7
🐍 Not using a specific python interpreter
💥 maturin failed
  Caused by: Failed to build source distribution
  Caused by: Failed to add local dependency polars at /home/ritchie46/code/polars/polars/Cargo.toml to the source distribution
  Caused by: cargo metadata does not know about the path for dependencies.polars-algo present in /home/ritchie46/code/polars/polars/Cargo.toml, which should never happen ಠ_ಠ

My maturin version is 0.13.2. Sorry that I don't have an MWE other than an existing project. A bit hard to publish crates for this.

[messense]

What's the output of cargo metadata --format-version 1?

maturin relies on cargo metadata to find out the paths to all its dependencies, my guess is that comment out https://github.com/pola-rs/polars/blob/c8cfc9957de9f56a3c7a9a25752cb09ab80c687d/py-polars/Cargo.toml#L87 somehow "removes" polars-algo from the output of cargo metadata.

[ritchie46]

This is the failing one:
meta.txt

This is the successful one:
meta_success.txt

I did find indeed less matches to the "polars-algo" query in the files. Note that we don't want to include it (we don't activate that dependency). So it could be ignored if not resolved in the metadata.

Edit: Ah, I see you are already further than I am. :) #1084

[messense]

Note that we don't want to include it (we don't activate that dependency). So it could be ignored if not resolved in the metadata.

That does not work: #1084 (comment) , cargo requires all of its path dependencies to exist I think.

[messense]

Please try v0.13.3-beta.2 when it's out.

[Stranger6667]

Hey, I observed the same issue in jsonschema with the newest maturin (0.14.10). This PR contains changes to reproduce this behavior:

⇒  maturin build --sdist  
🍹 Building a mixed python/rust project
🔗 Found pyo3 bindings
🐍 Found CPython 3.8 at /.../jsonschema/bin/python3
💥 maturin failed
  Caused by: Failed to build source distribution
  Caused by: Failed to add local dependency json_schema_test_suite at /.../jsonschema/jsonschema-test-suite/Cargo.toml to the source distribution
  Caused by: cargo metadata does not know about the path for dependencies.json_schema_test_suite_proc_macro present in /.../jsonschema/jsonschema-test-suite/Cargo.toml, which should never happen ಠ_ಠ

It seems like the same problem, but if it is not, let me know if I need to create a new issue.

[messense]

@Stranger6667 I’ll take a look tomorrow.

[messense]

The problem is that json_schema_test_suite_proc_macro and json_schema_test_suite_test_case isn't present in cargo metadata --format-version 1 of bindings/python, thus find_path_deps didn't find them, it's a bit tricky.

maturin/src/source_distribution.rs

Lines 470 to 499 in d7e8ba5

	fn find_path_deps(cargo_metadata: &Metadata) -> Result<HashMap<String, PathBuf>> {
	let root = cargo_metadata
	.root_package()
	.context("Expected the dependency graph to have a root package")?;
	// scan the dependency graph for path dependencies
	let mut path_deps = HashMap::new();
	let mut stack: Vec<&cargo_metadata::Package> = vec![root];
	while let Some(top) = stack.pop() {
	for dependency in &top.dependencies {
	if let Some(path) = &dependency.path {
	// we search for the respective package by `manifest_path`, there seems
	// to be no way to query the dependency graph given `dependency`
	let dep_manifest_path = path.join("Cargo.toml");
	path_deps.insert(
	dependency.name.clone(),
	PathBuf::from(dep_manifest_path.clone()),
	);
	if let Some(dep_package) = cargo_metadata
	.packages
	.iter()
	.find(|package| package.manifest_path == dep_manifest_path)
	{
	// scan the dependencies of the path dependency
	stack.push(dep_package)
	}
	}
	}
	}
	Ok(path_deps)
	}

[messense]

@Stranger6667 IMO the easiest way to fix it is to only depends on jsonschema from crates.io instead of local path. Path dependencies are complicated when doing sdist because maturin has to package the main package and all of it's path dependencies. maturin relies on cargo metadata to find them, now that cargo metadata didn't report them it's broken.

[messense]

That said I wonder if it's possible to skip dev-dependencies entirely when building sdist, that'd avoid this issue since json_schema_test_suite is a dev-only dependencies.

Local test shows that it works for jsonschema_rs, opened #1435.

[Stranger6667]

@messense

Thank you for the detailed explanation! Newer maturin version works :)

[kylebarron]

I hit this same bug using maturin 1.2.0.

Repro:

git clone https://github.com/kylebarron/geoarrow-rs
cd geoarrow-rs/python/geoarrow-rust/
git checkout d815cf5ce4c15a23473fda26c5b23b133fd31ba9
pip install 'maturin==1.2.0'
maturin build --sdist

gives

> maturin build --sdist
🍹 Building a mixed python/rust project
🔗 Found pyo3 bindings with abi3 support for Python ≥ 3.8
🐍 Not using a specific python interpreter
📡 Using build options features from pyproject.toml
💥 maturin failed
  Caused by: Failed to build source distribution
  Caused by: cargo metadata does not know about the path for dependencies.geoarrow present in /Users/kyle/tmp/geoarrow-rs/python/geoarrow-rust/Cargo.toml, which should never happen ಠ_ಠ

Should I create a new issue for this?

In this case the issue is I suppose slightly different, as geoarrow is indeed on crates.io, but published as geoarrow2 and referenced in Cargo.toml as geoarrow

[messense]

@kylebarron Feel free to create a new issue, just remember to include the cargo metadata output in it.