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

Move ensure_rust_toolchain to buildall #83

Merged
merged 6 commits into from
Jan 15, 2025

Conversation

hoodmane
Copy link
Member

Rather than installing the rust toolchain in each build step that uses rust, do it once in buildall.

Work split off from #81.

Rather than installing the rust toolchain in each build step that uses rust,
do it once in buildall
[integration]
@@ -19,8 +19,7 @@
from pyodide_build.recipe import load_all_recipes

RUST_BUILD_PRELUDE = """
rustup toolchain install ${RUST_TOOLCHAIN} && rustup default ${RUST_TOOLCHAIN}
rustup target add wasm32-unknown-emscripten --toolchain ${RUST_TOOLCHAIN}
rustup default ${RUST_TOOLCHAIN}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose we could also do this in _ensure_rust_toolchain() and remove RUST_BUILD_PRELUDE entirely.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then , will pyodide build-recipes-no-deps work?

Copy link
Member

@ryanking13 ryanking13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Mostly looks good to me. But please check it works correctly with pyodide build-recipes-no-deps CLI.

Also you can add [integration] in the commit message to run integration test, which includes building cryptography package.

@@ -19,8 +19,7 @@
from pyodide_build.recipe import load_all_recipes

RUST_BUILD_PRELUDE = """
rustup toolchain install ${RUST_TOOLCHAIN} && rustup default ${RUST_TOOLCHAIN}
rustup target add wasm32-unknown-emscripten --toolchain ${RUST_TOOLCHAIN}
rustup default ${RUST_TOOLCHAIN}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then , will pyodide build-recipes-no-deps work?

@hoodmane
Copy link
Member Author

hoodmane commented Jan 15, 2025

Thanks for the review @ryanking13! Will check for the no-deps entrypoint. Presumably it can run the same code.

@hoodmane hoodmane merged commit 7ebf9a2 into pyodide:main Jan 15, 2025
5 checks passed
@hoodmane hoodmane deleted the ensure-rust-toolchain branch January 15, 2025 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants