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

Remove Cargo requirement from prepare_metadata_for_build_wheel? #563

Open
uranusjr opened this issue Jun 7, 2021 · 3 comments
Open

Remove Cargo requirement from prepare_metadata_for_build_wheel? #563

uranusjr opened this issue Jun 7, 2021 · 3 comments
Labels
sdist Source distribution

Comments

@uranusjr
Copy link

uranusjr commented Jun 7, 2021

Currently prepare_metadata_for_build_wheel would check Cargo’s existence before generating metadata, and persuambly also needs Cargo during the process, but I’m unsure why that is the case—Admittedly I did not read the source and maybe there is a valid reason. But from my naive understanding, the Python package’s metadata can be retrieved entirely by parsing pyproject.toml and Cargo.toml manually, and should not require Cargo (since anything that’s too complicated to parse without Cargo are Rust-specific from what I can tell). What am I missing here?

Removing the dependency on Cargo during metadata-generation would be beneficial for tools that perform dependency resolution (locking) and package installation separately, like Pipenv and Poetry. Currently a lock file must be generated on a platform with compatible wheels published, or have Cargo installed. This means that if a project does not publish (say) wheels for Mac, a user must install Cargo for the locking phase, even if they never intend to build the project locally (since the installation happens elsewhere, e.g. on the server or in a Docker container running Linux).

@konstin
Copy link
Member

konstin commented Jun 7, 2021

should not require Cargo (since anything that’s too complicated to parse without Cargo are Rust-specific from what I can tell). What am I missing here?

maturin's prepare_metadata_for_build_wheel needs to call cargo metadata to figure out which method is used for generating bindings, so it can determine if we need cffi or not. cargo's logic for specifying dependencies has a lot of features and lots of complexity that is definitively to difficult to parse. But more importantly, unlike a custom implementation cargo metadata is forward compatible to any future cargo releases.

Removing the dependency on Cargo during metadata-generation would be beneficial for tools that perform dependency resolution (locking) and package installation separately, like Pipenv and Poetry. Currently a lock file must be generated on a platform with compatible wheels published, or have Cargo installed. This means that if a project does not publish (say) wheels for Mac, a user must install Cargo for the locking phase, even if they never intend to build the project locally (since the installation happens elsewhere, e.g. on the server or in a Docker container running Linux).

Do you have any example of this? We provide templates how to ship wheels for windows/linux/mac with github actions, so I'd hope just shipping wheels for the three major platforms should be a none-issue.

maturin already writes complete source distribution metadata, even though currently tagged as Metadata-Version 2.1. Since PEP 643 is accepted, I've opened #564 to bump the version. Tools performing resolution should then just read PKG-INFO instead of setting up and running PEP 517 hooks.

@uranusjr
Copy link
Author

uranusjr commented Jun 7, 2021

maturin's prepare_metadata_for_build_wheel needs to call cargo metadata to figure out which method is used for generating bindings, so it can determine if we need cffi or not. cargo's logic for specifying dependencies has a lot of features and lots of complexity that is definitively to difficult to parse. But more importantly, unlike a custom implementation cargo metadata is forward compatible to any future cargo releases.

I see, thanks for the explaination. My interpretation to the issue here is that cffi needs to be dynamically injected as a run-time dependency, so Cargo is needed for that. Is this correct?

Do you have any example of this? We provide templates how to ship wheels for windows/linux/mac with github actions, so I'd hope just shipping wheels for the three major platforms should be a none-issue.

A friend of mine asked about this privately, the machine has an obscure dev setup (non-manylinux-compatible, I think). It is definitely an edge case. If Cargo is too beneficial to drop, it is definitely reasonable to ask users to install it instead. Fortunately that’s not a big hurdle either, thanks to rustup.

@uranusjr
Copy link
Author

uranusjr commented Jun 11, 2021

One thought: Would it be possible to run cargo metadata when an sdist is generated, and only require Cargo in prepare_metadata_for_build_wheel when that information isn’t previously calculated? This would solve most of the resolve-time metadata retrieval issue. If that’s possible, the main issue would be how we can detect whether a pre-generated cargo metadata output is out-of-date (for example, if a user extract an sdist, modify the code, and try to install that extracted tree without deleting the pre-generated cache), but maybe that can be considered an unsupported operation instead.

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

Successfully merging a pull request may close this issue.

3 participants