-
Notifications
You must be signed in to change notification settings - Fork 5.6k
Add C++ toolchain selection enforcement infrastructure with user-friendly error messages #42274
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
Changes from 2 commits
53a3f7a
96065be
af353f5
3e9d1ef
b292a53
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -56,6 +56,13 @@ build --action_env=BAZEL_VOLATILE_DIRTY --host_action_env=BAZEL_VOLATILE_DIRTY | |
|
|
||
| build --test_summary=terse | ||
|
|
||
| # Disable automatic C++ toolchain detection | ||
| # Users must explicitly select a toolchain with --config=gcc or --config=clang | ||
| build --action_env=BAZEL_DO_NOT_DETECT_CPP_TOOLCHAIN=1 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you still havent removed this
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was already removed in 3e9d1ef. The current HEAD has the flag removed - only the pre-existing rbe-toolchain config retains it at line 394. |
||
|
|
||
| # Enforce explicit toolchain selection | ||
| build --//tools/build_config:enforce_toolchain | ||
|
|
||
| # TODO(keith): Remove once these 2 are the default | ||
| build --incompatible_config_setting_private_default_visibility | ||
| build --incompatible_enforce_config_setting_visibility | ||
|
|
@@ -122,12 +129,14 @@ common:clang-common --action_env=CXX=clang++ --host_action_env=CXX=clang++ | |
| common:clang --config=clang-common | ||
| common:clang --config=libc++ | ||
| common:clang --action_env=LDFLAGS="-fuse-ld=lld" | ||
| common:clang --//tools/build_config:toolchain_identifier=clang | ||
|
|
||
| # Use gold linker for gcc compiler. | ||
| build:gcc --config=libstdc++ | ||
| build:gcc --test_env=HEAPCHECK= | ||
| build:gcc --action_env=BAZEL_COMPILER=gcc | ||
| build:gcc --action_env=CC=gcc --action_env=CXX=g++ | ||
| build:gcc --//tools/build_config:toolchain_identifier=gcc | ||
| # This is to work around a bug in GCC that makes debug-types-section | ||
| # option not play well with fission: | ||
| # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110885 | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| load("@bazel_skylib//rules:common_settings.bzl", "string_flag") | ||
| load(":defs.bzl", "toolchain_enforcement") | ||
|
|
||
| licenses(["notice"]) # Apache 2 | ||
|
|
||
| string_flag( | ||
| name = "toolchain_identifier", | ||
| build_setting_default = "", | ||
| visibility = ["//visibility:public"], | ||
| ) | ||
|
|
||
| toolchain_enforcement( | ||
| name = "enforce_toolchain", | ||
| toolchain_identifier = ":toolchain_identifier", | ||
| visibility = ["//visibility:public"], | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| load("@bazel_skylib//rules:common_settings.bzl", "BuildSettingInfo") | ||
|
|
||
| def _toolchain_enforcement_impl(ctx): | ||
| """Implementation of the toolchain enforcement rule. | ||
|
|
||
| This rule checks if a C++ toolchain has been explicitly selected. | ||
| If not, it fails with a helpful error message. | ||
| """ | ||
| toolchain_id = ctx.attr.toolchain_identifier[BuildSettingInfo].value | ||
|
|
||
| if toolchain_id == "": | ||
| fail(""" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. move this string to a constant outside the function - eg
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved to |
||
| ╔═══════════════════════════════════════════════════════════════════════════════╗ | ||
| ║ C++ Toolchain Selection Required ║ | ||
| ╚═══════════════════════════════════════════════════════════════════════════════╝ | ||
|
|
||
| ERROR: No C++ toolchain has been selected for this build. | ||
|
|
||
| Automatic C++ toolchain detection is disabled in this repository. | ||
| You must explicitly specify a toolchain using one of the following options: | ||
|
|
||
| • For GCC: bazel build --config=gcc <target> | ||
| • For Clang: bazel build --config=clang <target> | ||
|
|
||
| You can also set a default in your user.bazelrc file: | ||
| build --config=gcc | ||
|
|
||
| For more information, see the Envoy developer documentation. | ||
| """) | ||
|
|
||
| # Return an empty default info provider - this rule doesn't produce any outputs | ||
| return [DefaultInfo()] | ||
|
|
||
| toolchain_enforcement = rule( | ||
| implementation = _toolchain_enforcement_impl, | ||
| attrs = { | ||
| "toolchain_identifier": attr.label( | ||
| providers = [BuildSettingInfo], | ||
| mandatory = True, | ||
| doc = "The toolchain_identifier build setting to check", | ||
| ), | ||
| }, | ||
| doc = """ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same with this ->
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved to |
||
| Rule that enforces explicit C++ toolchain selection. | ||
|
|
||
| This rule reads the toolchain_identifier build setting and fails the build | ||
| if no toolchain has been explicitly selected (i.e., the value is empty). | ||
| """, | ||
| ) | ||
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.
@copilot remove this - we are not ready for this yet
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.
Removed in 3e9d1ef. The enforcement mechanism remains in place but the global BAZEL_DO_NOT_DETECT_CPP_TOOLCHAIN=1 flag has been removed.