Skip to content

Make ada-rust compile for wasm32-wasi and -unknown #23

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

Merged
merged 4 commits into from
Aug 21, 2023

Conversation

RReverser
Copy link
Contributor

@RReverser RReverser commented Aug 21, 2023

This adds ability to compile for wasm32-wasi and wasm32-unknown-unknown targets.

Both of those use WASI SDK for compilation and add necessary flags for linking C++ library. wasm32-wasi is a bit more straightforward, while wasm32-unknown-unknown requires a few hacks to get a pure Wasm output w/o any imports, but at least it can work too. Demo:

image

While changing flags in build.rs, also leveraged some new cc options and made it possible to compile on Windows via Clang instead of MSVC if user chooses to do so.

@RReverser
Copy link
Contributor Author

RReverser commented Aug 21, 2023

I see I broke Windows somehow too (which is weird as I tested it locally and it worked). I'll fix that when addressing review comments. Done.

@d3lm
Copy link
Contributor

d3lm commented Aug 21, 2023

@anonrig Thank you so much for your quick response / review! This one is really nice as we'd like to include ada-url into our existing Rust crate that is compiled to wasm32-unknown-unknown.

@anonrig
Copy link
Member

anonrig commented Aug 21, 2023

@anonrig Thank you so much for your quick response / review! This one is really nice as we'd like to include ada-url into our existing Rust crate that is compiled to wasm32-unknown-unknown.

Happy to help! You should also know that the ada repository also supports webassembly by default.

@d3lm
Copy link
Contributor

d3lm commented Aug 21, 2023

Oh yea, I have seen that but it only compiles to wasm32-unknown-emscripten AFAIK and that unfortunately can't be included into a Rust crate that is built with wasm-bindgen to wasm32-unknown-unknown. Or did I miss something?

@anonrig
Copy link
Member

anonrig commented Aug 21, 2023

Oh yea, I have seen that but it only compiles to wasm32-unknown-emscripten AFAIK and that unfortunately can't be included into a Rust crate that is built with wasm-bindgen to wasm32-unknown-unknown. Or did I miss something?

You're probably right.

@anonrig
Copy link
Member

anonrig commented Aug 21, 2023

@RReverser Would you mind also updating readme and mention that webassembly is supported?

@RReverser
Copy link
Contributor Author

Not at all. I just need to do the CI thing first - it's a bit involved.

@RReverser RReverser force-pushed the main branch 2 times, most recently from d9a5d3d to 9206d00 Compare August 21, 2023 18:45
@RReverser
Copy link
Contributor Author

CI integration for wasm32-wasi is done, can be seen running tests via Wasmtime here: https://github.com/RReverser/ada-rust-wasm/actions/runs/5930243141/job/16079561574

Would you mind also updating readme and mention that webassembly is supported?

Now that I looked, I'm actually not sure where would be appropriate place to add it. README currently doesn't mentioned supported or explicitly unsupported platforms, so I'm not sure if it's worth calling out Wasm specifically and how.

@anonrig
Copy link
Member

anonrig commented Aug 21, 2023

I've updated main with Ada v2.6.1. Can you rebase?

This adds ability to compile for wasm32-wasi and wasm32-unknown-unknown targets.

Both of those use WASI SDK for compilation and add necessary flags for linking C++ library.
WASI SDK is a bit more straightforward, while wasm32-unknown-unknown requires a few hacks
to get a pure Wasm output w/o any imports, but it works too.

While changing flags in build.rs, also leveraged some new `cc` options and made it possible
to compile on Windows via Clang instead of MSVC if user chooses to do so.
Already included in the check job.
@RReverser
Copy link
Contributor Author

Done.

@anonrig anonrig merged commit 3ac525d into ada-url:main Aug 21, 2023
@d3lm
Copy link
Contributor

d3lm commented Aug 21, 2023

Could we get a new version released of this crate?

@RReverser
Copy link
Contributor Author

Could we get a new version released of this crate?

Looks like already out: https://crates.io/crates/ada-url

@d3lm
Copy link
Contributor

d3lm commented Aug 21, 2023

Oh ups. Never mind. Thanks a ton 🙏

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.

4 participants