Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generated source distribution does not handle aliased dependencies correctly #1781

Closed
1 of 2 tasks
0xbe7a opened this issue Sep 28, 2023 · 5 comments · Fixed by #1794
Closed
1 of 2 tasks

Generated source distribution does not handle aliased dependencies correctly #1781

0xbe7a opened this issue Sep 28, 2023 · 5 comments · Fixed by #1794
Labels
bug Something isn't working sdist Source distribution

Comments

@0xbe7a
Copy link

0xbe7a commented Sep 28, 2023

Bug Description

maturin sdist does not handle "aliased" dependencies of workspaces correctly:

I have a package py_adder which is the only member of a workspace (Link to a repository containing the full minimal reproducible example is at the end).
The workspaces Cargo.toml looks like this:

[workspace]
members = [
  "py-adder"
]

[workspace.dependencies.backend]
package = "adder_backend"
version = "0.1.0"
path = "adder_backend"

adder_backend is just a minimal library containing a single function add.

adder_backends Cargo.toml looks like this:

[package]
name = "adder_backend"
version = "0.1.0"
edition = "2021"

My package py-adder uses this backend like this:

[dependencies]
backend = { workspace = true }

When running maturin build or maturin develop inside the py-adder folder everything works as expected and the functions can be used by python.

We can also build the source distribution by running maturin sdist inside the py-adder folder.

However, when trying to run maturin build inside the extracted source distribution folder we get:

error: no matching package named `backend` found
location searched: /private/tmp/source_code/adder-0.1.0/local_dependencies/backend
required by package `adder v0.1.0 (/private/tmp/source_code/adder-0.1.0)`
💥 maturin failed
  Caused by: Cargo metadata failed. Does your crate compile with `cargo build`?
  Caused by: `cargo metadata` exited with an error:

Which is expected, as the Cargo.toml inside folder local_dependencies/backend declares a package called adder_backend and we are looking for backend.

We recently hit this bug in polars pola-rs/polars#11371.

I compiled a repository containing a minimal reproducible example: https://github.com/0xbe7a/maturin-dependeny-rename-mre

Your maturin version (maturin --version)

maturin 1.2.3

Your Python version (python -V)

Python 3.11.5

Your pip version (pip -V)

pip 23.2.1

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

  1. Clone https://github.com/0xbe7a/maturin-dependeny-rename-mre
  2. cd py-adder
  3. Run maturin develop and observe that everything works as expected
  4. Run maturin sdist
  5. Extract the source distribution into a new folder
  6. Run maturin build inside the new folder containing the extracted source
@0xbe7a 0xbe7a added the bug Something isn't working label Sep 28, 2023
@0xbe7a 0xbe7a changed the title Maturin sdist does not handle aliased dependencies correctly Generated source distribution does not handle aliased dependencies correctly Sep 28, 2023
@messense
Copy link
Member

Thanks for the report, can you try matruin main branch to see whether #1741 has fixed it already?

@messense messense added the sdist Source distribution label Sep 29, 2023
@0xbe7a
Copy link
Author

0xbe7a commented Sep 29, 2023

Using maturin from main also produces a non-compileable source distribution.

➜  adder-0.1.0 tree
.
├── PKG-INFO
├── adder_backend
│   ├── Cargo.toml
│   └── src
│       └── lib.rs
├── py-adder
│   ├── Cargo.lock
│   ├── Cargo.toml
│   └── src
│       └── lib.rs
└── pyproject.toml

5 directories, 7 files
➜  adder-0.1.0 cat py-adder/Cargo.toml 
[package]
name = "adder"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[lib]
name = "adder"
# "cdylib" is necessary to produce a shared library for Python to import from.
crate-type = ["cdylib"]

[dependencies]
backend = { workspace = true }

[dependencies.pyo3]
version = "0.18.0"
features = ["abi3-py37"]
➜  adder-0.1.0 cat adder_backend/Cargo.toml 
[package]
name = "adder_backend"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
➜  adder-0.1.0 maturin build
error: failed to parse manifest at `/private/tmp/source_code/adder-0.1.0/py-adder/Cargo.toml`

Caused by:
  error inheriting `backend` from workspace root manifest's `workspace.dependencies.backend`

Caused by:
  failed to find a workspace root
💥 maturin failed
  Caused by: Cargo metadata failed. Does your crate compile with `cargo build`?
  Caused by: `cargo metadata` exited with an error: 
➜  adder-0.1.0

@messense
Copy link
Member

messense commented Sep 29, 2023

For your repro, if you remove py-adder/Cargo.lock it should work? I'm not sure why/how you created that file in the first place, the canonical lock file for the workspace should be the top level Cargo.lock.

@0xbe7a
Copy link
Author

0xbe7a commented Sep 29, 2023

In polars the equivalent of py-adder was not part of the workspace, however while minimizing the example i realized that this is not required to trigger the behaviour.

Here is the commit where i moved py-adder into the workspace 0xbe7a/maturin-dependeny-rename-mre@bcd5b48

When i remove py-adders invalid Cargo.lock the latest maturin from main works perfectly 😄, 1.2.3 is still broken !

Would you mind releasing the new sdist implementation?

@0xbe7a 0xbe7a closed this as completed Sep 29, 2023
@0xbe7a 0xbe7a reopened this Sep 29, 2023
@messense
Copy link
Member

Would you mind releasing the new sdist implementation?

I'll probably release a new version next week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working sdist Source distribution
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants