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 binaryen URL and use updated binary-install to fix installation on macOS (adding to @matheus23's PR) #1188

Merged
merged 20 commits into from
Jan 15, 2023

Conversation

printfn
Copy link
Contributor

@printfn printfn commented Oct 30, 2022

This includes all the commits from #1163, uses the updated binary-install crate (rustwasm/binary-install#21), switches from failure to anyhow to match what binary-install uses, and fixes wasm-opt installation on macOS.

Close #1163
Close #1175
Close #1176


Make sure these boxes are checked! 📦✅

  • You have the latest version of rustfmt installed
$ rustup component add rustfmt
  • You ran cargo fmt on the code base before submitting
  • You reference which issue is being closed in the PR text

✨✨ 😄 Thanks so much for contributing to wasm-pack! 😄 ✨✨

@printfn printfn changed the title Fix binaryen URL and use updated binary-install to fix installation on macOS (adding to @matheus23's PR) [WIP] Fix binaryen URL and use updated binary-install to fix installation on macOS (adding to @matheus23's PR) Oct 31, 2022
@printfn printfn changed the title [WIP] Fix binaryen URL and use updated binary-install to fix installation on macOS (adding to @matheus23's PR) Fix binaryen URL and use updated binary-install to fix installation on macOS (adding to @matheus23's PR) Oct 31, 2022
@printfn
Copy link
Contributor Author

printfn commented Nov 5, 2022

Hi @drager, apologies for the ping. Do you think you might potentially be able to take a look at this PR? It’s gotten a bit large unfortunately due to the error handling changes, but I believe it should work now. Thanks!

Copy link

@joshuataylor joshuataylor left a comment

Choose a reason for hiding this comment

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

This is great, I'll do more testing on x86 linux and macos arm64.

It no longer errors for me though :-).

src/bindgen.rs Show resolved Hide resolved
src/install/mod.rs Outdated Show resolved Hide resolved
@joshuataylor
Copy link

I've been testing this on both MacOS Monterey ARM64 and Linux AMD64 and it works great! No issues so far. 👍

Copy link
Member

@drager drager left a comment

Choose a reason for hiding this comment

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

I think this looks good. Thanks a lot for all the work and effort!

Cargo.toml Outdated Show resolved Hide resolved
@printfn
Copy link
Contributor Author

printfn commented Jan 14, 2023

Updated to use the new version of binary-install :)

@drager drager merged commit bbc539e into rustwasm:master Jan 15, 2023
@drager drager mentioned this pull request Jan 15, 2023
@printfn printfn deleted the fix-binaryen branch January 16, 2023 04:55
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.

4 participants