wasm-pack icon indicating copy to clipboard operation
wasm-pack copied to clipboard

Pass through --weak-refs --reference-types flags to bindgen

Open serprex opened this issue 5 years ago • 14 comments

Fixes #930 Closes #888

I'm wondering if it'd make sense to have a --bind-gen-opts flag which allows the user to manually specify args to pass through

serprex avatar Nov 19 '20 18:11 serprex

@serprex Thank you for putting this together. It appears that a patch is also required to wasm-opt?

I've been successfully building my project using:

cargo build --target wasm32-unknown-unknown
wasm-bindgen ./target/wasm32-unknown-unknown/release/wasm_test.wasm --out-dir pkg --target web --reference-types

But when I attempt to build with your fork I get:

matt@matt-X570-AORUS-ELITE:~/rust/wasm_test$ ~/rust/wasm-pack/target/debug/wasm-pack build --target web --reference-types
[INFO]: Checking for the Wasm target...
[INFO]: Compiling to Wasm...
    Finished release [optimized] target(s) in 0.01s
:-) [WARN]: origin crate has no README
[INFO]: Installing wasm-bindgen...
[INFO]: Optimizing wasm binaries with `wasm-opt`...
[parse exception: Only 1 table definition allowed in MVP (at 0:467)]
Fatal: error in parsing input
Error: failed to execute `wasm-opt`: exited with exit code: 1
  full command: "/home/matt/.cache/.wasm-pack/wasm-opt-4d7a65327e9363b7/wasm-opt" "/home/matt/rust/wasm_test/pkg/wasm_test_bg.wasm" "-o" "/home/matt/rust/wasm_test/pkg/wasm_test_bg.wasm-opt.wasm" "-O"
To disable `wasm-opt`, add `wasm-opt = false` to your package metadata in your `Cargo.toml`.

mattcollier avatar Nov 29 '20 02:11 mattcollier

Yes, I've disabled wasm-opt in my project because it raises errors about mutable global state not being allowed. I added reference-types since that exists in wasm bindgen, but am only actually using --weak-refs

serprex avatar Nov 29 '20 02:11 serprex

@serprex https://stackoverflow.com/a/64507864

lights0123 avatar Nov 29 '20 02:11 lights0123

Any updates on this. This could be really useful.

felipellrocha avatar Jan 11 '22 16:01 felipellrocha

Can this one be merged? It could be really helpful.

ZhaoXiangXML avatar Jun 21 '22 03:06 ZhaoXiangXML

We've moved from running wasm-pack to running wasm-bindgen and wasm-opt directly and just hand-coding the package.json and LICENSE file, etc. because we're missing the --weak-refs feature in wasm-pack.

I used this opportunity to test out why wasm-opt was failing, as @mattcollier noticed above. This PR would simply need a change to pass --enable-reference-types to wasm-opt in order for it to run through.

Perhaps that's the change needed in this PR to get this merging? @serprex are you interested in adjusting this PR with my suggested change? And @ the maintainers of wasm-pack: Would you merge this PR once it works with wasm-opt enabled?

matheus23 avatar Jul 28 '22 18:07 matheus23

@matheus23 updated. Unfortunately like you I've moved to bypassing wasm-pack in my build process a long time ago. Are you able to test this change is effective?

serprex avatar Jul 29 '22 11:07 serprex

@serprex Yes, confirming that this works :+1:

matheus23 avatar Jul 29 '22 13:07 matheus23

Any updates on this PR? Running into this issue as well and would love to be able to pass in --weak-refs to wasm-bindgen

RaminKav avatar Aug 05 '22 15:08 RaminKav

Yes, I'd love to see this merged & released as well! Is there some remaining work to be done?

The tests seem to be failing due to this:

thread 'build::build_different_profiles' panicked at 'called Result::unwrap() on an Err value: ErrorMessage { msg: "received a bad HTTP status code (404) when requesting https://github.com/WebAssembly/binaryen/releases/download/version_108/binaryen-version_108-x86-linux.tar.gz" }

But that error happens with all recent PRs.

Is there something else To-Do? Do we need to bump the version/write docs? Anything we can do to help this get pushed over the finish line?

matheus23 avatar Aug 08 '22 09:08 matheus23

Yes, I'd love to see this merged & released as well! Is there some remaining work to be done?

The tests seem to be failing due to this:

thread 'build::build_different_profiles' panicked at 'called Result::unwrap() on an Err value: ErrorMessage { msg: "received a bad HTTP status code (404) when requesting https://github.com/WebAssembly/binaryen/releases/download/version_108/binaryen-version_108-x86-linux.tar.gz" }

But that error happens with all recent PRs.

Is there something else To-Do? Do we need to bump the version/write docs? Anything we can do to help this get pushed over the finish line?

I think the tests just need to be fixed in order to create a new release with this and the other recently merged PRs. If someone would be interesting in debugging and make the tests pass it would be really nice! I don't got much time to debug it right now unfortunately... Another solution would be to revert the PR that caused the failing tests...

drager avatar Aug 11 '22 10:08 drager

Another solution would be to revert the PR that caused the failing tests...

I actually think there wasn't a PR that caused the failing tests, it was just the URLs for the binaryen stuff changing under our feet:

Before/after: https://github.com/WebAssembly/binaryen/releases/download/version_108/binaryen-version_108-x86-linux.tar.gz https://github.com/WebAssembly/binaryen/releases/download/version_108/binaryen-version_108-x86_64-linux.tar.gz

You can see that the top URL fails to load (it's the one from the failing test) and the bottom one loads successfully.

I can see if I can create a PR.

matheus23 avatar Aug 11 '22 18:08 matheus23

Any chance this can be merged soon? Kinda a waste to have to just bypass wasm-pack if features aren't available...

dtzxporter avatar Sep 11 '22 13:09 dtzxporter

For those coming across this issue, you can also set WASM_BINDGEN_WEAKREF=1 and WASM_BINDGEN_EXTERNREF=1 environment flags and they will be passed down to wasm-bindgen

bouk avatar Sep 22 '22 09:09 bouk