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

Add feature to build for web #1921

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Add feature to build for web #1921

wants to merge 1 commit into from

Conversation

Mubelotix
Copy link

The build for wasm32-unknown-unknown with ring used to fail, but with this feature it works out of the box

@@ -34,6 +34,7 @@ logging = ["log"]
aws_lc_rs = ["dep:aws-lc-rs", "webpki/aws_lc_rs"]
aws-lc-rs = ["aws_lc_rs"] # Alias because Cargo features commonly use `-`
ring = ["dep:ring", "webpki/ring"]
web = ["ring/wasm32_unknown_unknown_js"]
Copy link
Member

@cpu cpu Apr 25, 2024

Choose a reason for hiding this comment

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

Isn't this something that could be activated outside of rustls by a dependent crate that wanted to use *ring* by taking its own direct dependency w/ the feature enabled?

If a feature for this were to be added here, should it also activate pki-types/web?

Copy link
Member

Choose a reason for hiding this comment

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

Also should this be ring?/wasm32_unknown_unknown_js to avoid enabling the ring dependency in scenarios where the aws-lc-rs provider is actually selected?

Copy link
Author

Choose a reason for hiding this comment

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

Isn't this something that could be activated outside of rustls by a dependent crate that wanted to use *ring* by taking its own direct dependency w/ the feature enabled?

If a feature for this were to be added here, should it also activate pki-types/web?

Yes indeed, they don't have the feature yet but I made a PR, let's wait to see if they merge it

Copy link
Author

Choose a reason for hiding this comment

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

Also should this be ring?/wasm32_unknown_unknown_js to avoid enabling the ring dependency in scenarios where the aws-lc-rs provider is actually selected?

I thought it would only enable ring if I wrote it that way web = ["dep:ring", "ring/wasm32_unknown_unknown_js"] but I will check

@ctz
Copy link
Member

ctz commented Apr 26, 2024

I think we are probably overdue for regularising wasm support in this crate. What I mean by that is:

  • a small example for one flavour of wasm or another (probably wasi is easiest?) and thereby document which features, providers, etc. are required -- ideally this would be a new way to build an existing example (eg, simpleclient).
  • CI job that builds & runs that example to make sure we don't regress anything in the future.

@cpu
Copy link
Member

cpu commented Apr 26, 2024

@Mubelotix Are you interested in trying to achieve the broader changeset Ctz described above?

@Mubelotix
Copy link
Author

@Mubelotix Are you interested in trying to achieve the broader changeset Ctz described above?

Sure. I only have experience with wasm in the browser though, so that's what the example would be about

@Mubelotix Mubelotix marked this pull request as draft April 28, 2024 10:19
@cpu
Copy link
Member

cpu commented Apr 29, 2024

I only have experience with wasm in the browser though, so that's what the example would be about

Personally my understanding of the general ecosystem here is lacking, but I think we have a light preference for WASI over an example fixed to the browser context. I don't know how far the two goals are from one another to gauge if it would be helpful to consider a browser example as a stepping stone towards a more complete WASI example.

@Mubelotix
Copy link
Author

I only have experience with wasm in the browser though, so that's what the example would be about

Personally my understanding of the general ecosystem here is lacking, but I think we have a light preference for WASI over an example fixed to the browser context. I don't know how far the two goals are from one another to gauge if it would be helpful to consider a browser example as a stepping stone towards a more complete WASI example.

Ok I will look into WASI

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.

None yet

4 participants