From 5e848c1434d3458018734238dbc4781f43992ea5 Mon Sep 17 00:00:00 2001 From: Googler Date: Tue, 11 Jun 2024 16:44:04 -0700 Subject: [PATCH] Remove the "data" attribute from cc_action_type_config. BEGIN_PUBLIC Remove the "data" attribute from cc_action_type_config. Technically speaking, data shouldn't be associated with action types. Instead, data should be associated with either a tool or a set of flags. For example, instead of the cc_compile action having the header files as a data dependency, the "include_paths" cc_arg should instead declare `data =
`. This will allow us to, once we have a fully starlark-ified c++ toolchain, do much finer-grained dependencies. This will allow us to, for example, not provide header files to the action when the user enables the feature "nostdlib". END_PUBLIC PiperOrigin-RevId: 642434412 Change-Id: Id16fe05a1c86bbaf4718cd36a15f8a9d6afb0163 --- cc/toolchains/action_type_config.bzl | 14 +------------- .../rule_based_toolchain/action_type_config/BUILD | 3 --- .../action_type_config/action_type_config_test.bzl | 7 ++----- 3 files changed, 3 insertions(+), 21 deletions(-) diff --git a/cc/toolchains/action_type_config.bzl b/cc/toolchains/action_type_config.bzl index fa2fc50..4752ac2 100644 --- a/cc/toolchains/action_type_config.bzl +++ b/cc/toolchains/action_type_config.bzl @@ -17,7 +17,6 @@ load( "//cc/toolchains/impl:collect.bzl", "collect_action_types", "collect_features", - "collect_files", "collect_tools", ) load( @@ -36,7 +35,6 @@ def _cc_action_type_config_impl(ctx): tools = tuple(collect_tools(ctx, ctx.attr.tools)) implies = collect_features(ctx.attr.implies) - files = collect_files(ctx.attr.data) configs = {} for action_type in collect_action_types(ctx.attr.action_types).to_list(): @@ -45,9 +43,7 @@ def _cc_action_type_config_impl(ctx): action_type = action_type, tools = tools, implies = implies, - files = ctx.runfiles( - transitive_files = depset(transitive = [files]), - ).merge_all([tool.runfiles for tool in tools]), + files = ctx.runfiles().merge_all([tool.runfiles for tool in tools]), ) return [ActionTypeConfigSetInfo(label = ctx.label, configs = configs)] @@ -80,14 +76,6 @@ satisfy the currently enabled feature set is used. providers = [FeatureSetInfo], doc = "Features that should be enabled when this action is used.", ), - "data": attr.label_list( - allow_files = True, - doc = """Files required for this action type. - -For example, the c-compile action type might add the C standard library header -files from the sysroot. -""", - ), }, provides = [ActionTypeConfigSetInfo], doc = """Declares the configuration and selection of `cc_tool` rules. diff --git a/tests/rule_based_toolchain/action_type_config/BUILD b/tests/rule_based_toolchain/action_type_config/BUILD index 689babd..3c44113 100644 --- a/tests/rule_based_toolchain/action_type_config/BUILD +++ b/tests/rule_based_toolchain/action_type_config/BUILD @@ -7,9 +7,6 @@ util.helper_target( cc_action_type_config, name = "file_map", action_types = ["//tests/rule_based_toolchain/actions:all_compile"], - data = [ - "//tests/rule_based_toolchain/testdata:multiple2", - ], tools = [ "//tests/rule_based_toolchain/testdata:bin_wrapper.sh", "//tests/rule_based_toolchain/tool:wrapped_tool", 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 bbb5de4..3394ee2 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 @@ -28,19 +28,16 @@ _TOOL_FILES = [ "tests/rule_based_toolchain/testdata/bin_wrapper", "tests/rule_based_toolchain/testdata/bin_wrapper.sh", ] -_ADDITIONAL_FILES = [ - "tests/rule_based_toolchain/testdata/multiple2", -] 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(_TOOL_FILES + _ADDITIONAL_FILES) + c_compile.files().contains_exactly(_TOOL_FILES) cpp_compile = configs.get(targets.cpp_compile[ActionTypeInfo]) - cpp_compile.files().contains_exactly(_TOOL_FILES + _ADDITIONAL_FILES) + cpp_compile.files().contains_exactly(_TOOL_FILES) def _merge_distinct_configs_succeeds_test(env, targets): configs = env.expect.that_value(