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
Allow downloading static library for platforms where one is available #161
Comments
Reusing the official binaries is a clever idea. It sounds possible and I can't think of any reason it couldn't work in common cases. The main complication I can think of are related to conditional compilation of the LLVM passes - sometimes we compile without them, and in that case couldn't use the pre-built binaries. Thanks for the idea. |
Heh no worries. It would certainly help us, as for now we have to ask users to install correct wasm-opt binaries for their platform as we don't want to add expensive C++ compilation to our build, but with this change we could just use wasm-opt-rs and have it do that for us on all platforms we care about. |
perhaps provide a new feature |
It's not really the same though as automated download of the correct static library by Cargo / build.rs itself, as it still puts the onus for cross-platform shenanigans on the user. |
indeed, haha sry, I'm about to open an issue about it but saw this issue, so I just commented without reading the content in detail |
As the docs mention, building Binaryen from source takes a long time to build, prohibitively so in some projects.
On the other hand, Binaryen already provides a static library inside the
lib
directory of its official prebuilt releases.Assuming it contains all the same APIs used by this crate, would it be possible to add an opt-in feature that would download & link those static libraries in build.rs instead of building everything from source at least on supported platforms? Given that both source code and prebuilt releases come from the same Github it should be no more dangerous in terms of trusting 3rd-party code than what's happening implicitly right now anyway.
The text was updated successfully, but these errors were encountered: