search

bytecodealliance / cargo-wasi

A lightweight Cargo subcommand to build Rust code for the `wasm32-wasi` target
https://bytecodealliance.github.io/cargo-wasi/
Apache License 2.0
445 stars 29 forks source link

Get wasm-opt working on MacOS #119

Closed jeffcharles closed 2 years ago

jeffcharles commented 2 years ago

Fixes #112 and adds supports downloading wasm-opt when running on Apple Silicon.

This also introduces a ToolPath struct for representing the various paths involved with wasm-opt. I tried doing this without introducing a new type but the code is harder to follow IMO. There's the path to the binary, the base path, the subpaths, and whether the binary path is an overridden path that need to all be represented and used correctly.

I also debated whether to adjust the get_wasm_bindgen function to return that ToolPath type. I opted against it because it doesn't add any value beyond consistency with get_wasm_opt's signature. I would be okay with changing get_wasm_bindgen in a similar manner if that's preferred.

I also considered trying to keep the code adjustments more local to install_wasm_opt but couldn't figure out a clean way to deal with the path being returned sometimes being a path to the binary and sometimes being a path to the cache directory without introducing something similar to ToolPath.

jeffcharles commented 2 years ago

I'm not sure what's going on with the publish step failing

alexcrichton commented 2 years ago

Looks good to me, thanks! I'll try to poke at the CI failure but I think it's unrelated to this PR.

jeffcharles commented 2 years ago

👋 would it be possible to publish a release that includes this fix? 🙂

alexcrichton commented 2 years ago

Sure yeah, that's at https://github.com/bytecodealliance/cargo-wasi/pull/121