-
Notifications
You must be signed in to change notification settings - Fork 479
Fix build scripts targeting the wrong architecture #1564
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
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.
Thanks for the PR! Is this specific to a particular toolchain you're using? It'd be great to add a test for this (as I can see breaking it in the future), but I don't think we currently have any tests using a custom toolchain... We do have an example which builds an iOS app on macos on CI, which is currently passing, though...
I've added a small check for this to the examples/ios folder - if the first commit is reverted, the test will fail. Note it only affects build scripts that compile C(PP) code, such as blake3/zstd. |
Thanks! Sorry, I meant an end-to-end example, rather than a targeted unit test - could we e.g. add a |
I've spent about 3 hours trying to get an end-to-end example based on crates_universe working, but don't understand why the library is not available to the sh_test(). Any help would be appreciated. |
9d6afd4
to
f572e7b
Compare
Ok, finally figured it out - had to use a filegroup due to bazelbuild/bazel#4519. |
PR bazelbuild#1081 started passing CFLAGS to build scripts, which cargo does too. But unfortunately Bazel's cpp toolchain adds an explicit target to the CFLAGS it provides, and that target uses the host platform instead of the target platform. When building a crate like blake3 or zstd on a Mac with iOS as the target, this ends up in the crate being built for macOS instead of iOS.
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.
Thanks!
This PR breaks toolchains that set CFLAGS and do expect it to work. For example, I have a zig toolchain set up, which I'm using for cross compilation. I need to set the target when creating the |
@duarten Sorry for the breakage! Do you have a test-case you could supply to repro the issue? When to forward what things from the toolchain is a bit of an awkward dance (which is why @dae added a test-case for the iOS case) - it'd be great to get better coverage here to help us work out exactly when we should/shouldn't be forwarding what - particularly, that would help us to highlight an iOS toolchain bug. |
I pushed a repro here. To repro, run It contains a rust program that's using ring. Ring attempts to compile a file in arm assembly, which will immediately cause a build failure. I'm using zig for cross compilation, and I need the |
It looks like the reported issue is probably related to the ios toolchain in use, rather than our general toolchain flag propagation.
It looks like the reported issue is probably related to the ios toolchain in use, rather than our general toolchain flag propagation.
It looks like the reported issue is probably related to the ios toolchain in use, rather than our general toolchain flag propagation.
Thanks @illicitonion! :) |
It looks like the reported issue is probably related to the ios toolchain in use, rather than our general toolchain flag propagation.
* Add example workspace cross-compiling with zig We've run into a number of issues, both in rustc and in cargo build scripts, where we're not properly propagating cc_toolchain information to compiles and particularly link actions. These have often been hard to test because we don't have an example set-up with a non-default C++ toolchain. By wiring up a zig cross-compiling toolchain, we have a concrete example we can work with. It's also an interesting potential example for folks who want to set something similar up. * Revert #1564 It looks like the reported issue is probably related to the ios toolchain in use, rather than our general toolchain flag propagation. * Add platform_mappings and --cpu flag to ios_build example
PR #1081 started passing CFLAGS to build scripts, which cargo does too. But unfortunately Bazel's cpp toolchain adds an explicit target to the CFLAGS it provides, and that target uses the host platform instead of the target platform. When building a crate like zstd on a Mac with iOS as the target, this ends up in the crate being built for macOS instead of iOS.