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

Add ability to run on Apple silicon #25

Merged
merged 3 commits into from
Dec 14, 2021
Merged

Conversation

pauldorehill
Copy link
Contributor

Hi,

This allows the use of the plugin on macOS with Apple silicon and addresses #24. While the issue has nothing to do with this package (the cause is the wasm-pack npm package will throw on macOS unless arch === "x64"), it provides a work around where the user can use their local installation of wasm-pack.

One thing to also note is the following exception that throws Error: no prebuilt wasm-opt binaries are available for this platform: Unrecognized target!. However this can also be fixed by adding the following in the Cargo.toml:

[package.metadata.wasm-pack.profile.release]
wasm-opt = false

[package.metadata.wasm-pack.profile.dev]
wasm-opt = false

@pauldorehill pauldorehill changed the title Add abilty to run on Apple silicon Add ability to run on Apple silicon Dec 7, 2021
@Pauan
Copy link
Collaborator

Pauan commented Dec 7, 2021

I would rather provide an option to specify a custom path to wasm-pack, instead of trying to auto-detect things.

Also, note that optionalDependencies does not really make the dependency optional, it will still be installed regardless.

@pauldorehill
Copy link
Contributor Author

I agree that would be better 😄 - I'll take a look at how to do it.

This is an easy fix in the wasm-pack package... however to get that updated is probably unlikely.

The reason for moving it to an optional dependency is that npm install will fail for all packages and not install anything on Apple silicon: the wasm-pack code runs a postinstall script that throws. Making it optional allows all the other packages to get installed.

@pauldorehill
Copy link
Contributor Author

Ok.. so I've updated to add a wasmPackPath option.

I actually realized it that it will run ok so long as wasm-pack is installed globally... so if you wanted to keep it simple all that needs to be changed for it to work is the optionalDependancy?

Copy link
Collaborator

@Pauan Pauan left a comment

Choose a reason for hiding this comment

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

Looking a lot better, just a few minor things to fix.

example/Cargo.toml Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@pauldorehill
Copy link
Contributor Author

ok should be addressed.

@Pauan Pauan merged commit c3e0676 into wasm-tool:master Dec 14, 2021
@Pauan
Copy link
Collaborator

Pauan commented Dec 14, 2021

Thanks!

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.

2 participants