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 compile errors when built with unsupported wasm features #1353

Merged
merged 5 commits into from
Oct 1, 2024

Conversation

leighmcculloch
Copy link
Member

@leighmcculloch leighmcculloch commented Sep 29, 2024

What

Add compile errors when built with unsupported wasm features reference-types and multivalue.

Why

Rust 1.82 is likely to ship with target feature reference-types and multivalue enabled on wasm builds. These target features are not supported by the Soroban Env in the current protocol 21 or next planned protocol 22. It's not trivial for developers to disable target features because of how the rustc compiler only exposes the ability to buildstd with nightly.

These compile errors will prevent someone from building .wasm files with the sdk when either of those target features are enabled.

A Rust version check is not being done because it's actually easier to do a target feature check, and in the off chain somehow Rust 1.82 shipped without the target features enabled then the sdk could still be used with 1.82.

Links:

Releasing

This change will be merged to main targeting v22, then should be backported to a v21 patch release immediately following.

@leighmcculloch leighmcculloch marked this pull request as ready for review September 29, 2024 22:40
@leighmcculloch
Copy link
Member Author

leighmcculloch commented Sep 29, 2024

I tested this on 1.82.0-beta.5 (6a3b69c6b 2024-09-27), and found the compile errors did not trigger, it seems like reference-types and multivalue are not actually enabled on 1.82-beta, even though the Rust Foundation's blog post says they are: https://blog.rust-lang.org/2024/09/24/webassembly-targets-change-in-default-target-features.html.

However, when I tested with 1.83.0-nightly (ed04567ba 2024-09-28) I do see the compiler error as expected.

Before we merge this it would be ideal to confirm why these features are not enabled on 1.82-beta.

I've started a coversation in Zulip to find out more:

@leighmcculloch
Copy link
Member Author

Got feedback on https://rust-lang.zulipchat.com/#narrow/stream/122651-general/topic/wasm32.20reference-types.20.2F.20multivalue.20in.201.2E82-beta.20not.20enabled/near/473794770 that indicated that the cfg guards for target features won't work on beta because they're still marked as unstable. But the fact they work on nightly is a good signal that the change is good to go.

We can merge this and release it, and backport it to the previous release too.

@leighmcculloch leighmcculloch added this pull request to the merge queue Oct 1, 2024
Merged via the queue into main with commit 6c45fb9 Oct 1, 2024
16 checks passed
@leighmcculloch leighmcculloch deleted the rust182prep branch October 1, 2024 01:07
leighmcculloch added a commit that referenced this pull request Oct 1, 2024
### What
Add compile errors when built with unsupported wasm features
reference-types and multivalue.

### Why
Rust 1.82 is likely to ship with target feature reference-types and
multivalue enabled on wasm builds. These target features are not
supported by the Soroban Env in the current protocol 21 or next planned
protocol 22. It's not trivial for developers to disable target features
because of how the rustc compiler only exposes the ability to buildstd
with nightly.

These compile errors will prevent someone from building .wasm files with
the sdk when either of those target features are enabled.

A Rust version check is not being done because it's actually easier to
do a target feature check, and in the off chain somehow Rust 1.82
shipped without the target features enabled then the sdk could still be
used with 1.82.

Links:
- https://discord.com/channels/897514728459468821/1289090985569026048
- stellar/rs-soroban-env#1469
- WebAssembly/tool-conventions#233

### Releasing
This change will be merged to `main` targeting v22, then should be
backported to a v21 patch release immediately following.

(cherry picked from commit 6c45fb9)
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.

2 participants