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

Install rust toolchain on building source distribution #2120

Open
padix-key opened this issue Jun 25, 2024 · 8 comments
Open

Install rust toolchain on building source distribution #2120

padix-key opened this issue Jun 25, 2024 · 8 comments
Labels
enhancement New feature or request help wanted Extra attention is needed sdist Source distribution

Comments

@padix-key
Copy link

Currently, maturin requires the rust toolchain (cargo, rustc) already to be installed. This is no problem in a development scenario as Rust developers presumably have Rust installed.

However, when a user installs the source distribution of a package built with maturin, the user also requires the Rust toolchain, although they may be only interested in the package, but not in Rust. Rust is also not a part (yet) of the major OS distributions, so that user would need to go the extra mile of running rustup first.

Therefore, I propose installing the Rust toolchain in an isolated environment (e.g. similar to https://github.com/chriskuehl/rustenv) when maturin is used as [build-system], at least in case the rust toolchain is not installed, yet.

@padix-key padix-key added the enhancement New feature or request label Jun 25, 2024
@messense
Copy link
Member

I don't think this should be done in maturin, when using source distribution, users should be aware of the build system requirement.

Even though gcc/clang is part of the major OS distributions, when using sdist, it's still possible that they are missing from the OS environment of the end user, in such case should setuptools or other build backends install gcc/clang for the users? Absolutely not, IMO.

@messense messense closed this as not planned Won't fix, can't repro, duplicate, stale Jun 25, 2024
@konstin
Copy link
Member

konstin commented Jun 25, 2024

Personally, i think a source distribution should install all build dependencies it needs; it should not be the responsibility of a python user to understand the native build system of their transitive dependencies only because they are let's say on an arm platform without wheels. We'd need a way to bootstrap rust and cargo for this though that doesn't change the users environment (like rustup does). It would be ideal to have wheels for cargo and rustc on pypi like we have with zig. Relying on (linux) distros is often insufficient, they require root access to install new packages that is not available in many environments and only ship specific versions of tools. It would honestly be amazing if we could get gcc and clang from pypi too.

@messense
Copy link
Member

It sounds amazing but surely there will be people start complain that using sdist download lots of huge dependencies.

IMO if it's really desirable, perhaps it should just be another dependency in [build-system] table that gets installed by a build frontend, instead of installing by maturin?

@konstin
Copy link
Member

konstin commented Jun 25, 2024

We can check for rust and cargo in get_requires_for_build_wheel and only if it is not installed add a dependency on the pypi package(s). For me, maturin should handle as much as possible end-to-end without the developer specifying something additionally.

@messense
Copy link
Member

We can check for rust and cargo in get_requires_for_build_wheel and only if it is not installed add a dependency on the pypi package(s).

I'm fine with this solution if it works, but consider when a sdist requires a specific
version of nightly Rust toolchain that wasn't not installed, it seems impractical to have every single nightly Rust toolchain on pypi.

@konstin
Copy link
Member

konstin commented Jun 25, 2024

Totally, this would be limited to stable rust

@messense messense reopened this Jun 26, 2024
@messense messense added help wanted Extra attention is needed sdist Source distribution labels Jun 26, 2024
@padix-key
Copy link
Author

Thanks for the discussion. For me both options, an explicit extra dependency in build-system or an implicit installation in an environment by maturin would work.

@Owen-CH-Leung
Copy link
Contributor

I'll try to file a PR to address this in the coming few days

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed sdist Source distribution
Projects
None yet
Development

No branches or pull requests

4 participants