Skip to content

Commit

Permalink
Replace toolchain_features with known_features and enabled_features.
Browse files Browse the repository at this point in the history
BEGIN_PUBLIC
Replace toolchain_features with known_features and enabled_features.

This should allow for the deprecation of action_type_config, as `implies` there is no longer required.
END_PUBLIC

PiperOrigin-RevId: 658620044
Change-Id: Idda9bd77edad1be1fd26d5a655e3b9084d38bca8
  • Loading branch information
matts1 authored and copybara-github committed Aug 2, 2024
1 parent dcf1dc1 commit af92637
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 24 deletions.
1 change: 1 addition & 0 deletions cc/toolchains/cc_toolchain_info.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ ToolchainConfigInfo = provider(
fields = {
"label": "(Label) The label defining this provider. Place in error messages to simplify debugging",
"features": "(Sequence[FeatureInfo]) The features available for this toolchain",
"enabled_features": "(Sequence[FeatureInfo]) The features That are enabled by default for this toolchain",
"action_type_configs": "(dict[ActionTypeInfo, ActionTypeConfigInfo]) The configuration of action configs for the toolchain.",
"args": "(Sequence[ArgsInfo]) A list of arguments to be unconditionally applied to the toolchain.",
"files": "(dict[ActionTypeInfo, depset[File]]) Files required for the toolchain, keyed by the action type.",
Expand Down
4 changes: 2 additions & 2 deletions cc/toolchains/impl/legacy_converter.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ def convert_toolchain(toolchain):
name = "implied_by_always_enabled",
enabled = True,
args = ArgsListInfo(args = toolchain.args),
implies = depset([]),
implies = depset([ft for ft in toolchain.enabled_features]),
requires_any_of = [],
mutually_exclusive = [],
external = False,
Expand All @@ -168,6 +168,6 @@ def convert_toolchain(toolchain):
]

return struct(
features = sorted([ft for ft in features if ft != None], key = lambda ft: ft.name),
features = [ft for ft in features if ft != None],
action_configs = sorted(action_configs, key = lambda ac: ac.action_name),
)
8 changes: 5 additions & 3 deletions cc/toolchains/impl/toolchain_config.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,15 @@ cc_legacy_file_group = rule(

def _cc_toolchain_config_impl(ctx):
if ctx.attr.features:
fail("Features is a reserved attribute in bazel. Did you mean 'toolchain_features'")
fail("Features is a reserved attribute in bazel. Did you mean 'known_features' or 'enabled_features'?")

if not ctx.attr._enabled[BuildSettingInfo].value and not ctx.attr.skip_experimental_flag_validation_for_test:
fail("Rule based toolchains are experimental. To use it, please add --@rules_cc//cc/toolchains:experimental_enable_rule_based_toolchains to your bazelrc")

toolchain_config = toolchain_config_info(
label = ctx.label,
features = ctx.attr.toolchain_features + [ctx.attr._builtin_features],
known_features = ctx.attr.known_features + [ctx.attr._builtin_features],
enabled_features = ctx.attr.enabled_features,
action_type_configs = ctx.attr.action_type_configs,
args = ctx.attr.args,
)
Expand Down Expand Up @@ -109,7 +110,8 @@ cc_toolchain_config = rule(
# Attributes new to this rule.
"action_type_configs": attr.label_list(providers = [ActionTypeConfigSetInfo]),
"args": attr.label_list(providers = [ArgsListInfo]),
"toolchain_features": attr.label_list(providers = [FeatureSetInfo]),
"known_features": attr.label_list(providers = [FeatureSetInfo]),
"enabled_features": attr.label_list(providers = [FeatureSetInfo]),
"skip_experimental_flag_validation_for_test": attr.bool(default = False),
"_builtin_features": attr.label(default = "//cc/toolchains/features:all_builtin_features"),
"_enabled": attr.label(default = "//cc/toolchains:experimental_enable_rule_based_toolchains"),
Expand Down
16 changes: 13 additions & 3 deletions cc/toolchains/impl/toolchain_config_info.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -132,20 +132,29 @@ def _collect_files_for_action_type(atc, features, args):

return depset(transitive = transitive_files)

def toolchain_config_info(label, features = [], args = [], action_type_configs = [], fail = fail):
def toolchain_config_info(label, known_features = [], enabled_features = [], args = [], action_type_configs = [], fail = fail):
"""Generates and validates a ToolchainConfigInfo from lists of labels.
Args:
label: (Label) The label to apply to the ToolchainConfigInfo
features: (List[Target]) A list of targets providing FeatureSetInfo
known_features: (List[Target]) A list of features that can be enabled.
enabled_features: (List[Target]) A list of features that are enabled by
default. Every enabled feature is implicitly also a known feature.
args: (List[Target]) A list of targets providing ArgsListInfo
action_type_configs: (List[Target]) A list of targets providing
ActionTypeConfigSetInfo
fail: A fail function. Use only during tests.
Returns:
A validated ToolchainConfigInfo
"""
features = collect_features(features).to_list()

# Later features will come after earlier features on the command-line, and
# thus override them. Because of this, we ensure that known_features comes
# *after* enabled_features, so that if we do enable them, they override the
# default feature flags.
features = collect_features(enabled_features + known_features).to_list()
enabled_features = collect_features(enabled_features).to_list()

args = collect_args_lists(args, label = label)
action_type_configs = collect_action_type_config_sets(
action_type_configs,
Expand All @@ -160,6 +169,7 @@ def toolchain_config_info(label, features = [], args = [], action_type_configs =
toolchain_config = ToolchainConfigInfo(
label = label,
features = features,
enabled_features = enabled_features,
action_type_configs = action_type_configs,
args = args.args,
files = files,
Expand Down
1 change: 1 addition & 0 deletions tests/rule_based_toolchain/subjects.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ _ToolchainConfigFactory = generate_factory(
"ToolchainConfigInfo",
dict(
features = ProviderDepset(_FeatureFactory),
enabled_features = _subjects.collection,
action_type_configs = dict_key_subject(_ActionTypeConfigFactory.factory),
args = ProviderSequence(_ArgsFactory),
files = dict_key_subject(_subjects.depset_file),
Expand Down
3 changes: 2 additions & 1 deletion tests/rule_based_toolchain/toolchain_config/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,10 @@ util.helper_target(
cxx_builtin_include_directories = [
"//tests/rule_based_toolchain/testdata:directory",
],
enabled_features = [":simple_feature"],
known_features = [":compile_feature"],
skip_experimental_flag_validation_for_test = True,
sysroot = "//tests/rule_based_toolchain/testdata:directory",
toolchain_features = [":compile_feature"],
)

util.helper_target(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ def _empty_toolchain_valid_test(env, _targets):
def _duplicate_feature_names_invalid_test(env, targets):
_expect_that_toolchain(
env,
features = [targets.simple_feature, targets.same_feature_name],
known_features = [targets.simple_feature, targets.same_feature_name],
expr = "duplicate_feature_name",
).err().contains_all_of([
"The feature name simple_feature was defined by",
Expand All @@ -67,14 +67,14 @@ def _duplicate_feature_names_invalid_test(env, targets):
# Overriding a feature gives it the same name. Ensure this isn't blocked.
_expect_that_toolchain(
env,
features = [targets.builtin_feature, targets.overrides_feature],
known_features = [targets.builtin_feature, targets.overrides_feature],
expr = "override_feature",
).ok()

def _duplicate_action_type_invalid_test(env, targets):
_expect_that_toolchain(
env,
features = [targets.simple_feature],
known_features = [targets.simple_feature],
action_type_configs = [targets.compile_config, targets.c_compile_config],
).err().contains_all_of([
"The action type %s is configured by" % targets.c_compile.label,
Expand All @@ -85,14 +85,14 @@ def _duplicate_action_type_invalid_test(env, targets):
def _action_config_implies_missing_feature_invalid_test(env, targets):
_expect_that_toolchain(
env,
features = [targets.simple_feature],
known_features = [targets.simple_feature],
action_type_configs = [targets.c_compile_config],
expr = "action_type_config_with_implies",
).ok()

_expect_that_toolchain(
env,
features = [],
known_features = [],
action_type_configs = [targets.c_compile_config],
expr = "action_type_config_missing_implies",
).err().contains(
Expand All @@ -103,12 +103,12 @@ def _feature_config_implies_missing_feature_invalid_test(env, targets):
_expect_that_toolchain(
env,
expr = "feature_with_implies",
features = [targets.simple_feature, targets.implies_simple_feature],
known_features = [targets.simple_feature, targets.implies_simple_feature],
).ok()

_expect_that_toolchain(
env,
features = [targets.implies_simple_feature],
known_features = [targets.implies_simple_feature],
expr = "feature_missing_implies",
).err().contains(
"%s implies the feature %s" % (targets.implies_simple_feature.label, targets.simple_feature.label),
Expand All @@ -117,37 +117,37 @@ def _feature_config_implies_missing_feature_invalid_test(env, targets):
def _feature_missing_requirements_invalid_test(env, targets):
_expect_that_toolchain(
env,
features = [targets.requires_any_simple_feature, targets.simple_feature],
known_features = [targets.requires_any_simple_feature, targets.simple_feature],
expr = "requires_any_simple_has_simple",
).ok()
_expect_that_toolchain(
env,
features = [targets.requires_any_simple_feature, targets.simple_feature2],
known_features = [targets.requires_any_simple_feature, targets.simple_feature2],
expr = "requires_any_simple_has_simple2",
).ok()
_expect_that_toolchain(
env,
features = [targets.requires_any_simple_feature],
known_features = [targets.requires_any_simple_feature],
expr = "requires_any_simple_has_none",
).err().contains(
"It is impossible to enable %s" % targets.requires_any_simple_feature.label,
)

_expect_that_toolchain(
env,
features = [targets.requires_all_simple_feature, targets.simple_feature, targets.simple_feature2],
known_features = [targets.requires_all_simple_feature, targets.simple_feature, targets.simple_feature2],
expr = "requires_all_simple_has_both",
).ok()
_expect_that_toolchain(
env,
features = [targets.requires_all_simple_feature, targets.simple_feature],
known_features = [targets.requires_all_simple_feature, targets.simple_feature],
expr = "requires_all_simple_has_simple",
).err().contains(
"It is impossible to enable %s" % targets.requires_all_simple_feature.label,
)
_expect_that_toolchain(
env,
features = [targets.requires_all_simple_feature, targets.simple_feature2],
known_features = [targets.requires_all_simple_feature, targets.simple_feature2],
expr = "requires_all_simple_has_simple2",
).err().contains(
"It is impossible to enable %s" % targets.requires_all_simple_feature.label,
Expand All @@ -157,13 +157,13 @@ def _args_missing_requirements_invalid_test(env, targets):
_expect_that_toolchain(
env,
args = [targets.requires_all_simple_args],
features = [targets.simple_feature, targets.simple_feature2],
known_features = [targets.simple_feature, targets.simple_feature2],
expr = "has_both",
).ok()
_expect_that_toolchain(
env,
args = [targets.requires_all_simple_args],
features = [targets.simple_feature],
known_features = [targets.simple_feature],
expr = "has_only_one",
).err().contains(
"It is impossible to enable %s" % targets.requires_all_simple_args.label,
Expand All @@ -185,6 +185,16 @@ def _toolchain_collects_files_test(env, targets):

legacy = convert_toolchain(tc.actual)
env.expect.that_collection(legacy.features).contains_exactly([
legacy_feature(
name = "simple_feature",
enabled = False,
flag_sets = [legacy_flag_set(
actions = ["c_compile"],
flag_groups = [
legacy_flag_group(flags = ["c_compile_args"]),
],
)],
),
legacy_feature(
name = "compile_feature",
enabled = True,
Expand All @@ -198,6 +208,7 @@ def _toolchain_collects_files_test(env, targets):
legacy_feature(
name = "implied_by_always_enabled",
enabled = True,
implies = ["simple_feature"],
flag_sets = [legacy_flag_set(
actions = ["c_compile"],
flag_groups = [
Expand Down

0 comments on commit af92637

Please sign in to comment.