-
Notifications
You must be signed in to change notification settings - Fork 58
Bump rustworkx to 0.17.1 [full build] #316
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
|
I need to investigate the compilation failure, my apologies |
|
No worries, please let us know when you are stuck. We also have some Rust-related information in our docs. Most of them should be automatically handled inside pyodide-build (as we already build the alpha version of rustworkx), but it might be helpful in some cases. |
Package Build ResultsTotal packages built: 28 Package Build Times (click to expand)
Longest build: rustworkx (3m 19s) |
|
There was nothing wrong with the library code per se, but right now A small change to |
| build: | ||
| script: export RUSTFLAGS="$RUSTFLAGS -C target-feature=+atomics,+bulk-memory,+mutable-globals" | ||
| script: | | ||
| rustup component add rust-src |
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.
@hoodmane WDYT about this? I am not sure if it is a good idea for individual package to run this command, as it will make global effect (AFAIK).
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.
If we could move it to the original rustup setup, it would be best.
I also think rust-src has no effect? Unless users pass the stable flag to recompile std
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.
@hoodmane Do you think that rustup component add rust-src can overwrite the custom stdlib that we built with wasm-exception enabled?
Rust build toolchain is always confusing to me...
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 is okay. It would also be fine to add it as part of our normal setup. But that is a pyodide-build change, so I think we can merge this as-is (if it otherwise looks good) and move it into pyodide-build later.
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.
Oh okay. The rustup component add rust-src downloads the source, not the compiled binary... so it should be unrelated to it. Thanks for the response.
ryanking13
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.
Looks good. Sorry for the delayed review!
Follow up to #90
rustworkx now supports Pyodide in a released version! Previously, it used an alpha version from my personal repository.
Thanks again for providing
pyodide-buildand all the recipes!