-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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 conlyopts and cxxopts attributes to cc rules #23792
Add conlyopts and cxxopts attributes to cc rules #23792
Conversation
The inability to pass C or C++ specific compiler flags to targets that contain a mix of those sources is a common sticking point for new users. These mirror the global `--conlyopt` and `--cxxopt` flags but at the target level. Fixes bazelbuild#22041
Thank you! I'm not familiar enough with Bazel to review this, but this would solve a ton of problems with Bazel that prevent BoringSSL from using it effectively. We support many build systems, and issue alone, due to all the problems it causes down the line, is probably singlehandedly the reason why it is the most expensive build system for us to support right now. E.g. we'd also be able to enable |
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.
LGTM from Blaze perspective.
I want LGTM also from @trybka, before merging.
@bazel-io fork 7.4.0 |
LGTM. I am extremely supportive. |
@keith: could you add a RELNOTES line to the PR description? I'll attach it during import. |
done! |
FTR, this is breaking Google-internal tests because some other code is calling Could you perhaps make the new param optional? |
made it optional |
The inability to pass C or C++ specific compiler flags to targets that
contain a mix of those sources is a common sticking point for new users.
These mirror the global
--conlyopt
and--cxxopt
flags but at thetarget level.
Fixes #22041
RELNOTES: Add conlyopts and cxxopts attributes to cc rules