Populate CFLAGS and CXXFLAGS when invoking cargo build script.#1081
Merged
illicitonion merged 2 commits intobazelbuild:mainfrom Mar 10, 2022
Merged
Populate CFLAGS and CXXFLAGS when invoking cargo build script.#1081illicitonion merged 2 commits intobazelbuild:mainfrom
illicitonion merged 2 commits intobazelbuild:mainfrom
Conversation
ddeville
force-pushed
the
cargo-build-cflags
branch
3 times, most recently
from
1c7940d to
aff27ba
Compare
Collaborator
|
Thanks for putting this together! It's pretty similar to #1004 but the differences are pretty interesting, so it's probably worth lingering on them for a moment:
Either way, there's an open question for @hlopko - Do we have any prior art on wiring up a custom cc_toolchain in a unit analysis test, so that we could test that args get properly propagated from there? (cc @ikalchev - sorry we've left your PR open for so long! I don't really have a preference for which PR we end up merging, but it'd be great to come to a consensus on the open questions above :)) |
Member
|
We don't have an existing example, and creating even a fake cc toolchain is quite an endeavor (docs are here: https://docs.bazel.build/versions/main/cc-toolchain-config-reference.html, and I'd be surprised if they were up to date). I think I cannot ask anybody to try doing that without feeling like the pure evil. So I can commit to sending an PR with this fake cc toolchain next week. Hope that's quickly enough. |
ddeville
force-pushed
the
cargo-build-cflags
branch
2 times, most recently
from
27dd302 to
953cce3
Compare
Contributor
|
I can confirm that this fixes some build issues for me. Thanks! |
illicitonion
approved these changes
Feb 10, 2022
Collaborator
illicitonion
left a comment
illicitonion
left a comment
There was a problem hiding this comment.
Let's merge this - we can address the c/cpp and sysroot questions in the future as/when needed :)
Sorry for the delay here!
Downstream tools (in particular cc-rs) depend on them being set. The particular use case that I noticed was broken was the macos deployment target (specified via the --macos_minimun_os Bazel option) wasn't being applied on binaries being built via cc-rs in a build.rs script. This was causing all sort of bugs such as symbols not being weakly linked since it was using the SDK version (the default) for the deployment target and bazel-built Rust binaries wouldn't load on older OS versions due to failures to resolve symbols at runtime, even though the deployment target was correctly set in Bazel.
ddeville
force-pushed
the
cargo-build-cflags
branch
from
f513424 to
b327469
Compare
dae
added a commit
to ankitects/rules_rust
that referenced
this pull request
Sep 25, 2022
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.
dae
added a commit
to ankitects/rules_rust
that referenced
this pull request
Sep 25, 2022
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.
dae
added a commit
to ankitects/rules_rust
that referenced
this pull request
Sep 25, 2022
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.
dae
added a commit
to ankitects/rules_rust
that referenced
this pull request
Sep 25, 2022
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.
dae
added a commit
to ankitects/rules_rust
that referenced
this pull request
Oct 7, 2022
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.
illicitonion
pushed a commit
that referenced
this pull request
Oct 7, 2022
* Fix build scripts targeting the wrong architecture 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 blake3 or zstd on a Mac with iOS as the target, this ends up in the crate being built for macOS instead of iOS. * Add an end-to-end test to make sure building zstd for iOS works
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Downstream tools (in particular cc-rs) depend on them being set.
The particular use case that I noticed was broken was the macos deployment target (specified via the --macos_minimun_os Bazel option) wasn't being applied on binaries being built via cc-rs in a build.rs script. This was causing all sort of bugs such as symbols not being weakly linked since it was using the SDK version (the default) for the deployment target and bazel-built Rust binaries wouldn't load on older OS versions due to failures to resolve symbols at runtime, even though the deployment target was correctly set in Bazel.