Remove support for args from action_type_config.
BEGIN_PUBLIC
Remove support for args from action_type_config.
Args in individual action type configs result in redundant configuration, where both the cc_args and the cc_action_type_config control which actions they're enabled for. Instead of:
```
cc_args(name = "compile_args", action_types = [":c_compile", ":cpp_compile])
cc_action_type_config(name = "c_compile_config", actions = [":c_compile"], args = [":compile_args"])
cc_action_type_config(name = "cpp_compile_config", actions = [":cpp_compile"], args = [":compile_args"])
cc_toolchain(action_type_configs = [":c_compile_config", ":cpp_compile_config"])
```
We should force users to write the following:
```
cc_args(name = "compile_args", action_types = [":c_compile", ":cpp_compile])
cc_action_type_config(name = "c_compile_config", actions = [":c_compile"])
cc_action_type_config(name = "cpp_compile_config", actions = [":cpp_compile"])
cc_toolchain(action_type_configs = [":c_compile_config", ":cpp_compile_config"], args = [":compile_args"])
```
END_PUBLIC
PiperOrigin-RevId: 642432029
Change-Id: I1aa7c1752f4d915d8c84c17a06314ae9ad2c69f0
diff --git a/cc/toolchains/action_type_config.bzl b/cc/toolchains/action_type_config.bzl
index 4d5a8cd..fa2fc50 100644
--- a/cc/toolchains/action_type_config.bzl
+++ b/cc/toolchains/action_type_config.bzl
@@ -13,11 +13,9 @@
# limitations under the License.
"""Implementation of cc_action_type_config."""
-load("//cc/toolchains/impl:args_utils.bzl", "get_action_type")
load(
"//cc/toolchains/impl:collect.bzl",
"collect_action_types",
- "collect_args_lists",
"collect_features",
"collect_files",
"collect_tools",
@@ -27,7 +25,6 @@
"ActionTypeConfigInfo",
"ActionTypeConfigSetInfo",
"ActionTypeSetInfo",
- "ArgsListInfo",
"FeatureSetInfo",
)
@@ -39,20 +36,17 @@
tools = tuple(collect_tools(ctx, ctx.attr.tools))
implies = collect_features(ctx.attr.implies)
- args_list = collect_args_lists(ctx.attr.args, ctx.label)
files = collect_files(ctx.attr.data)
configs = {}
for action_type in collect_action_types(ctx.attr.action_types).to_list():
- for_action = get_action_type(args_list, action_type)
configs[action_type] = ActionTypeConfigInfo(
label = ctx.label,
action_type = action_type,
tools = tools,
- args = for_action.args,
implies = implies,
files = ctx.runfiles(
- transitive_files = depset(transitive = [files, for_action.files]),
+ transitive_files = depset(transitive = [files]),
).merge_all([tool.runfiles for tool in tools]),
)
@@ -82,12 +76,6 @@
satisfy the currently enabled feature set is used.
""",
),
- "args": attr.label_list(
- providers = [ArgsListInfo],
- doc = """Labels that point to `cc_arg`s / `cc_arg_list`s that are
-unconditionally bound to the specified actions.
-""",
- ),
"implies": attr.label_list(
providers = [FeatureSetInfo],
doc = "Features that should be enabled when this action is used.",
diff --git a/cc/toolchains/cc_toolchain_info.bzl b/cc/toolchains/cc_toolchain_info.bzl
index 3a499f6..f056995 100644
--- a/cc/toolchains/cc_toolchain_info.bzl
+++ b/cc/toolchains/cc_toolchain_info.bzl
@@ -163,7 +163,6 @@
"label": "(Label) The label defining this provider. Place in error messages to simplify debugging",
"action_type": "(ActionTypeInfo) The type of the action",
"tools": "(Sequence[ToolInfo]) The tool applied to the action will be the first tool in the sequence with a feature set that matches the feature configuration",
- "args": "(Sequence[ArgsInfo]) Set of flag sets the action sets",
"implies": "(depset[FeatureInfo]) Set of features implied by this action config",
"files": "(runfiles) The files required to run these actions",
},
diff --git a/cc/toolchains/impl/legacy_converter.bzl b/cc/toolchains/impl/legacy_converter.bzl
index 9f9d2a9..c01f3ec 100644
--- a/cc/toolchains/impl/legacy_converter.bzl
+++ b/cc/toolchains/impl/legacy_converter.bzl
@@ -136,8 +136,6 @@
def _convert_action_type_config(atc):
implies = sorted([ft.name for ft in atc.implies.to_list()])
- if atc.args:
- implies.append("implied_by_%s" % atc.action_type.name)
return legacy_action_config(
action_name = atc.action_type.name,
@@ -167,22 +165,10 @@
mutually_exclusive = [],
external = False,
)))
- action_configs = []
- for atc in toolchain.action_type_configs.values():
- # Action configs don't take in an env like they do a flag set.
- # In order to support them, we create a feature with the env that the action
- # config will enable, and imply it in the action config.
- if atc.args:
- features.append(convert_feature(FeatureInfo(
- name = "implied_by_%s" % atc.action_type.name,
- enabled = False,
- args = ArgsListInfo(args = atc.args),
- implies = depset([]),
- requires_any_of = [],
- mutually_exclusive = [],
- external = False,
- )))
- action_configs.append(_convert_action_type_config(atc))
+ action_configs = [
+ _convert_action_type_config(atc)
+ for atc in toolchain.action_type_configs.values()
+ ]
return struct(
features = sorted([ft for ft in features if ft != None], key = lambda ft: ft.name),
diff --git a/cc/toolchains/impl/toolchain_config_info.bzl b/cc/toolchains/impl/toolchain_config_info.bzl
index e2a8b37..698c2ec 100644
--- a/cc/toolchains/impl/toolchain_config_info.bzl
+++ b/cc/toolchains/impl/toolchain_config_info.bzl
@@ -116,8 +116,6 @@
_validate_implies(self, known_features, fail = fail)
for tool in self.tools:
_validate_tool(tool, known_features, fail = fail)
- for args in self.args:
- _validate_args(args, known_features, fail = fail)
def _validate_feature(self, known_features, fail):
_validate_requires_any_of_feature_set(self, known_features, fail = fail)
diff --git a/tests/rule_based_toolchain/action_type_config/BUILD b/tests/rule_based_toolchain/action_type_config/BUILD
index e7b9194..689babd 100644
--- a/tests/rule_based_toolchain/action_type_config/BUILD
+++ b/tests/rule_based_toolchain/action_type_config/BUILD
@@ -7,7 +7,6 @@
cc_action_type_config,
name = "file_map",
action_types = ["//tests/rule_based_toolchain/actions:all_compile"],
- args = ["//tests/rule_based_toolchain/args_list"],
data = [
"//tests/rule_based_toolchain/testdata:multiple2",
],
diff --git a/tests/rule_based_toolchain/action_type_config/action_type_config_test.bzl b/tests/rule_based_toolchain/action_type_config/action_type_config_test.bzl
index 7ee85e6..bbb5de4 100644
--- a/tests/rule_based_toolchain/action_type_config/action_type_config_test.bzl
+++ b/tests/rule_based_toolchain/action_type_config/action_type_config_test.bzl
@@ -31,36 +31,16 @@
_ADDITIONAL_FILES = [
"tests/rule_based_toolchain/testdata/multiple2",
]
-_C_COMPILE_FILES = [
- "tests/rule_based_toolchain/testdata/file1",
- "tests/rule_based_toolchain/testdata/multiple1",
-]
-_CPP_COMPILE_FILES = [
- "tests/rule_based_toolchain/testdata/file2",
- "tests/rule_based_toolchain/testdata/multiple1",
-]
collect_action_type_configs = result_fn_wrapper(_collect_action_type_configs)
def _files_taken_test(env, targets):
configs = env.expect.that_target(targets.file_map).provider(ActionTypeConfigSetInfo).configs()
c_compile = configs.get(targets.c_compile[ActionTypeInfo])
- c_compile.files().contains_exactly(
- _C_COMPILE_FILES + _TOOL_FILES + _ADDITIONAL_FILES,
- )
- c_compile.args().contains_exactly([
- targets.c_compile_args.label,
- targets.all_compile_args.label,
- ])
+ c_compile.files().contains_exactly(_TOOL_FILES + _ADDITIONAL_FILES)
cpp_compile = configs.get(targets.cpp_compile[ActionTypeInfo])
- cpp_compile.files().contains_exactly(
- _CPP_COMPILE_FILES + _TOOL_FILES + _ADDITIONAL_FILES,
- )
- cpp_compile.args().contains_exactly([
- targets.cpp_compile_args.label,
- targets.all_compile_args.label,
- ])
+ cpp_compile.files().contains_exactly(_TOOL_FILES + _ADDITIONAL_FILES)
def _merge_distinct_configs_succeeds_test(env, targets):
configs = env.expect.that_value(
@@ -95,10 +75,6 @@
":cpp_compile_config",
"//tests/rule_based_toolchain/actions:c_compile",
"//tests/rule_based_toolchain/actions:cpp_compile",
- "//tests/rule_based_toolchain/args_list:c_compile_args",
- "//tests/rule_based_toolchain/args_list:cpp_compile_args",
- "//tests/rule_based_toolchain/args_list:all_compile_args",
- "//tests/rule_based_toolchain/args_list:args_list",
]
TESTS = {
diff --git a/tests/rule_based_toolchain/subjects.bzl b/tests/rule_based_toolchain/subjects.bzl
index f42d5d7..0bc54af 100644
--- a/tests/rule_based_toolchain/subjects.bzl
+++ b/tests/rule_based_toolchain/subjects.bzl
@@ -191,7 +191,6 @@
dict(
action_type = _ActionTypeFactory,
tools = ProviderSequence(_ToolFactory),
- args = ProviderSequence(_ArgsFactory),
implies = ProviderDepset(_FeatureFactory),
files = runfiles_subject,
),
diff --git a/tests/rule_based_toolchain/toolchain_config/BUILD b/tests/rule_based_toolchain/toolchain_config/BUILD
index bb01110..569d91c 100644
--- a/tests/rule_based_toolchain/toolchain_config/BUILD
+++ b/tests/rule_based_toolchain/toolchain_config/BUILD
@@ -80,7 +80,6 @@
cc_action_type_config,
name = "compile_config",
action_types = ["//tests/rule_based_toolchain/actions:all_compile"],
- args = [":cpp_compile_args"],
tools = [
"//tests/rule_based_toolchain/tool:wrapped_tool",
],
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 e188772..4a08ab8 100644
--- a/tests/rule_based_toolchain/toolchain_config/toolchain_config_test.bzl
+++ b/tests/rule_based_toolchain/toolchain_config/toolchain_config_test.bzl
@@ -16,8 +16,6 @@
load(
"//cc:cc_toolchain_config_lib.bzl",
legacy_action_config = "action_config",
- legacy_env_entry = "env_entry",
- legacy_env_set = "env_set",
legacy_feature = "feature",
legacy_flag_group = "flag_group",
legacy_flag_set = "flag_set",
@@ -223,20 +221,6 @@
],
)],
),
- legacy_feature(
- name = "implied_by_cpp_compile",
- enabled = False,
- flag_sets = [legacy_flag_set(
- actions = ["cpp_compile"],
- flag_groups = [
- legacy_flag_group(flags = ["cpp_compile_args"]),
- ],
- )],
- env_sets = [legacy_env_set(
- actions = ["cpp_compile"],
- env_entries = [legacy_env_entry(key = "CPP_COMPILE", value = "1")],
- )],
- ),
]).in_order()
exe = tc.action_type_configs().get(
@@ -252,7 +236,7 @@
action_name = "cpp_compile",
enabled = True,
tools = [legacy_tool(tool = exe)],
- implies = ["implied_by_cpp_compile"],
+ implies = [],
),
]).in_order()
