Add docs and actionable error message for failed wasm-opt executions

[EverlastingBugstopper]

In light of our upcoming release and the reported bug regarding wasm-opt on Linux, this PR updates the docs and includes a helpful error message on failed wasm-opt executions.

Output looks like this:

$ wasm-pack build --release
[INFO]: 🎯  Checking for the Wasm target...
[INFO]: 🌀  Compiling to Wasm...
    Finished release [optimized] target(s) in 0.02s
[INFO]: Optimizing wasm binaries with `wasm-opt`...
Error: failed to execute `wasm-opt`: exited with exit code: 1
  full command: "/Users/averyharnish/Library/Caches/.wasm-pack/wasm-opt-86de3fed0e357d00/wasm-opt" "/Users/averyharnish/Documents/work/_tmp/test-wasm/pkg/test_wasm_bg.wasm" "-X" "/Users/averyharnish/Documents/work/_tmp/test-wasm/pkg/test_wasm_bg.wasm-opt.wasm" "-O"
To disable `wasm-opt`, add `wasm-opt = false` to your package metadata in your `Cargo.toml`.

[EverlastingBugstopper]

@torch2424 if you have time could you see if this branch lets you build w/o changes to your config?

[torch2424]

Yep! It works! This is half for my future reference and for you but:

I went ahead and re-enabled wasm-opt in the Cargo.toml of the project:

Tried to build with the latest wasm-pack master, and it fails:

Then I cloned down your fork (I think it's your fork? Let me know if it isn't), switched to your branch, and I just got the warning and I was able to build 😄

[EverlastingBugstopper]

Awesome! Thanks for checking that out for me :)

[EverlastingBugstopper]

this pr changes the expected behavior so i changed this failure to a success. i've talked with @ashleygwilliams about this change and we think it makes sense to build and warn if wasm-opt fails since it's not a necessary build step. That being said, I could see how you might want to make it fail instead of warn if it was custom arguments that failed and not the default wasm-opt enablement that failed.

cc @alexcrichton - this is related to #625

[EverlastingBugstopper]

I could see this being a problem for people who have wasm-pack in their CI and wasm-opt enabled in their Cargo.toml - since this no longer fails it is easier to ignore if wasm-opt fails which could lead to people accidentally publishing a release that did not run wasm-opt without giving them time to debug why wasm-opt failed.

[ashleygwilliams]

@Pauan @fitzgen @alexcrichton do you have thoughts on this being a breaking change? and/or what behavior you'd want to see?

[Pauan]

I'm not really comfortable with this change. Right now, wasm-opt is only run in release builds, and the point of release builds is to be as optimized as possible.

So if wasm-opt fails, then the file size and performance will be degraded. That's something important to notice and fix. And warnings can get lost easily and overlooked, leading to accidental performance regressions.

My solution to this problem is: if there's a bug causing wasm-opt to error, I'd rather fix that bug. And if users don't want to run wasm-opt at all, I'm fine with providing a flag or option to toggle that.

But I think the default should be to run wasm-opt (in release mode), and hard error if something goes wrong.

[EverlastingBugstopper]

I've updated this PR to include docs changes and a (hopefully) more helpful error message. Happy to change the wording of either

[ashleygwilliams]

cc @Pauan - ultimately i think you are right- do you think the error message and docs help?

[ashleygwilliams]

mergin