-
Notifications
You must be signed in to change notification settings - Fork 195
Make rustworkx build and run with pyiodide
#1447
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
Conversation
Pull Request Test Coverage Report for Build 16921538859Details
💛 - Coveralls |
|
So I tried following: https://pyodide.org/en/stable/usage/building-and-testing-packages.html I had to install I got this error: https://pastebin.com/TaT892sU. So yeah, I'd like to keep this build outside of our repository but at least add the code such that when |
|
@sorin-bolos how did you setup the SDK to build the version from your PR? I might need some help here |
|
Maybe I should try using the version pinned here: https://github.com/pyodide/pyodide-build/blob/7126c5f1bd56ed7f40ce29cbcbe588267a42434c/pyodide_build/config.py#L240C24-L240C42 |
|
Somehow I got |
|
Well, I can |
|
I think setting verbose on the previous comment erased the flags from |
|
Ok, we definetely build. Any code that calls Rayon fails, but:
It works! The issues inside the |
|
After telling the nightly compiler to rebuild I will try to see if I figure out how to run our unit tests to confirm things work. But this is shockingly good. |
.cargo/config.toml
Outdated
| ] | ||
|
|
||
| [target.wasm32-unknown-emscripten] | ||
| rustflags = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: it seems like pyodide-build overrides these, so I will open an issue to talk to them and check for alternatives. In the meantime, this is helpful for reference and when we compile outside pyodide-build
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It also is useful for rustworkx-core and using rustworkx without python in wasm.
rustworkx build and run with pyiodiderustworkx build and run with pyiodide
|
I have no clue how to include this on CI or even if we should, but there more I test the more things seem to work. I will open an issue on the |
mtreinish
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall this looks fine to me, Just one nit inline. I'd like to try to find time soon to pull this locally and test out wasm and pyodide before approving though just to validate all the new options.
.cargo/config.toml
Outdated
| ] | ||
|
|
||
| [target.wasm32-unknown-emscripten] | ||
| rustflags = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It also is useful for rustworkx-core and using rustworkx without python in wasm.
Some minor notes about Rust WASM. There are three Rust targets! We could support more, but here are the caveats:
After #1450 lands, I think we could test |
mtreinish
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was able to compile a wasm module using this, there are definitely some rough edges still but it's a starting point and explicitly experimental so I think that's fine. I just had one question inline
.cargo/config.toml
Outdated
| "-C", "target-feature=+atomics,+bulk-memory,+mutable-globals", | ||
| "-C", "link-arg=-sSIDE_MODULE=2", | ||
| "-C", "link-arg=-sWASM_BIGINT", | ||
| "-Z", "emscripten-wasm-eh", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a nightly option, is it needed, or do we only support building wasm on nightly rust? I was able to build with the latest stable if I exclude this option
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They use nightly for pyodide-build, see https://pyodide.org/en/stable/development/abi.html#id2
I think pyodide will be stuck with nightly for a while. That’s why I am so reticent to say we support it. I pinned the nightly version in the Pixi CL exactly for that reason
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well I guess my concern here is that having this in the .cargo/config.toml with a -Z flag prevents us from building rustworkx-core against the emscripten target without pyodide. My preference would be to remove it from the default config and inject the flag when it's needed during a pyodide build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will update the PR. Pixi should always get those flags anyway, we use the pyodide-build CLI to retrieve the flags

Closes #703
This adds support for Pyodide. We list Pyodie in a brand new tier that is Tier Experimental. Currently, we build with
pyoide-build==0.30.2using the following flags:This requires a Rust with
nightly-2025-02-01toolchain andemsdk 4.0.8. I tested with Pyodide 0.28.1a which maps to Python 3.13. Because we depend on so much unstable stuff, this is not ready yet for our CI. I need to figure out a way of adding it.Related: pyodide/pyodide-recipes#90