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

Fix build on wasm32-unknown-unknown #3367

Closed
wants to merge 2 commits into from

Conversation

FreezyLemon
Copy link
Contributor

  • crate-type = "cdylib" is needed for wasm-pack to work
  • There is no need for a wasm feature, everything can be auto-detected (-F wasm is now a no-op)
  • wasm-bindgen should only be included for wasm32-unknown-unknown. wasm32-wasi does not need it. I'm not sure about emscripten

wasm-pack build and other commands should now work. --no-default-features is necessary because the binaries feature does not compile on wasm.

Note that testing on wasm32-unknown-unknown can't just be done with cargo test and requires more work. I'll do that in a separate PR (incl. CI).

`wasm-pack build --no-default-features -F wasm`
`wasm-pack build --no-default-features`

The wasm feature still exists for compatibility but is a no-op.
@FreezyLemon
Copy link
Contributor Author

This seems to break fuzzing somehow. I'll take a look and try to fix it

@@ -167,6 +173,8 @@ bench = false

[lib]
bench = false
# cdylib is required for wasm32-unknown-unknown
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is potentially annoying, rustwasm/wasm-pack#1297 should fix it.

Copy link
Collaborator

@lu-zero lu-zero left a comment

Choose a reason for hiding this comment

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

I'd wait for rustwasm/wasm-pack#1297 to get addressed and/or check if the unreleased wasm-pack doesn't address it otherwise.

I do not consider acceptable to add cdylib unconditionally.

@FreezyLemon
Copy link
Contributor Author

I see. wasm-pack's master branch doesn't seem to include any changes that would help work around this problem. Not much to do except wait.

I'll close this. When this is fixed in wasm-pack, it should be easy to reimplement.

@FreezyLemon FreezyLemon deleted the make-wasm32-build branch March 14, 2024 12:32
@lu-zero
Copy link
Collaborator

lu-zero commented Mar 14, 2024

https://crates.io/crates/trunk might be an alternative.

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

2 participants