Merge pull request #256 from fmeum:235-better-tool-map

PiperOrigin-RevId: 691529088
Change-Id: Idbda52a6b2eecbdefd83b2c89a0c7eb01dfee074

cc/toolchains/tool_map.bzl

@@ -32,7 +32,7 @@ def _cc_tool_map_impl(ctx):
     action_to_as = {}
     for i in range(len(action_sets)):
         action_set = action_sets[i]
-        tool = tools[i]
+        tool = tools[ctx.attr.tool_index_for_action[i]]
 
         for action in action_set.actions.to_list():
             if action in action_to_as:
@@ -63,6 +63,10 @@ See //cc/toolchains/actions:BUILD for valid options.
 The tool may be a `cc_tool` or other executable rule.
 """,
         ),
+        "tool_index_for_action": attr.int_list(
+            mandatory = True,
+            doc = """The index of the tool in `tools` for the action in `actions`.""",
+        ),
     },
     provides = [ToolConfigInfo],
 )
@@ -100,11 +104,6 @@ def cc_tool_map(name, tools, **kwargs):
     )
     ```
 
-    Note:
-       Due to an implementation limitation, if you need to map the same tool to multiple actions,
-       you will need to create an intermediate alias for the tool for each set of actions. See
-       https://github.com/bazelbuild/rules_cc/issues/235 for more details.
-
     Args:
         name: (str) The name of the target.
         tools: (Dict[Label, Label]) A mapping between
@@ -112,9 +111,20 @@ def cc_tool_map(name, tools, **kwargs):
             and the `cc_tool` or executable target that implements that action.
         **kwargs: [common attributes](https://bazel.build/reference/be/common-definitions#common-attributes) that should be applied to this rule.
     """
+    actions = []
+    tool_index_for_action = []
+    deduplicated_tools = {}
+    for action, tool in tools.items():
+        actions.append(action)
+        label = native.package_relative_label(tool)
+        if label not in deduplicated_tools:
+            deduplicated_tools[label] = len(deduplicated_tools)
+        tool_index_for_action.append(deduplicated_tools[label])
+
     _cc_tool_map(
         name = name,
-        actions = tools.keys(),
+        actions = actions,
-        tools = tools.values(),
+        tools = deduplicated_tools.keys(),
+        tool_index_for_action = tool_index_for_action,
         **kwargs
     )

docs/toolchain_api.md

@@ -657,11 +657,6 @@ cc_tool_map(
 )
 ```
 
-Note:
-   Due to an implementation limitation, if you need to map the same tool to multiple actions,
-   you will need to create an intermediate alias for the tool for each set of actions. See
-   https://github.com/bazelbuild/rules_cc/issues/235 for more details.
-
 
 **PARAMETERS**
 

examples/rule_based_toolchain/toolchain/tools/BUILD.bazel

@@ -30,7 +30,7 @@ alias(
 )
 
 COMMON_TOOLS = {
-    "@rules_cc//cc/toolchains/actions:assembly_actions": ":asm",
+    "@rules_cc//cc/toolchains/actions:assembly_actions": ":clang",
     "@rules_cc//cc/toolchains/actions:c_compile": ":clang",
     "@rules_cc//cc/toolchains/actions:cpp_compile_actions": ":clang++",
     "@rules_cc//cc/toolchains/actions:link_actions": ":lld",
@@ -54,13 +54,6 @@ cc_tool_map(
     visibility = ["//visibility:private"],
 )
 
-# TODO: https://github.com/bazelbuild/rules_cc/issues/235 - Workaround until
-# Bazel has a more robust way to implement `cc_tool_map`.
-alias(
-    name = "asm",
-    actual = ":clang",
-)
-
 cc_tool(
     name = "clang",
     src = select({

tests/rule_based_toolchain/tool_map/BUILD

@@ -1,9 +1,9 @@
 load(
     ":tool_map_test.bzl",
-    "duplicate_tool_test",
+    "duplicate_action_test",
     "valid_config_test",
 )
 
-duplicate_tool_test(name = "duplicate_tool_test")
+duplicate_action_test(name = "duplicate_action_test")
 
 valid_config_test(name = "valid_config_test")

tests/rule_based_toolchain/tool_map/tool_map_test.bzl

@@ -23,6 +23,7 @@ _C_COMPILE = "//cc/toolchains/actions:c_compile"
 _CPP_COMPILE = "//cc/toolchains/actions:cpp_compile"
 _ALL_CPP_COMPILE = "//cc/toolchains/actions:cpp_compile_actions"
 _STRIP = "//cc/toolchains/actions:strip"
+_LINK_DYNAMIC_LIBRARY = "//cc/toolchains/actions:cpp_link_executable"
 _BIN = "//tests/rule_based_toolchain/testdata:bin"
 _BIN_WRAPPER = "//tests/rule_based_toolchain/testdata:bin_wrapper"
 
@@ -31,6 +32,7 @@ def valid_config_test(name):
     cc_tool_map(
         name = subject_name,
         tools = {
+            _LINK_DYNAMIC_LIBRARY: _BIN,
             _C_COMPILE: _BIN_WRAPPER,
             _ALL_CPP_COMPILE: _BIN,
         },
@@ -42,6 +44,7 @@ def valid_config_test(name):
         targets = {
             "c_compile": _C_COMPILE,
             "cpp_compile": _CPP_COMPILE,
+            "link_dynamic_library": _LINK_DYNAMIC_LIBRARY,
             "strip": _STRIP,
             "subject": subject_name,
         },
@@ -53,8 +56,9 @@ def _valid_config_test_impl(env, targets):
     configs.contains(targets.strip[ActionTypeInfo]).equals(False)
     configs.get(targets.c_compile[ActionTypeInfo]).exe().path().split("/").offset(-1, subjects.str).equals("bin_wrapper")
     configs.get(targets.cpp_compile[ActionTypeInfo]).exe().path().split("/").offset(-1, subjects.str).equals("bin")
+    configs.get(targets.link_dynamic_library[ActionTypeInfo]).exe().path().split("/").offset(-1, subjects.str).equals("bin")
 
-def duplicate_tool_test(name):
+def duplicate_action_test(name):
     subject_name = "_%s_subject" % name
     helper_target(
         cc_tool_map,