-
Notifications
You must be signed in to change notification settings - Fork 503
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 aarch64-pc-windows-gnullvm
and x86_64-pc-windows-gnullvm
targets
#1883
Conversation
I don't know what is wrong but git keeps interpreting |
That's odd, the Line 7 in 4ba369a
|
Any general pointers/tips on building your toolchain locally for testing? I'm going through the steps of compiling |
You could try first if unstable
It requires nightly Rust though. If you want full toolchain I'll provide you the exact steps on how to build it today/tomorrow evening CEST. |
Ah cool, I didn't realize this made it into nightly. 👍 No full toolchain steps needed for our purposes at this time, although it might be handy to add to the docs at some point. Compiling Full output:
|
Is there no 32-bit target for gnullvm? |
ARM and i686 might be added in the future, right now I skipped them because Rust breaks their exception handling (the "glue" code is very specific to libgcc and doesn't work with LLVM's libunwind). 64-bit targets use SEH and are free of this issue. |
@riverar could you try using CLANG64 subsystem instead of MINGW64? |
@mati865 Yep, that works great. I'll add a review comment to the README then. |
Data point: My x86_64/aarch64 |
Ran I can squash commits manually and force-push once you are satisfied with this PR. |
Not to worry, we always "squash and merge" when completing. |
Looking pretty good! @mati865 Can you update our rudimentary cross-compile CI tests here to test this new target? https://github.com/microsoft/windows-rs/blob/master/.github/workflows/cross-compile.yml One possible matrix strategy could look like: matrix:
image: [macos-latest, ubuntu-latest]
version: [stable, nightly]
target: [x86_64-pc-windows-gnu, xxxxxx_gnullvm]
exclude:
- target: xxxxxx_gnullvm
version: stable If you don't feel comfortable with that, need help, etc. just let us know. |
@riverar linking the executable will fail without manually installing mingw-w64+LLVM toolchain like this one: https://github.com/mstorsjo/llvm-mingw |
Yup, add a step for it, with a conditional for gnullvm. |
🎉 Passing the smoke tests, sweet! I don't think this is specific to your gnullvm target but I'm wondering if the LLVM/mingw-w64 toolchain can be tweaked to leverage Hybrid CRT semantics to eliminate the dynamic CRT external dependency. I don't think this is a blocker for adoption but that's not my call. Also, can you help me understand the differences between mstorsjo/llvm-mingw and me installing |
I don't think so, mingw-w64 doesn't support linking static CRT at all because it doesn't have access to Microsoft's source codes nor static libraries. So I'd expect this Hybrid thing to have the same problem.
MSYS2 (the tool that provides MSYS2's So TL;DR |
Thanks for your efforts on this - its looks good. @riverar is away for a few days but should be able to help you get this completed and signed off as soon as he returns. |
Looks good to me, just a nit on the caching that should be easy to resolve. Thanks for your patience while I was out! |
Thanks, I'm out until Sunday/Monday. I'll address feedback then. |
a45856e
to
81a68e3
Compare
Opened #1949 to address build failures here. |
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.
Rebase once more (fix went in to master), should build cleanly now.
dc88f47
to
4f214d8
Compare
@riverar should be ready. |
@mati865 🥳🎉 |
Creating new target as told to do so in #1846 (review)
Fixes #1881