From 84fceed8876865dc3419e9646a03091e44e90699 Mon Sep 17 00:00:00 2001 From: Googler Date: Thu, 1 Aug 2024 19:19:23 -0700 Subject: [PATCH] Disallow features from specifying whether they are enabled by default or not. BEGIN_PUBLIC Disallow features from specifying whether they are enabled by default or not. Such a decision shouldn't be made by the feature, but instead by the toolchain author. END_PUBLIC PiperOrigin-RevId: 658621275 Change-Id: I4dae8ee1acc349a0ff6f09e6cf68e15fdc481a48 --- cc/toolchains/feature.bzl | 16 ++++++---------- tests/rule_based_toolchain/features/BUILD | 7 ------- .../features/features_test.bzl | 1 - .../rule_based_toolchain/toolchain_config/BUILD | 8 -------- .../toolchain_config/toolchain_config_test.bzl | 2 +- 5 files changed, 7 insertions(+), 27 deletions(-) diff --git a/cc/toolchains/feature.bzl b/cc/toolchains/feature.bzl index 312f184..075ff07 100644 --- a/cc/toolchains/feature.bzl +++ b/cc/toolchains/feature.bzl @@ -65,7 +65,9 @@ def _cc_feature_impl(ctx): feature = FeatureInfo( label = ctx.label, name = name, - enabled = ctx.attr.enabled, + # Unused field, but leave it just in case we want to reuse it in the + # future. + enabled = False, args = collect_args_lists(ctx.attr.args, ctx.label), implies = collect_features(ctx.attr.implies), requires_any_of = tuple(collect_provider( @@ -120,10 +122,6 @@ Example: ) """, ), - "enabled": attr.bool( - mandatory = True, - doc = """Whether or not this feature is enabled by default.""", - ), "args": attr.label_list( doc = """Args that, when expanded, implement this feature.""", providers = [ArgsListInfo], @@ -137,7 +135,8 @@ deemed compatible and may be enabled. Note: Even if `cc_feature.requires_any_of` is satisfied, a feature is not enabled unless another mechanism (e.g. command-line flags, `cc_feature.implies`, -`cc_feature.enabled`) signals that the feature should actually be enabled. +`cc_toolchain_config.enabled_features`) signals that the feature should actually +be enabled. """, providers = [FeatureSetInfo], ), @@ -159,8 +158,7 @@ It can be either: `mutually_exclusive = [":category"]` are mutually exclusive with each other. If this feature has a side-effect of implementing another feature, it can be -useful to list that feature here to ensure they aren't enabled at the -same time. +useful to list that feature here to ensure they aren't enabled at the same time. """, ), "overrides": attr.label( @@ -224,7 +222,6 @@ Examples: # A feature that can be easily toggled to optimize for size cc_feature( name = "optimize_for_size", - enabled = False, feature_name = "optimize_for_size", args = [":optimize_for_size_args"], ) @@ -235,7 +232,6 @@ Examples: # https://bazel.build/docs/cc-toolchain-config-reference#wellknown-features cc_feature( name = "supports_pic", - enabled = True, overrides = "//cc/toolchains/features:supports_pic ) """, diff --git a/tests/rule_based_toolchain/features/BUILD b/tests/rule_based_toolchain/features/BUILD index a8bfb54..9a142be 100644 --- a/tests/rule_based_toolchain/features/BUILD +++ b/tests/rule_based_toolchain/features/BUILD @@ -20,7 +20,6 @@ util.helper_target( cc_feature, name = "simple", args = [":c_compile"], - enabled = False, feature_name = "feature_name", visibility = ["//tests/rule_based_toolchain:__subpackages__"], ) @@ -29,7 +28,6 @@ util.helper_target( cc_feature, name = "simple2", args = [":c_compile"], - enabled = False, feature_name = "simple2", ) @@ -46,7 +44,6 @@ util.helper_target( cc_feature, name = "requires", args = [":c_compile"], - enabled = True, feature_name = "requires", requires_any_of = [":feature_set"], ) @@ -55,7 +52,6 @@ util.helper_target( cc_feature, name = "implies", args = [":c_compile"], - enabled = True, feature_name = "implies", implies = [":simple"], ) @@ -68,7 +64,6 @@ util.helper_target( cc_feature, name = "mutual_exclusion_feature", args = [":c_compile"], - enabled = True, feature_name = "mutual_exclusion", mutually_exclusive = [ ":simple", @@ -105,14 +100,12 @@ util.helper_target( cc_feature, name = "overrides", args = [":c_compile"], - enabled = True, overrides = ":builtin_feature", ) util.helper_target( cc_feature, name = "sentinel_feature", - enabled = True, feature_name = "sentinel_feature_name", ) diff --git a/tests/rule_based_toolchain/features/features_test.bzl b/tests/rule_based_toolchain/features/features_test.bzl index ee18a07..332d27e 100644 --- a/tests/rule_based_toolchain/features/features_test.bzl +++ b/tests/rule_based_toolchain/features/features_test.bzl @@ -41,7 +41,6 @@ def _sentinel_feature_test(env, targets): sentinel_feature = env.expect.that_target(targets.sentinel_feature).provider(FeatureInfo) sentinel_feature.name().equals("sentinel_feature_name") sentinel_feature.args().args().contains_exactly([]) - sentinel_feature.enabled().equals(True) def _simple_feature_test(env, targets): simple = env.expect.that_target(targets.simple).provider(FeatureInfo) diff --git a/tests/rule_based_toolchain/toolchain_config/BUILD b/tests/rule_based_toolchain/toolchain_config/BUILD index 7f03a88..7acb55e 100644 --- a/tests/rule_based_toolchain/toolchain_config/BUILD +++ b/tests/rule_based_toolchain/toolchain_config/BUILD @@ -89,7 +89,6 @@ util.helper_target( cc_feature, name = "compile_feature", args = [":compile_args"], - enabled = True, feature_name = "compile_feature", ) @@ -107,7 +106,6 @@ util.helper_target( cc_feature, name = "implies_simple_feature", args = [":c_compile_args"], - enabled = True, feature_name = "implies", implies = [":simple_feature"], ) @@ -116,7 +114,6 @@ util.helper_target( cc_feature, name = "overrides_feature", args = [":c_compile_args"], - enabled = True, overrides = ":builtin_feature", ) @@ -132,7 +129,6 @@ util.helper_target( cc_feature, name = "requires_all_simple_feature", args = [":c_compile_args"], - enabled = True, feature_name = "requires_any_simple", requires_any_of = [":all_simple_features"], ) @@ -141,7 +137,6 @@ util.helper_target( cc_feature, name = "requires_any_simple_feature", args = [":c_compile_args"], - enabled = True, feature_name = "requires_any_simple", requires_any_of = [ ":simple_feature", @@ -153,7 +148,6 @@ util.helper_target( cc_feature, name = "same_feature_name", args = [":c_compile_args"], - enabled = False, feature_name = "simple_feature", visibility = ["//tests/rule_based_toolchain:__subpackages__"], ) @@ -162,7 +156,6 @@ util.helper_target( cc_feature, name = "simple_feature", args = [":c_compile_args"], - enabled = False, feature_name = "simple_feature", ) @@ -170,7 +163,6 @@ util.helper_target( cc_feature, name = "simple_feature2", args = [":c_compile_args"], - enabled = False, feature_name = "simple_feature2", visibility = ["//tests/rule_based_toolchain:__subpackages__"], ) diff --git a/tests/rule_based_toolchain/toolchain_config/toolchain_config_test.bzl b/tests/rule_based_toolchain/toolchain_config/toolchain_config_test.bzl index 845d595..a550c50 100644 --- a/tests/rule_based_toolchain/toolchain_config/toolchain_config_test.bzl +++ b/tests/rule_based_toolchain/toolchain_config/toolchain_config_test.bzl @@ -197,7 +197,7 @@ def _toolchain_collects_files_test(env, targets): ), legacy_feature( name = "compile_feature", - enabled = True, + enabled = False, flag_sets = [legacy_flag_set( actions = ["c_compile", "cpp_compile"], flag_groups = [