diff --git a/cc/toolchains/README.md b/cc/toolchains/README.md index a6658c1..42266b7 100644 --- a/cc/toolchains/README.md +++ b/cc/toolchains/README.md @@ -40,7 +40,7 @@ An action config is a mapping from action to: * A set of additional files required for the action Each action can only be specified once in the toolchain. Specifying multiple -actions in a single `cc_action_config` is just a shorthand for specifying the +actions in a single `cc_action_type_config` is just a shorthand for specifying the same config for every one of those actions. If you're already familiar with how to define toolchains, the additional files @@ -50,7 +50,7 @@ Additionally, to replace `all_files`, we add `cc_additional_files_for_actions`. This allows you to specify that particular files are required for particular actions. -We provide `additional_files` on the `cc_action_config` as a shorthand for +We provide `additional_files` on the `cc_action_type_config` as a shorthand for specifying `cc_additional_files_for_actions` Warning: Implying a feature that is not listed directly in the toolchain will throw @@ -58,7 +58,7 @@ an error. This is to ensure you don't accidentally add a feature to the toolchain. ``` -cc_action_config( +cc_action_type_config( name = "c_compile", actions = ["@rules_cc//actions:all_c_compile"], tools = ["@sysroot//:clang"], @@ -196,7 +196,7 @@ cc_toolchain( name = "toolchain", features = [":my_feature"] unconditional_args = [":all_warnings"], - action_configs = [":c_compile"], + action_type_configs = [":c_compile"], additional_files = [":all_action_files"], ) ``` diff --git a/cc/toolchains/action_type_config.bzl b/cc/toolchains/action_type_config.bzl new file mode 100644 index 0000000..fbdc537 --- /dev/null +++ b/cc/toolchains/action_type_config.bzl @@ -0,0 +1,137 @@ +# Copyright 2024 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# 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", +) +load( + ":cc_toolchain_info.bzl", + "ActionTypeConfigInfo", + "ActionTypeConfigSetInfo", + "ActionTypeSetInfo", + "ArgsListInfo", + "FeatureSetInfo", +) + +def _cc_action_type_config_impl(ctx): + if not ctx.attr.action_types: + fail("At least one action type is required for cc_action_type_config") + if not ctx.attr.tools: + fail("At least one tool is required for cc_action_type_config") + + 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.additional_files) + + 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]), + ).merge_all([tool.runfiles for tool in tools]), + ) + + return [ActionTypeConfigSetInfo(label = ctx.label, configs = configs)] + +cc_action_type_config = rule( + implementation = _cc_action_type_config_impl, + # @unsorted-dict-items + attrs = { + "action_types": attr.label_list( + providers = [ActionTypeSetInfo], + mandatory = True, + doc = """A list of action names to apply this action to. + +See @toolchain//actions:all for valid options. +""", + ), + "tools": attr.label_list( + mandatory = True, + cfg = "exec", + allow_files = True, + doc = """The tool to use for the specified actions. + +A tool can be a `cc_tool`, or a binary. + +If multiple tools are specified, the first tool that has `with_features` that +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.", + ), + "additional_files": 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. + +Action configs are bound to a toolchain through `action_configs`, and are the +driving mechanism for controlling toolchain tool invocation/behavior. + +Action configs define three key things: + +* Which tools to invoke for a given type of action. +* Tool features and compatibility. +* `cc_args`s that are unconditionally bound to a tool invocation. + +Examples: + + cc_action_config( + name = "ar", + action_types = ["@toolchain//actions:all_ar_actions"], + implies = [ + "@toolchain//features/legacy:archiver_flags", + "@toolchain//features/legacy:linker_param_file", + ], + tools = [":ar_tool"], + ) + + cc_action_config( + name = "clang", + action_types = [ + "@toolchain//actions:all_asm_actions", + "@toolchain//actions:all_c_compiler_actions", + ], + tools = [":clang_tool"], + ) +""", +) diff --git a/cc/toolchains/actions.bzl b/cc/toolchains/actions.bzl index 7799a6e..31867dd 100644 --- a/cc/toolchains/actions.bzl +++ b/cc/toolchains/actions.bzl @@ -50,6 +50,8 @@ cc_action_type( ) def _cc_action_type_set_impl(ctx): + if not ctx.attr.actions: + fail("Each cc_action_type_set must contain at least one action type.") return [ActionTypeSetInfo( label = ctx.label, actions = collect_action_types(ctx.attr.actions), diff --git a/cc/toolchains/cc_toolchain_info.bzl b/cc/toolchains/cc_toolchain_info.bzl index c105364..ed2bdb6 100644 --- a/cc/toolchains/cc_toolchain_info.bzl +++ b/cc/toolchains/cc_toolchain_info.bzl @@ -133,13 +133,12 @@ ToolInfo = provider( }, ) -ActionConfigInfo = provider( +ActionTypeConfigInfo = provider( doc = "Configuration of a Bazel action.", # @unsorted-dict-items fields = { "label": "(Label) The label defining this provider. Place in error messages to simplify debugging", - "action": "(ActionTypeInfo) The name of the action", - "enabled": "(bool) If True, this action is enabled unless a rule type explicitly marks it as unsupported", + "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", @@ -147,11 +146,11 @@ ActionConfigInfo = provider( }, ) -ActionConfigSetInfo = provider( +ActionTypeConfigSetInfo = provider( doc = "A set of action configs", # @unsorted-dict-items fields = { "label": "(Label) The label defining this provider. Place in error messages to simplify debugging", - "action_configs": "(depset[ActionConfigInfo]) A set of action configs", + "configs": "(dict[ActionTypeInfo, ActionTypeConfigInfo]) A set of action configs", }, ) diff --git a/cc/toolchains/impl/args_utils.bzl b/cc/toolchains/impl/args_utils.bzl new file mode 100644 index 0000000..2ace6aa --- /dev/null +++ b/cc/toolchains/impl/args_utils.bzl @@ -0,0 +1,30 @@ +# Copyright 2024 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +""".""" + +def get_action_type(args_list, action_type): + """Returns the corresponding entry in ArgsListInfo.by_action. + + Args: + args_list: (ArgsListInfo) The args list to look through + action_type: (ActionTypeInfo) The action type to look up. + Returns: + The information corresponding to this action type. + + """ + for args in args_list.by_action: + if args.action == action_type: + return args + + return struct(action = action_type, args = tuple(), files = depset([])) diff --git a/cc/toolchains/impl/collect.bzl b/cc/toolchains/impl/collect.bzl index e1c508f..5865d27 100644 --- a/cc/toolchains/impl/collect.bzl +++ b/cc/toolchains/impl/collect.bzl @@ -15,8 +15,10 @@ load( "//cc/toolchains:cc_toolchain_info.bzl", + "ActionTypeConfigSetInfo", "ActionTypeSetInfo", "ArgsListInfo", + "FeatureSetInfo", "ToolInfo", ) @@ -57,6 +59,7 @@ def _make_collector(provider, field): return collector collect_action_types = _make_collector(ActionTypeSetInfo, "actions") +collect_features = _make_collector(FeatureSetInfo, "features") collect_files = _make_collector(DefaultInfo, "files") def collect_data(ctx, targets): @@ -145,3 +148,22 @@ def collect_args_lists(targets, label): for k, v in by_action.items() ]), ) + +def collect_action_type_config_sets(targets, label, fail = fail): + """Collects several `cc_action_type_config` labels together. + + Args: + targets: (List[Target]) A list of targets providing ActionTypeConfigSetInfo + label: The label to apply to the resulting config. + fail: (function) The fail function. Should only be used in tests. + Returns: + A combined ActionTypeConfigSetInfo representing a variety of action + types. + """ + configs = {} + for atcs in collect_provider(targets, ActionTypeConfigSetInfo): + for action_type, config in atcs.configs.items(): + if action_type in configs: + fail("The action type %s is configured by both %s and %s. Each action type may only be configured once." % (action_type.label, config.label, configs[action_type].label)) + configs[action_type] = config + return ActionTypeConfigSetInfo(label = label, configs = configs) diff --git a/tests/rule_based_toolchain/action_type_config/BUILD b/tests/rule_based_toolchain/action_type_config/BUILD new file mode 100644 index 0000000..b2fe529 --- /dev/null +++ b/tests/rule_based_toolchain/action_type_config/BUILD @@ -0,0 +1,42 @@ +load("@rules_testing//lib:util.bzl", "util") +load("//cc/toolchains:action_type_config.bzl", "cc_action_type_config") +load("//tests/rule_based_toolchain:analysis_test_suite.bzl", "analysis_test_suite") +load(":action_type_config_test.bzl", "TARGETS", "TESTS") + +util.helper_target( + cc_action_type_config, + name = "file_map", + action_types = ["//tests/rule_based_toolchain/actions:all_compile"], + additional_files = [ + "//tests/rule_based_toolchain/testdata:multiple2", + ], + args = ["//tests/rule_based_toolchain/args_list"], + tools = [ + "//tests/rule_based_toolchain/testdata:bin_wrapper.sh", + "//tests/rule_based_toolchain/tool:wrapped_tool", + ], +) + +util.helper_target( + cc_action_type_config, + name = "c_compile_config", + action_types = ["//tests/rule_based_toolchain/actions:c_compile"], + tools = [ + "//tests/rule_based_toolchain/testdata:bin_wrapper.sh", + ], +) + +util.helper_target( + cc_action_type_config, + name = "cpp_compile_config", + action_types = ["//tests/rule_based_toolchain/actions:cpp_compile"], + tools = [ + "//tests/rule_based_toolchain/testdata:bin_wrapper.sh", + ], +) + +analysis_test_suite( + name = "test_suite", + targets = TARGETS, + tests = TESTS, +) 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 new file mode 100644 index 0000000..7ee85e6 --- /dev/null +++ b/tests/rule_based_toolchain/action_type_config/action_type_config_test.bzl @@ -0,0 +1,108 @@ +# Copyright 2024 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""Tests for the action_type_config rule.""" + +load( + "//cc/toolchains:cc_toolchain_info.bzl", + "ActionTypeConfigSetInfo", + "ActionTypeInfo", +) +load("//cc/toolchains/impl:collect.bzl", _collect_action_type_configs = "collect_action_type_config_sets") +load("//tests/rule_based_toolchain:subjects.bzl", "result_fn_wrapper", "subjects") + +visibility("private") + +_TOOL_FILES = [ + "tests/rule_based_toolchain/testdata/bin", + "tests/rule_based_toolchain/testdata/bin_wrapper", + "tests/rule_based_toolchain/testdata/bin_wrapper.sh", +] +_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, + ]) + + 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, + ]) + +def _merge_distinct_configs_succeeds_test(env, targets): + configs = env.expect.that_value( + collect_action_type_configs( + targets = [targets.c_compile_config, targets.cpp_compile_config], + label = env.ctx.label, + ), + factory = subjects.result(subjects.ActionTypeConfigSetInfo), + ).ok().configs() + configs.get(targets.c_compile[ActionTypeInfo]).label().equals( + targets.c_compile_config.label, + ) + configs.get(targets.cpp_compile[ActionTypeInfo]).label().equals( + targets.cpp_compile_config.label, + ) + +def _merge_overlapping_configs_fails_test(env, targets): + err = env.expect.that_value( + collect_action_type_configs( + targets = [targets.file_map, targets.c_compile_config], + label = env.ctx.label, + ), + factory = subjects.result(subjects.ActionTypeConfigSetInfo), + ).err() + err.contains("//tests/rule_based_toolchain/actions:c_compile is configured by both") + err.contains("//tests/rule_based_toolchain/action_type_config:c_compile_config") + err.contains("//tests/rule_based_toolchain/action_type_config:file_map") + +TARGETS = [ + ":file_map", + ":c_compile_config", + ":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 = { + "files_taken_test": _files_taken_test, + "merge_distinct_configs_succeeds_test": _merge_distinct_configs_succeeds_test, + "merge_overlapping_configs_fails_test": _merge_overlapping_configs_fails_test, +} diff --git a/tests/rule_based_toolchain/args_list/BUILD b/tests/rule_based_toolchain/args_list/BUILD index 4120d28..fe62d06 100644 --- a/tests/rule_based_toolchain/args_list/BUILD +++ b/tests/rule_based_toolchain/args_list/BUILD @@ -10,6 +10,7 @@ util.helper_target( actions = ["//tests/rule_based_toolchain/actions:c_compile"], additional_files = ["//tests/rule_based_toolchain/testdata:file1"], args = ["c"], + visibility = ["//tests/rule_based_toolchain:__subpackages__"], ) util.helper_target( @@ -18,6 +19,7 @@ util.helper_target( actions = ["//tests/rule_based_toolchain/actions:cpp_compile"], additional_files = ["//tests/rule_based_toolchain/testdata:file2"], args = ["cpp"], + visibility = ["//tests/rule_based_toolchain:__subpackages__"], ) util.helper_target( @@ -26,6 +28,7 @@ util.helper_target( actions = ["//tests/rule_based_toolchain/actions:all_compile"], additional_files = ["//tests/rule_based_toolchain/testdata:multiple1"], args = ["all"], + visibility = ["//tests/rule_based_toolchain:__subpackages__"], ) util.helper_target( @@ -36,6 +39,7 @@ util.helper_target( ":cpp_compile_args", ":all_compile_args", ], + visibility = ["//tests/rule_based_toolchain:__subpackages__"], ) analysis_test_suite( diff --git a/tests/rule_based_toolchain/subjects.bzl b/tests/rule_based_toolchain/subjects.bzl index fbda2d9..5fcb8a1 100644 --- a/tests/rule_based_toolchain/subjects.bzl +++ b/tests/rule_based_toolchain/subjects.bzl @@ -17,8 +17,8 @@ load("@bazel_skylib//lib:structs.bzl", "structs") load("@rules_testing//lib:truth.bzl", _subjects = "subjects") load( "//cc/toolchains:cc_toolchain_info.bzl", - "ActionConfigInfo", - "ActionConfigSetInfo", + "ActionTypeConfigInfo", + "ActionTypeConfigSetInfo", "ActionTypeInfo", "ActionTypeSetInfo", "AddArgsInfo", @@ -163,38 +163,25 @@ _ToolFactory = generate_factory( ) # buildifier: disable=name-conventions -_ActionConfigFactory = generate_factory( - ActionConfigInfo, - "ActionConfigInfo", +_ActionTypeConfigFactory = generate_factory( + ActionTypeConfigInfo, + "ActionTypeConfigInfo", dict( - action = _ActionTypeFactory, - enabled = _subjects.bool, + action_type = _ActionTypeFactory, tools = ProviderSequence(_ToolFactory), args = ProviderSequence(_ArgsFactory), implies = ProviderDepset(_FeatureFactory), - files = _subjects.depset_file, + files = runfiles_subject, ), ) -def _action_config_set_factory_impl(value, *, meta): - # We can't use the usual strategy of "inline the labels" since all labels - # are the same. - transformed = {} - for ac in value.action_configs.to_list(): - key = ac.action.label - if key in transformed: - meta.add_failure("Action declared twice in action config", key) - transformed[key] = _ActionConfigFactory.factory( - value = ac, - meta = meta.derive(".get({})".format(key)), - ) - return transformed - # buildifier: disable=name-conventions -_ActionConfigSetFactory = struct( - type = ActionConfigSetInfo, - name = "ActionConfigSetInfo", - factory = _action_config_set_factory_impl, +_ActionTypeConfigSetFactory = generate_factory( + ActionTypeConfigSetInfo, + "ActionTypeConfigSetInfo", + dict( + configs = dict_key_subject(_ActionTypeConfigFactory.factory), + ), ) FACTORIES = [ @@ -208,7 +195,7 @@ FACTORIES = [ _FeatureConstraintFactory, _FeatureSetFactory, _ToolFactory, - _ActionConfigSetFactory, + _ActionTypeConfigSetFactory, ] result_fn_wrapper = _result_fn_wrapper diff --git a/tests/rule_based_toolchain/tool/BUILD b/tests/rule_based_toolchain/tool/BUILD index a6b01ea..de026f8 100644 --- a/tests/rule_based_toolchain/tool/BUILD +++ b/tests/rule_based_toolchain/tool/BUILD @@ -15,12 +15,7 @@ util.helper_target( cc_tool, name = "wrapped_tool", executable = "//tests/rule_based_toolchain/testdata:bin_wrapper", -) - -util.helper_target( - cc_tool, - name = "data_with_runfiles", - executable = "//tests/rule_based_toolchain/testdata:file1", + visibility = ["//tests/rule_based_toolchain:__subpackages__"], ) analysis_test_suite(