-
Notifications
You must be signed in to change notification settings - Fork 150
Expose compiler constraint values in public API #562
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
base: main
Are you sure you want to change the base?
Expose compiler constraint values in public API #562
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
This change addresses issue bazelbuild#330 by making the CC compiler constraint setting and constraint values publicly accessible in //cc/compiler. Previously, users who needed to define platforms constrained by compiler type had to reference @rules_cc//cc/private/toolchain:clang-cl (and similar), which: 1. Exposes private implementation details 2. Behaves inconsistently between Bazel 7 and 8 with bzlmod 3. Is not documented as the canonical location Changes: - Add constraint_setting "cc_compiler" to //cc/compiler - Add constraint_value targets (*_constraint) for each compiler type - Convert cc/private/toolchain constraints to aliases (backward compat) - Update BUILD.windows.tpl to use the new public paths Users can now use the public API: platform( name = "windows_clang_cl", constraint_values = [ "@platforms//os:windows", "@rules_cc//cc/compiler:clang-cl_constraint", ], ) Fixes bazelbuild#330
2297d8a to
259b448
Compare
|
I know that the Windows toolchain has been using this pattern for a while, but I'm not sure whether platform constraints are the best way to choose compilers vs. a Starlark flag. Have you considered that approach? |
|
Thanks for raising this - it's a good architectural question. I considered Starlark flags, but platform constraints seem more appropriate here for a few reasons:
That said, Starlark flags could work for use cases like "I want to switch compilers on the same platform" (e.g., clang vs gcc on Linux). Would a hybrid approach make sense - constraints for toolchain resolution, with an optional flag to override? Happy to discuss further or adjust the approach if there's consensus on a different direction. |
|
The situation is unfortunately more complex than that, partially for historical reasons:
2./3. The Windows toolchain has used constraints on the exec platform. You are suggesting that it's commonly a constraint on the target platform. I would argue it's neither since whether you can use a given compiler often depends more on how you write your code than on where it is supposed to run. Runtime libraries such as the choice of libc would clearly map to target platform constraints, but for compilers, this seems less clear. We probably need to collect more concrete use cases and examples of build configuration to distill put the right abstractions. There is a lot of value in that if we get it right. |
|
Good points - I appreciate the nuanced breakdown. You're right that I conflated exec/target platform constraints, and that required_settings does support Starlark flags for toolchain resolution. I'd be happy to help collect use cases. From my experience, the common scenarios are:
Should I hold this PR while we work through the design? Is there an existing issue or design doc tracking this, or should we create one? |
|
I think that the complexity of this topic deserves a prior discussion, which could be a short design doc or a GitHub discussion. Another important aspect is the distinction between hermetic toolchains and non-hermetic toolchains, where the latter may only be available on certain exec platforms (think RBE runners with a commercial compiler that has to be preinstalled). |
|
Created a design discussion to explore this properly: #564 Marking this PR as draft until we have consensus on the approach. |
Summary
constraint_settingandconstraint_valuetargets to//cc/compilercc/private/toolchainconstraints to aliases for backward compatibilityBUILD.windows.tplto use new public pathsProblem
Users who need to define platforms constrained by compiler type currently have to reference
@rules_cc//cc/private/toolchain:clang-cl(and similar), which:Solution
Users can now use the public API:
Test plan
bazel build //cc/compiler:all //cc/private/toolchain:allsucceedsFixes #330