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

wasm32: Remove "wasm32_c" feature and add "wasm32_unknown_unknown_js" feature. #1413

Merged
merged 2 commits into from
Oct 6, 2021

Conversation

briansmith
Copy link
Owner

See each commit message for detalis.

…_c" feature.

Always require a C compilare for wasm32, instead of trying to provide a subset
of the functionality.
…eb APIs.

Planning ahead for when WASI is to be supported, require the user to opt into
using web APIs for thw wasm32-unknown-unknown target with a new feature, since
wasm32-unknown-unknown could be used for either web or non-web environments.

Don't bother updating the tests to use this new flag, since the tests aren't
part of the "public API" of *ring*. When we add support for other WebAssembly
environments (e.g. WASI) we'll update the tests then.
@briansmith briansmith self-assigned this Oct 6, 2021
@codecov
Copy link

codecov bot commented Oct 6, 2021

Codecov Report

Merging #1413 (2a9b56b) into main (245a76a) will decrease coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1413      +/-   ##
==========================================
- Coverage   93.02%   93.02%   -0.01%     
==========================================
  Files         123      123              
  Lines       18231    18231              
  Branches      195      195              
==========================================
- Hits        16960    16959       -1     
  Misses       1237     1237              
- Partials       34       35       +1     
Impacted Files Coverage Δ
src/lib.rs 93.33% <ø> (ø)
src/rand.rs 65.43% <ø> (ø)
tests/aead_tests.rs 98.72% <ø> (ø)
tests/constant_time_tests.rs 100.00% <ø> (ø)
tests/hmac_tests.rs 97.01% <ø> (ø)
tests/pbkdf2_tests.rs 88.88% <ø> (ø)
tests/rsa_tests.rs 97.35% <ø> (ø)
src/aead/chacha20_poly1305.rs 93.08% <0.00%> (-1.89%) ⬇️
crypto/cpu-intel.c 68.85% <0.00%> (-1.64%) ⬇️
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 245a76a...2a9b56b. Read the comment docs.

@briansmith
Copy link
Owner Author

rg wasm32_c shows there is no more mention of "wasm32_c".

@briansmith briansmith merged commit 8d78cb2 into main Oct 6, 2021
@briansmith briansmith deleted the b/no-wasm-c branch October 6, 2021 22:53
@briansmith
Copy link
Owner Author

TARGET_CC=clang-11 cargo build --target=wasm32-unknown-unknown now fails with:

error[E0432]: unresolved import `super::sysrand_chunk`
   --> src/rand.rs:292:16
    |
292 |     use super::sysrand_chunk::chunk;
    |                ^^^^^^^^^^^^^ could not find `sysrand_chunk` in `super`

TARGET_CC=clang-11 cargo build --target=wasm32-unknown-unknown --features=wasm32_unknown_unknown_js succeeds. And CI/CD wasm32-test jobs succeed because they pass the feature flag.

This is the correct behavior.

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.

1 participant