-
Notifications
You must be signed in to change notification settings - Fork 467
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
Change approach for native and cross-compilation with clang++ on FreeBSD #794
Conversation
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.
The freebsd change is mostly what I had in mind. But the location of the target==host check will change the behavior for every non-cross scenario is going to break way too much.
I'm sympathetic to your suggestion from the other thread that it's better to error than silently be subtly broken. I don't know that it applies here though — this code isn't exactly new and people have been using it successfully for a while. I think it needs a high degree of motivation to change. That is:
-
I don't see evidence that the behavior at the moment (which essentially always passes
--target
) is in error. There are cases when we definitely need to do this even when not cross-compiling (It's possible for rustc to think it's targetting the host, and the c compiler to not). -
This crate is pretty old, and is used in far more configurations than we can possibly have tests for.
Not to mention I think this will concretely break usage when called from xcodebuild, when using
cargo.target.<triple>.runner
+qemu (or similar) to run the build script, etc. Perhaps I don't particularly care about such configurations, but this just breaks them for no reason, which is not good.While I don't take take the stance that we must keep perfect 100% compatibility with every potential configuration (doing so would effectively freeze changes to
cc
)... it's better not to cause problems for no reason.
That was my original thinking in 785, happy to do the same here. Thanks for reviewing. |
Ah, apologies if I misunderstood. (I should admit I've been a bit distracted by goings on at $dayjob) |
b430952
to
aad3207
Compare
No worries. My bit about "breaking rather than being subtly wrong" was only in argument to not go with the approach in #788. I support minimizing the impact of any changes to this crate and fully agree with the points you made. I just misunderstood what your recommendation was from the other thread but I'm glad we're on the same page now. I've updated the branch to just use the We could omit |
When cross compiling, we are forced to emit the `--target=<triple>` compiler flag, but this can break clang++ C++ builds on FreeBSD if the triple isn't set to a value that causes clang++ to infer the use of libc++ instead of libstdc++ (the latter of which does not support C++11 and beyond on FreeBSD). By manually specifying the usage of `-std=libc++` we avoid the need to know which version of FreeBSD we are targeting (as we do not have that information from the rust target triple and cannot infer it from the host as we are cross-compiling). Note that we were already specifying a default stdlib value libc++ for `cpp_link_stdlib` under FreeBSD but that is not sufficient. There are other solutions that would work when not cross compiling such as omitting the `--target` altogether, but there's no compelling reason to add more code if this solution works for both native and cross-compiling cases. This fix supersedes rust-lang#788. See relevant discussions in rust-lang#785 and rust-lang#788.
aad3207
to
aadfc25
Compare
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.
Sorry for the delay here.
No worries. Thanks for merging! |
Correctly fix C++ compilation w/ clang++ on FreeBSD
(Updated)