-
Notifications
You must be signed in to change notification settings - Fork 821
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 support for cross-compiling in create-exe with zig cc WIP #3076
Conversation
zig version must be at least 0.10.0-dev.3431+4a4f3c50c Closes #3042
lib/cli/src/commands/create_exe.rs
Outdated
wasmer_types::Vendor::Apple => !name.contains("windows"), | ||
wasmer_types::Vendor::Pc => !name.contains("apple"), | ||
_ => true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ummm... this seems somewhat incorrect?
What are we computing here? It's a bit confusing for me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is confusing yeah. Apple vendor does not necessarily mean that the given triple includes the vendor name, because zig
has its own triple format and it doesn't include the vendor (for example x86_64-macos-none
is a valid zig triple but in llvm/clang it's x86_64-apple-darwin
). So the checks here are reversed because they can only single out artifacts, not find them all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I mean is... what is the true/false value signaling for? I still have no idea. It's true that is Linux?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the vendor is Apple, it should definitely not include substring "windows". If the vendor is Pc, it should definitely not include "apple". In any other case just retain it anyways.
zig version must be at least 0.10.0-dev.3431+4a4f3c50c Closes #3042
Check that `zig` binary version is at least `0.10.0`
…cross-comp # Conflicts: # lib/cli/src/commands/create_exe.rs
* Add support for cross-compiling in create-exe with zig cc WIP zig version must be at least 0.10.0-dev.3431+4a4f3c50c Closes #3042 * Add support for cross-compiling in create-exe with zig cc WIP zig version must be at least 0.10.0-dev.3431+4a4f3c50c Closes #3042 * Add SSE2 features to the CPU * Add SSE2 features to the CPU * create_exe: locate zig binary and check minimum version Check that `zig` binary version is at least `0.10.0` * create-exe: refactor cross-comp cli parsing and error checks * create-exe: add -lunwind for cross-comp with zig * Set the proper library for windows * create-exe: add path exists check for --tarball value * create-exe: add -msvc environment in triple_to_zig_triple() Co-authored-by: Syrus Akbary <me@syrusakbary.com>
zig version must be at least 0.10.0-dev.3431+4a4f3c50c
Closes #3042
Description
Review