Skip to content

Commit

Permalink
Disallow features from specifying whether they are enabled by default…
Browse files Browse the repository at this point in the history
… 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
  • Loading branch information
matts1 authored and copybara-github committed Aug 2, 2024
1 parent af92637 commit 84fceed
Show file tree
Hide file tree
Showing 5 changed files with 7 additions and 27 deletions.
16 changes: 6 additions & 10 deletions cc/toolchains/feature.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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],
Expand All @@ -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],
),
Expand All @@ -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(
Expand Down Expand Up @@ -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"],
)
Expand All @@ -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
)
""",
Expand Down
7 changes: 0 additions & 7 deletions tests/rule_based_toolchain/features/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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__"],
)
Expand All @@ -29,7 +28,6 @@ util.helper_target(
cc_feature,
name = "simple2",
args = [":c_compile"],
enabled = False,
feature_name = "simple2",
)

Expand All @@ -46,7 +44,6 @@ util.helper_target(
cc_feature,
name = "requires",
args = [":c_compile"],
enabled = True,
feature_name = "requires",
requires_any_of = [":feature_set"],
)
Expand All @@ -55,7 +52,6 @@ util.helper_target(
cc_feature,
name = "implies",
args = [":c_compile"],
enabled = True,
feature_name = "implies",
implies = [":simple"],
)
Expand All @@ -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",
Expand Down Expand Up @@ -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",
)

Expand Down
1 change: 0 additions & 1 deletion tests/rule_based_toolchain/features/features_test.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 0 additions & 8 deletions tests/rule_based_toolchain/toolchain_config/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ util.helper_target(
cc_feature,
name = "compile_feature",
args = [":compile_args"],
enabled = True,
feature_name = "compile_feature",
)

Expand All @@ -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"],
)
Expand All @@ -116,7 +114,6 @@ util.helper_target(
cc_feature,
name = "overrides_feature",
args = [":c_compile_args"],
enabled = True,
overrides = ":builtin_feature",
)

Expand All @@ -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"],
)
Expand All @@ -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",
Expand All @@ -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__"],
)
Expand All @@ -162,15 +156,13 @@ util.helper_target(
cc_feature,
name = "simple_feature",
args = [":c_compile_args"],
enabled = False,
feature_name = "simple_feature",
)

util.helper_target(
cc_feature,
name = "simple_feature2",
args = [":c_compile_args"],
enabled = False,
feature_name = "simple_feature2",
visibility = ["//tests/rule_based_toolchain:__subpackages__"],
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down

0 comments on commit 84fceed

Please sign in to comment.