-
Notifications
You must be signed in to change notification settings - Fork 477
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 cross-compiling for Apple platforms #1389
Conversation
31e60ea
to
c12e9cb
Compare
9c4030b
to
ab6767e
Compare
03a896b
to
c60c0c4
Compare
c60c0c4
to
bfdf3d5
Compare
# Test with clang/llvm for now, has better cross-compilation support (GCC requires downloading a different toolchain) | ||
echo "CC=clang" >> $GITHUB_ENV | ||
echo "CXX=clang++" >> $GITHUB_ENV | ||
echo "AR=llvm-ar" >> $GITHUB_ENV |
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 unclear to me why llvm-ar
is required here, it seems to not be on my local Ubuntu 24 VM? But if we don't include it, we fail with:
rust-lld: error: /home/runner/work/cc-rs/cc-rs/target/aarch64-apple-darwin/debug/build/cc-test-5875314a6fda6e38/out/libOptLinkage.a: archive has no index; run ranlib to add one
rust-lld: error: undefined symbol: _answer
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.
No idea maybe there's some apple/clang stuff default ar not supporting?
I'm not sure if this is relevant to the last fix, but trying to bump cc in rust gives us rust-lang/rust#136720 (comment)
|
7e36542
to
7854f2f
Compare
Hmm, that's what #1384 fixed. Seems like it's not failing in bootstrap, but when building |
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.
Thank you!
(So, of course we weren't done here...)
Basically, the only difference between this and pre-#1384 is that on visionOS and Mac Catalyst, we pass a versioned
--target
instead of unversioned--target
+-mtargetos=
.This is similar to the alternative I noted in that PR, and while it does run into #1278 (the bug that configuration files + versioned targets are badly supported on older Clang) on those targets, I think that's the better option.
Fixes #1388. To avoid this in the future, I have added a CI step to verify that cross-compilation works (fails in the first commit, succeeds in the second).
Alternatives
I looked through Clang's parsing code, and from that I glean that an alternative would probably be to set
--target
+*_DEPLOYMENT_TARGET
. This has a few problems though:Tool::args
/.get_compiler().get_command().get_args()
, and forget to also look atTool::env
(I know at least Rust'sbootstrap
currently forgets to do this).-mmacosx-version-min=
there) (probably acceptable to not do if we did Detect common misconfigurations on error #1391).