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

Allow wasm to compile #939

Merged
merged 13 commits into from
Jul 18, 2022
Merged

Allow wasm to compile #939

merged 13 commits into from
Jul 18, 2022

Conversation

rylev
Copy link
Contributor

@rylev rylev commented Jul 15, 2022

Fixes #906

This cleans up our wasm support a bit and paves the way for actual wasm support to be added in the near future.

User experience

Users can now compiler for wasm32 targets as long as the reqwest features are not turned on (since reqwest does not have wasm support).

If the user compiles for wasm32 targets with the reqwest features enabled (as they are by default), they see the following error.

$ cargo c --target=wasm32-unknown-unknown -p azure_data_cosmos 

Checking azure_core v0.3.0 (/home/rylevick/code/azure-sdk/azure-sdk-for-rust/sdk/core)
error: The `enable_request` and `enable_reqwest_rustls` features are not allowed for `wasm32` targets`
  --> sdk/core/src/http_client/mod.rs:11:1
   |
11 | compile_error!("The `enable_request` and `enable_reqwest_rustls` features are not allowed for `wasm32` targets");
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: could not compile `azure_core` due to previous error

The noop client

We currently don't support any way for users to make requests when compiling for the wasm32 target. In the future, we will provide an easy way for users to supply there own transport policies. Users who compile for wasm in the browser, can provide a transport policy that uses the browser APIs, while users compiling for wasi, can use a transport policy that uses wasi based APIs.

Since there is (IMO) no obvious default, we have a NoopClient that simply panics and prints out the outgoing request. Most users who will be using the reqwest feature, will never run into this.

Some challenges

This solution isn't 100% elegant, but is IMO an improvement over the status quo. There are some downsides though:

  • We do enforce Send futures for wasm targets. This causes us to have to propogate this out since by default BoxFuture and the Futures created by the async_trait macro are all constrained to be Send. So, we still have a lot of cfg markers for wasm, but there in much more predictable places and we don't have to add a bunch of allow(unused) directives everywhere.

@rylev rylev requested review from bmc-msft and cataggar July 15, 2022 15:53
@rylev rylev requested a review from yoshuawuyts July 18, 2022 12:07
@rylev
Copy link
Contributor Author

rylev commented Jul 18, 2022

@ctaggart @bmc-msft this should be ready to merge!

Copy link
Contributor

@bmc-msft bmc-msft left a comment

Choose a reason for hiding this comment

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

In all of our uses, azure_core is set to default-features=false. Perhaps we should not set default features for azure_core, and have it's dependencies do that?

❯ rg -I ^azure_core -g '**/Cargo.toml' | sort -u
azure_core = { path = "../../../sdk/core", version = "0.3", default-features = false }
azure_core = { path = "../core", version = "0.3", default-features = false }
azure_core = { path = "../core", version = "0.3", default-features = false}
azure_core = { path = "../core", version = "0.3", default-features=false }
azure_core = { path = "../core", version = "0.3", default_features = false }
❯

@cataggar cataggar merged commit 51fd3dd into Azure:main Jul 18, 2022
@rylev rylev deleted the allow-wasm branch July 18, 2022 16:09
@radu-matei
Copy link

For WASI support, I've been using a modified version of this SDK that works with https://github.com/deislabs/wasi-experimental-http and Wasmtime — main...radu-matei:azure-sdk-for-rust:enable-wasi-experimental-http

@rylev
Copy link
Contributor Author

rylev commented Jul 18, 2022

@radu-matei awesome! I hope to have a change soon that allows for easily swapping out the transport policy. Once that's done, it should hopefully be easy to have a wasi backed transport policy that can be swapped in without the need for modifying any of the code.

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.

Build fails => cargo build --target wasm32-wasi --release
4 participants