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 an environment variable EMSCRIPTEN_ROOT for emscripten installed with brew and add supports for other wasm targets. #276

Closed

Conversation

TheVeryDarkness
Copy link
Contributor

Homebrew only provides emscripten but not a full emsdk, so brew users may need an extra environment variable EMSCRIPTEN_ROOT.

In the past versions, emscripten is considered only when the target is wasm32-unknown-emscripten, but it may be needed for other wasm targets. I add supports for other wasm32 targets. And missing emscripten won't cause a panic for non *-emscripten targets.

And we can test it on wasm32 targets if vcpkg-rs supports them later. Related PR has been merged but has not been published to crates.io.

Use vcpkg-rs to manage z3 instead. A non-default feature vcpkg is added.

However, vcpkg-rs does not support wasm32 target currently. I created a pull request there at mcgoo/vcpkg-rs#53
Make emscripten visible to wasm32-unknown-unknown if emscripten is installed.
Z3_HEADER_VAR will disappear when the feature vcpkg is enabled.
Use is_err instead of pattern matching.
Added missing cargo:rerun-if-changed for header.
So that rust can continue to build.
By this, we can check whether the toolchain matches the vcpkg triplet.
Emscripten can be missing if it's building for wasm32-unknown-unknown.
Copy link
Contributor

@waywardmonkeys waywardmonkeys left a comment

Choose a reason for hiding this comment

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

Would it be possible to rebase this against current master and eliminate a bunch of the commits so that I can see what a clean history might look like?

@waywardmonkeys
Copy link
Contributor

Since this is only used for getting a sysroot for running clang to generate the enum bindings, perhaps we can solve this in an even simpler way... Could we just copy a couple of files into an z3-sys/ext/wasm-sysroot to give a minimal stdbool.h, stdint.h and stdio.h?

Actually, I think I can submit a PR to upstream to remove in the include of stdiio.h from here...

@TheVeryDarkness
Copy link
Contributor Author

Would it be possible to rebase this against current master and eliminate a bunch of the commits so that I can see what a clean history might look like?

Oh, I'm sorry. Let me take a glance at git rebase.

@TheVeryDarkness
Copy link
Contributor Author

Since this is only used for getting a sysroot for running clang to generate the enum bindings, perhaps we can solve this in an even simpler way... Could we just copy a couple of files into an z3-sys/ext/wasm-sysroot to give a minimal stdbool.h, stdint.h and stdio.h?

Actually, I think I can submit a PR to upstream to remove in the include of stdiio.h from here...

Yep, I think those ways should be better.

@waywardmonkeys
Copy link
Contributor

I'm going to close this and will do the wasm-sysroot changes this weekend. Thanks for bringing this up!

(Also, I got a change merged upstream already to remove the inclusion of stdio.h)

@TheVeryDarkness
Copy link
Contributor Author

I'm going to close this and will do the wasm-sysroot changes this weekend. Thanks for bringing this up!

(Also, I got a change merged upstream already to remove the inclusion of stdio.h)

Sounds great :)

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