Implement new sdist feature to download rust toolchain #2177

Owen-CH-Leung · 2024-08-08T12:36:11Z

This PR implements feature to download rust toolchain when trying to pip install a python package built by maturin. Specifically, when maturin execute WriteDistInfo, when compiling with the rustls feature, it will try to check if toolchain is installed, and proceed to install the rustup installer and download the stable toolchain if toolchain is absent

Initial draft to implement downloading rust toolchain during sdist

netlify · 2024-08-08T12:36:26Z

✅ Deploy Preview for maturin-guide ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`908bec0`
🔍 Latest deploy log	https://app.netlify.com/sites/maturin-guide/deploys/66bb2365438ee80008aef3dc
😎 Deploy Preview	https://deploy-preview-2177--maturin-guide.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Owen-CH-Leung · 2024-08-09T01:27:55Z

@messense Can you review this PR ? I'm not sure if I'm heading to the right direction so I'd like to get some feedback before proceeding to polish it

messense · 2024-08-09T01:30:49Z

src/build_context.rs

@@ -1132,6 +1148,19 @@ impl BuildContext {
        }
        Ok(wheels)
    }
+    /// Check if user requires to install rust toolchain
+    ///
+    /// Loop over `build-system.requires` defined in pyproject.toml and see if `rust-toolchain` is provided


This isn't going to work because pip will try to install rust-toolchain package from pypi.

Since we don't have rust-toolchain on pypi yet, we can try to use rust-toolchain.toml / legacy rust-toolchain file or rust-version in Cargo.toml of the binding crate, then fallback to Rust stable.

@messense Thanks. So it means like if user provides a rust-toolchain.toml file in the project directory, that would be indicating that user wants to install rust-toolchain ?

To put my two cents in, I think that the rust toolchain should be present for the build, even if rust-toolchain.tomlis missing in which case a default toolchain would be used. Bonus points if [toolchain] can be specified directly in the pyproject.toml as e.g [tool.maturin.toolchain].

how about an additional command line flag such as maturin sdist --install-rust-toolchain ? (Or maybe when env variable MATURIN_INSTALL_RUST is set to true? ). This will be backward-compatible while keeping maturin flexible for users .

This would only be suitable in the case the user of the package knows what Maturin, Rust etc. is. However, the reason I brought up issue #2120, is that a typical user just wants to use a package and would probably do not like to think about the build backend of this package.

In other words, it would be nice in my opinion, if a user can install a source distribution of a package that uses maturin as build-system simply via pip install without knowing such internals

@messense @padix-key I've modified prepare_metadata_for_build_wheel to remove the check for rust-toolchain, and add logic in maturin pep517 write-dist-info command to check for toolchain from there ( and install from rustup if not present). Let me know what you think

I have actually barely insight into the maturin internals, I only know the user side 😃.

I think automatically downloading an installation script and running it for the user and installing to non-standard locations is undesirable. I would not be happy if this took place on my system without any confirmation at least. I could also see it causing problems for CI systems which may accidentally install rustup each build if misconfigured.
An opt-in command line flag or environment variable seems like a good approach to me. If not passed it could print

rust toolchain (cargo) not found. Please install it manually or pass `maturin sdist --auto-install-toolchain`

so discoverability wouldn't be an issue for the 'typical user' described as not being aware of what they need to have installed

messense · 2024-08-09T01:31:17Z

src/upload.rs

@@ -382,7 +382,7 @@ fn http_agent() -> Result<ureq::Agent, UploadError> {

 #[cfg(feature = "rustls")]
 #[allow(clippy::result_large_err)]
-fn http_agent() -> Result<ureq::Agent, UploadError> {
+pub fn http_agent() -> Result<ureq::Agent, UploadError> {


there is a http_agent below that also need to be made public.

messense · 2024-08-09T01:33:29Z

src/build_context.rs

+                        .to_str()
+                        .context("Fail to construct target path for installing rust toolchain")?;
+
+                    download_and_execute_rustup(target_path_str, target_path_str)


Skip download if there is already a working rust toolchain (probably need to validate version with rust-version in Cargo.toml and Cargo.toml of every crate dependencies, I think we can get that from cargo metadata output).

messense · 2024-08-09T01:36:45Z

cc @padix-key since you requested this feature.

for more information, see https://pre-commit.ci

…st_toolchain' into impl_sdist_feature_to_install_rust_toolchain

Initial draft to implement downloading rust toolchain during sdist

for more information, see https://pre-commit.ci

…//github.com/Owen-CH-Leung/maturin into impl_sdist_feature_to_install_rust_toolchain

konstin · 2024-10-20T17:23:02Z

(sorry for the further delay, i was on this but got thrown off again by #2268)

… to test pip install without toolchain

Owen-CH-Leung · 2024-10-21T15:32:05Z

I was trying to create a CI test to verify that pip install will work even without toolchain but no luck :(

mbway · 2024-10-21T21:33:49Z

Why are rustup and cargo being installed to non-standard locations? the code seems to be using home_dir() as CARGO_HOME. On unix the default is $HOME/.cargo (source).

padix-key · 2024-10-23T15:09:27Z

Would it be sensible to install cargo and rustup in a temporary location and clean it after finishing the build, or do you think the size of them would be too large for repeated install/uninstall?

Initial draft to implement downloading rust toolchain during sdist

57f35c9

Initial draft to implement downloading rust toolchain during sdist

messense reviewed Aug 9, 2024

View reviewed changes

messense requested a review from konstin August 9, 2024 01:36

Owen-CH-Leung and others added 12 commits August 13, 2024 09:08

Enhance pep517 write-dist-info command to check for rust toolchain

1ec7982

[pre-commit.ci] auto fixes from pre-commit.com hooks

6db3fdf

for more information, see https://pre-commit.ci

Fix window command

2a721a7

Merge remote-tracking branch 'origin/impl_sdist_feature_to_install_ru…

7b76520

…st_toolchain' into impl_sdist_feature_to_install_rust_toolchain

Fix failing cargo fmt and ruff CI jobs

136fee2

Fix failing codespell CI jobs

908bec0

Initial draft to implement downloading rust toolchain during sdist

c6c7d72

Initial draft to implement downloading rust toolchain during sdist

Enhance pep517 write-dist-info command to check for rust toolchain

d45b609

Fix window command

978677c

[pre-commit.ci] auto fixes from pre-commit.com hooks

620c8f3

for more information, see https://pre-commit.ci

Fix failing cargo fmt and ruff CI jobs

22f5885

Fix failing codespell CI jobs

e06953f

konstin force-pushed the impl_sdist_feature_to_install_rust_toolchain branch from 908bec0 to e06953f Compare October 17, 2024 12:57

Owen-CH-Leung added 3 commits October 19, 2024 16:57

Merge branch 'impl_sdist_feature_to_install_rust_toolchain' of https:…

ec03119

…//github.com/Owen-CH-Leung/maturin into impl_sdist_feature_to_install_rust_toolchain

Add CI test to test pip install without toolchain

cc62930

Merge branch 'main' into impl_sdist_feature_to_install_rust_toolchain

3c78b7c

Owen-CH-Leung added 3 commits October 21, 2024 21:37

Fix fmt & clippy CI job. Use upload & download artifact github action…

bbea38e

… to test pip install without toolchain

Fix pip install *.whl

4b67e92

Fix windows build

0be8c05

padix-key mentioned this pull request Oct 27, 2024

Introducing Rust code into Biotite biotite-dev/biotite#688

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement new sdist feature to download rust toolchain #2177

Implement new sdist feature to download rust toolchain #2177

Owen-CH-Leung commented Aug 8, 2024 •

edited

Loading

netlify bot commented Aug 8, 2024 •

edited

Loading

Owen-CH-Leung commented Aug 9, 2024

messense Aug 9, 2024

messense Aug 9, 2024

Owen-CH-Leung Aug 9, 2024 •

edited

Loading

padix-key Aug 9, 2024 •

edited

Loading

Owen-CH-Leung Aug 10, 2024

padix-key Aug 10, 2024

Owen-CH-Leung Aug 13, 2024

padix-key Aug 13, 2024

mbway Oct 21, 2024 •

edited

Loading

messense Aug 9, 2024

messense Aug 9, 2024 •

edited

Loading

messense commented Aug 9, 2024

konstin commented Oct 20, 2024

Owen-CH-Leung commented Oct 21, 2024 •

edited

Loading

mbway commented Oct 21, 2024

padix-key commented Oct 23, 2024

Implement new sdist feature to download rust toolchain #2177

Are you sure you want to change the base?

Implement new sdist feature to download rust toolchain #2177

Conversation

Owen-CH-Leung commented Aug 8, 2024 • edited Loading

netlify bot commented Aug 8, 2024 • edited Loading

✅ Deploy Preview for maturin-guide ready!

Owen-CH-Leung commented Aug 9, 2024

messense Aug 9, 2024

Choose a reason for hiding this comment

messense Aug 9, 2024

Choose a reason for hiding this comment

Owen-CH-Leung Aug 9, 2024 • edited Loading

Choose a reason for hiding this comment

padix-key Aug 9, 2024 • edited Loading

Choose a reason for hiding this comment

Owen-CH-Leung Aug 10, 2024

Choose a reason for hiding this comment

padix-key Aug 10, 2024

Choose a reason for hiding this comment

Owen-CH-Leung Aug 13, 2024

Choose a reason for hiding this comment

padix-key Aug 13, 2024

Choose a reason for hiding this comment

mbway Oct 21, 2024 • edited Loading

Choose a reason for hiding this comment

messense Aug 9, 2024

Choose a reason for hiding this comment

messense Aug 9, 2024 • edited Loading

Choose a reason for hiding this comment

messense commented Aug 9, 2024

konstin commented Oct 20, 2024

Owen-CH-Leung commented Oct 21, 2024 • edited Loading

mbway commented Oct 21, 2024

padix-key commented Oct 23, 2024

Owen-CH-Leung commented Aug 8, 2024 •

edited

Loading

netlify bot commented Aug 8, 2024 •

edited

Loading

Owen-CH-Leung Aug 9, 2024 •

edited

Loading

padix-key Aug 9, 2024 •

edited

Loading

mbway Oct 21, 2024 •

edited

Loading

messense Aug 9, 2024 •

edited

Loading

Owen-CH-Leung commented Oct 21, 2024 •

edited

Loading