From 123601af6efe96caf860aa55cfa8bd447dab6b84 Mon Sep 17 00:00:00 2001 From: David Zbarsky Date: Sun, 8 Feb 2026 07:54:03 -0500 Subject: [PATCH] Allow passing env to cc_tool --- cc/toolchains/cc_toolchain_info.bzl | 1 + cc/toolchains/impl/collect.bzl | 1 + cc/toolchains/impl/legacy_converter.bzl | 21 +++- cc/toolchains/tool.bzl | 108 +++++++++++++++++- docs/toolchain_api.md | 98 ++++++++-------- tests/rule_based_toolchain/subjects.bzl | 1 + tests/rule_based_toolchain/tool/BUILD | 17 +++ tests/rule_based_toolchain/tool/tool_test.bzl | 9 ++ .../toolchain_config/BUILD | 22 ++++ .../toolchain_config_test.bzl | 32 ++++++ 10 files changed, 261 insertions(+), 49 deletions(-) diff --git a/cc/toolchains/cc_toolchain_info.bzl b/cc/toolchains/cc_toolchain_info.bzl index 38148b463..ea6f31b7c 100644 --- a/cc/toolchains/cc_toolchain_info.bzl +++ b/cc/toolchains/cc_toolchain_info.bzl @@ -198,6 +198,7 @@ ToolInfo = provider( "exe": "(File) The file corresponding to the tool", "runfiles": "(runfiles) The files required to run the tool", "execution_requirements": "(Sequence[str]) A set of execution requirements of the tool", + "env": "(dict[str, str]) Environment variables applied when using this tool", "allowlist_include_directories": "(depset[DirectoryInfo]) Built-in include directories implied by this tool that should be allowlisted in Bazel's include checker", "capabilities": "(Sequence[ToolCapabilityInfo]) Capabilities supported by the tool.", }, diff --git a/cc/toolchains/impl/collect.bzl b/cc/toolchains/impl/collect.bzl index 671bb65b4..b4921b625 100644 --- a/cc/toolchains/impl/collect.bzl +++ b/cc/toolchains/impl/collect.bzl @@ -106,6 +106,7 @@ def collect_tools(ctx, targets, fail = fail): exe = info.files_to_run.executable, runfiles = collect_data(ctx, [target]), execution_requirements = tuple(), + env = {}, allowlist_include_directories = depset(), capabilities = tuple(), )) diff --git a/cc/toolchains/impl/legacy_converter.bzl b/cc/toolchains/impl/legacy_converter.bzl index 5e8a14481..370ba5c4d 100644 --- a/cc/toolchains/impl/legacy_converter.bzl +++ b/cc/toolchains/impl/legacy_converter.bzl @@ -102,6 +102,18 @@ def _convert_args_sequence(args_sequence, strip_actions = False): return struct(flag_sets = flag_sets, env_sets = env_sets) +def _convert_tool_env(tool, action_name): + if not tool.env: + return None + + return legacy_env_set( + actions = [action_name], + env_entries = [ + legacy_env_entry(key = key, value = value) + for key, value in sorted(tool.env.items()) + ], + ) + def convert_feature(feature, enabled = False): if feature.external: return None @@ -188,6 +200,13 @@ def convert_toolchain(toolchain): args.flag_sets.extend(new_args.flag_sets) args.env_sets.extend(new_args.env_sets) + toolchain_args = _convert_args_sequence(toolchain.args.args) + tool_env_sets = [] + for action_type in sorted(toolchain.tool_map.configs.keys(), key = lambda action: action.name): + env_set = _convert_tool_env(toolchain.tool_map.configs[action_type], action_type.name) + if env_set: + tool_env_sets.append(env_set) + action_configs, cap_features = _convert_tool_map(toolchain.tool_map, args_by_action) features = [ convert_feature(feature, enabled = feature in toolchain.enabled_features) @@ -200,7 +219,7 @@ def convert_toolchain(toolchain): # conflict with the name of a feature the user creates. name = "implied_by_always_enabled_env_sets", enabled = True, - env_sets = _convert_args_sequence(toolchain.args.args).env_sets, + env_sets = tool_env_sets + toolchain_args.env_sets, )) cxx_builtin_include_directories = [ diff --git a/cc/toolchains/tool.bzl b/cc/toolchains/tool.bzl index 3ee1cd5a5..1c247fcd2 100644 --- a/cc/toolchains/tool.bzl +++ b/cc/toolchains/tool.bzl @@ -15,6 +15,7 @@ load("@bazel_skylib//rules/directory:providers.bzl", "DirectoryInfo") load("//cc/toolchains/impl:collect.bzl", "collect_data", "collect_provider") +load("//cc/toolchains/impl:nested_args.bzl", "format_dict_values") load( ":cc_toolchain_info.bzl", "ToolCapabilityInfo", @@ -30,12 +31,20 @@ def _cc_tool_impl(ctx): else: fail("Expected cc_tool's src attribute to be either an executable or a single file") + format_targets = {k: v for v, k in ctx.attr.format.items()} + env, _ = format_dict_values( + env = ctx.attr.env, + format = format_targets, + must_use = format_targets.keys(), + ) + runfiles = collect_data(ctx, ctx.attr.data + [ctx.attr.src]) tool = ToolInfo( label = ctx.label, exe = exe, runfiles = runfiles, execution_requirements = tuple(ctx.attr.tags), + env = env, allowlist_include_directories = depset( direct = [d[DirectoryInfo] for d in ctx.attr.allowlist_include_directories], ), @@ -59,7 +68,7 @@ def _cc_tool_impl(ctx): ), ] -cc_tool = rule( +_cc_tool = rule( implementation = _cc_tool_impl, # @unsorted-dict-items attrs = { @@ -98,6 +107,20 @@ add them to 'data' as well. This can help work around errors like: `the source file 'main.c' includes the following non-builtin files with absolute paths (if these are builtin files, make sure these paths are in your toolchain)`. +""", + ), + "env": attr.string_dict( + doc = """Environment variables to apply when running this tool. + +Format expansion is performed on values using the format attribute. +""", + ), + "format": attr.label_keyed_string_dict( + allow_files = True, + doc = """Variables to be used in substitutions for env values. + +The keys are targets and the values are format strings. Use the cc_tool macro +to provide the inverted mapping from format string to target. """, ), "capabilities": attr.label_list( @@ -136,3 +159,86 @@ cc_tool( """, executable = True, ) + +def cc_tool( + *, + name, + src = None, + data = None, + allowlist_include_directories = None, + env = None, + format = {}, + capabilities = None, + **kwargs): + """Declares a tool for use by toolchain actions. + + `cc_tool` rules are used in a `cc_tool_map` rule to ensure all files and + metadata required to run a tool are available when constructing a `cc_toolchain`. + + In general, include all files that are always required to run a tool (e.g. libexec/** and + cross-referenced tools in bin/*) in the [data](#cc_tool-data) attribute. If some files are only + required when certain flags are passed to the tool, consider using a `cc_args` rule to + bind the files to the flags that require them. This reduces the overhead required to properly + enumerate a sandbox with all the files required to run a tool, and ensures that there isn't + unintentional leakage across configurations and actions. + + Example: + ``` + load("//cc/toolchains:tool.bzl", "cc_tool") + + cc_tool( + name = "clang", + src = "@llvm_toolchain//:bin/clang", + # Suppose clang needs libc to run. + data = ["@llvm_toolchain//:lib/x86_64-linux-gnu/libc.so.6"] + capabilities = ["//cc/toolchains/capabilities:supports_pic"], + ) + ``` + + Args: + name: (str) The name of the target. + src: (Label) The underlying binary that this tool represents. + Usually just a single prebuilt (e.g. @toolchain//:bin/clang), but may be any + executable label. + data: (List[Label]) Additional files that are required for this tool to run. + Frequently, clang and gcc require additional files to execute as they often shell out to + other binaries (e.g. `cc1`). + allowlist_include_directories: (List[Label]) Include paths implied by using this tool. + Compilers may include a set of built-in headers that are implicitly available + unless flags like `-nostdinc` are provided. Bazel checks that all included + headers are properly provided by a dependency or allowlisted through this + mechanism. + + As a rule of thumb, only use this if Bazel is complaining about absolute paths in your + toolchain and you've ensured that the toolchain is compiling with the + `-no-canonical-prefixes` and/or `-fno-canonical-system-headers` arguments. + + These files are not automatically passed to each action. If they need to be, + add them to 'data' as well. + + This can help work around errors like: + `the source file 'main.c' includes the following non-builtin files with absolute paths + (if these are builtin files, make sure these paths are in your toolchain)`. + env: (Dict[str, str]) Environment variables to apply when running this tool. + Format expansion is performed on values using `format`. + format: (Dict[str, Label]) A mapping of format strings to the label of a corresponding + target. This target can be a `directory`, `subdirectory`, or a single file that the + value should be pulled from. All instances of `{variable_name}` in the `env` dictionary + values will be replaced with the expanded value in this dictionary. + capabilities: (List[Label]) Declares that a tool is capable of doing something. + For example, `@rules_cc//cc/toolchains/capabilities:supports_pic`. + **kwargs: [common attributes](https://bazel.build/reference/be/common-definitions#common-attributes) + that should be applied to this rule. + """ + return _cc_tool( + name = name, + src = src, + data = data, + allowlist_include_directories = allowlist_include_directories, + env = env, + # We flip the key/value pairs in the dictionary here because Bazel doesn't have a + # string-keyed label dict attribute type. + format = {k: v for v, k in format.items()}, + capabilities = capabilities, + **kwargs + ) diff --git a/docs/toolchain_api.md b/docs/toolchain_api.md index eee9241f1..73579d25d 100755 --- a/docs/toolchain_api.md +++ b/docs/toolchain_api.md @@ -474,53 +474,6 @@ cc_feature( | name | A unique name for this target. | Name | required | | - - -## cc_tool - -
-load("@rules_cc//cc/toolchains/impl:documented_api.bzl", "cc_tool")
-
-cc_tool(name, src, data, allowlist_include_directories, capabilities)
-
- -Declares a tool for use by toolchain actions. - -[`cc_tool`](#cc_tool) rules are used in a [`cc_tool_map`](#cc_tool_map) rule to ensure all files and -metadata required to run a tool are available when constructing a [`cc_toolchain`](#cc_toolchain). - -In general, include all files that are always required to run a tool (e.g. libexec/** and -cross-referenced tools in bin/*) in the [data](#cc_tool-data) attribute. If some files are only -required when certain flags are passed to the tool, consider using a [`cc_args`](#cc_args) rule to -bind the files to the flags that require them. This reduces the overhead required to properly -enumerate a sandbox with all the files required to run a tool, and ensures that there isn't -unintentional leakage across configurations and actions. - -Example: -``` -load("@rules_cc//cc/toolchains:tool.bzl", "cc_tool") - -cc_tool( - name = "clang", - src = "@llvm_toolchain//:bin/clang", - # Suppose clang needs libc to run. - data = ["@llvm_toolchain//:lib/x86_64-linux-gnu/libc.so.6"] - capabilities = ["@rules_cc//cc/toolchains/capabilities:supports_pic"], -) -``` - -**ATTRIBUTES** - - -| Name | Description | Type | Mandatory | Default | -| :------------- | :------------- | :------------- | :------------- | :------------- | -| name | A unique name for this target. | Name | required | | -| src | The underlying binary that this tool represents.

Usually just a single prebuilt (eg. @toolchain//:bin/clang), but may be any executable label. | Label | optional | `None` | -| data | Additional files that are required for this tool to run.

Frequently, clang and gcc require additional files to execute as they often shell out to other binaries (e.g. `cc1`). | List of labels | optional | `[]` | -| allowlist_include_directories | Include paths implied by using this tool.

Compilers may include a set of built-in headers that are implicitly available unless flags like `-nostdinc` are provided. Bazel checks that all included headers are properly provided by a dependency or allowlisted through this mechanism.

As a rule of thumb, only use this if Bazel is complaining about absolute paths in your toolchain and you've ensured that the toolchain is compiling with the `-no-canonical-prefixes` and/or `-fno-canonical-system-headers` arguments.

These files are not automatically passed to each action. If they need to be, add them to 'data' as well.

This can help work around errors like: `the source file 'main.c' includes the following non-builtin files with absolute paths (if these are builtin files, make sure these paths are in your toolchain)`. | List of labels | optional | `[]` | -| capabilities | Declares that a tool is capable of doing something.

For example, `@rules_cc//cc/toolchains/capabilities:supports_pic`. | List of labels | optional | `[]` | - - ## cc_tool_capability @@ -725,6 +678,57 @@ use this rule. | kwargs | [common attributes](https://bazel.build/reference/be/common-definitions#common-attributes) that should be applied to this rule. | none | + + +## cc_tool + +
+load("@rules_cc//cc/toolchains/impl:documented_api.bzl", "cc_tool")
+
+cc_tool(*, name, src, data, allowlist_include_directories, env, format, capabilities, **kwargs)
+
+ +Declares a tool for use by toolchain actions. + +[`cc_tool`](#cc_tool) rules are used in a [`cc_tool_map`](#cc_tool_map) rule to ensure all files and +metadata required to run a tool are available when constructing a [`cc_toolchain`](#cc_toolchain). + +In general, include all files that are always required to run a tool (e.g. libexec/** and +cross-referenced tools in bin/*) in the [data](#cc_tool-data) attribute. If some files are only +required when certain flags are passed to the tool, consider using a [`cc_args`](#cc_args) rule to +bind the files to the flags that require them. This reduces the overhead required to properly +enumerate a sandbox with all the files required to run a tool, and ensures that there isn't +unintentional leakage across configurations and actions. + +Example: +``` +load("@rules_cc//cc/toolchains:tool.bzl", "cc_tool") + +cc_tool( + name = "clang", + src = "@llvm_toolchain//:bin/clang", + # Suppose clang needs libc to run. + data = ["@llvm_toolchain//:lib/x86_64-linux-gnu/libc.so.6"] + capabilities = ["@rules_cc//cc/toolchains/capabilities:supports_pic"], +) +``` + + +**PARAMETERS** + + +| Name | Description | Default Value | +| :------------- | :------------- | :------------- | +| name | (str) The name of the target. | none | +| src | (Label) The underlying binary that this tool represents. Usually just a single prebuilt (e.g. @toolchain//:bin/clang), but may be any executable label. | `None` | +| data | (List[Label]) Additional files that are required for this tool to run. Frequently, clang and gcc require additional files to execute as they often shell out to other binaries (e.g. `cc1`). | `None` | +| allowlist_include_directories | (List[Label]) Include paths implied by using this tool. Compilers may include a set of built-in headers that are implicitly available unless flags like `-nostdinc` are provided. Bazel checks that all included headers are properly provided by a dependency or allowlisted through this mechanism.

As a rule of thumb, only use this if Bazel is complaining about absolute paths in your toolchain and you've ensured that the toolchain is compiling with the `-no-canonical-prefixes` and/or `-fno-canonical-system-headers` arguments.

These files are not automatically passed to each action. If they need to be, add them to 'data' as well.

This can help work around errors like: `the source file 'main.c' includes the following non-builtin files with absolute paths (if these are builtin files, make sure these paths are in your toolchain)`. | `None` | +| env | (Dict[str, str]) Environment variables to apply when running this tool. Format expansion is performed on values using `format`. | `None` | +| format | (Dict[str, Label]) A mapping of format strings to the label of a corresponding target. This target can be a `directory`, `subdirectory`, or a single file that the value should be pulled from. All instances of `{variable_name}` in the `env` dictionary values will be replaced with the expanded value in this dictionary. | `{}` | +| capabilities | (List[Label]) Declares that a tool is capable of doing something. For example, `@rules_cc//cc/toolchains/capabilities:supports_pic`. | `None` | +| kwargs | [common attributes](https://bazel.build/reference/be/common-definitions#common-attributes) that should be applied to this rule. | none | + + ## cc_tool_map diff --git a/tests/rule_based_toolchain/subjects.bzl b/tests/rule_based_toolchain/subjects.bzl index 79e6f1b50..f24f94d4b 100644 --- a/tests/rule_based_toolchain/subjects.bzl +++ b/tests/rule_based_toolchain/subjects.bzl @@ -210,6 +210,7 @@ _ToolFactory = generate_factory( exe = _subjects.file, runfiles = runfiles_subject, execution_requirements = _subjects.collection, + env = _subjects.dict, allowlist_include_directories = _FakeDirectoryDepset, capabilities = ProviderSequence(_ToolCapabilityFactory), ), diff --git a/tests/rule_based_toolchain/tool/BUILD b/tests/rule_based_toolchain/tool/BUILD index 8b1c07303..3c0257ebe 100644 --- a/tests/rule_based_toolchain/tool/BUILD +++ b/tests/rule_based_toolchain/tool/BUILD @@ -25,6 +25,23 @@ cc_tool( visibility = ["//tests/rule_based_toolchain:__subpackages__"], ) +cc_tool( + name = "tool_with_env", + src = "//tests/rule_based_toolchain/testdata:bin_wrapper.sh", + data = [ + "//tests/rule_based_toolchain/testdata:bin", + "//tests/rule_based_toolchain/testdata:file1", + ], + env = { + "STATIC": "value", + "TOOL_ENV": "{tool_env}", + }, + format = { + "tool_env": "//tests/rule_based_toolchain/testdata:file1", + }, + visibility = ["//tests/rule_based_toolchain:__subpackages__"], +) + cc_directory_tool( name = "directory_tool", data = ["bin"], diff --git a/tests/rule_based_toolchain/tool/tool_test.bzl b/tests/rule_based_toolchain/tool/tool_test.bzl index ebc516461..051ae9a48 100644 --- a/tests/rule_based_toolchain/tool/tool_test.bzl +++ b/tests/rule_based_toolchain/tool/tool_test.bzl @@ -65,6 +65,13 @@ def _tool_with_allowlist_include_directories_test(env, targets): _FILE1, ]) +def _tool_env_expansion_test(env, targets): + tool = env.expect.that_target(targets.tool_with_env).provider(ToolInfo) + tool.env().contains_exactly({ + "STATIC": "value", + "TOOL_ENV": "tests/rule_based_toolchain/testdata/file1", + }) + def _collect_tools_collects_tools_test(env, targets): env.expect.that_value( value = collect_tools(env.ctx, [targets.tool, targets.wrapped_tool]), @@ -108,6 +115,7 @@ TARGETS = [ "//tests/rule_based_toolchain/tool:wrapped_tool", "//tests/rule_based_toolchain/tool:directory_tool", "//tests/rule_based_toolchain/tool:tool_with_allowlist_include_directories", + "//tests/rule_based_toolchain/tool:tool_with_env", "//tests/rule_based_toolchain/testdata:bin_wrapper", "//tests/rule_based_toolchain/testdata:multiple", "//tests/rule_based_toolchain/testdata:bin_filegroup", @@ -124,4 +132,5 @@ TESTS = { "collect_tools_collects_single_files_test": _collect_tools_collects_single_files_test, "collect_tools_fails_on_non_binary_test": _collect_tools_fails_on_non_binary_test, "tool_with_allowlist_include_directories_test": _tool_with_allowlist_include_directories_test, + "tool_env_expansion_test": _tool_env_expansion_test, } diff --git a/tests/rule_based_toolchain/toolchain_config/BUILD b/tests/rule_based_toolchain/toolchain_config/BUILD index 7bf8d3b43..0847dec15 100644 --- a/tests/rule_based_toolchain/toolchain_config/BUILD +++ b/tests/rule_based_toolchain/toolchain_config/BUILD @@ -46,6 +46,13 @@ util.helper_target( env = {"CPP_COMPILE": "1"}, ) +util.helper_target( + cc_args, + name = "c_compile_env_args", + actions = ["//tests/rule_based_toolchain/actions:c_compile"], + env = {"FROM_ARGS": "args-env"}, +) + cc_tool( name = "c_compile_tool", src = "//tests/rule_based_toolchain/testdata:bin_wrapper", @@ -113,6 +120,14 @@ util.helper_target( }, ) +util.helper_target( + cc_tool_map, + name = "tool_env_tool_map", + tools = { + "//tests/rule_based_toolchain/actions:c_compile": "//tests/rule_based_toolchain/tool:tool_with_env", + }, +) + util.helper_target( cc_feature, name = "compile_feature", @@ -199,6 +214,13 @@ util.helper_target( visibility = ["//tests/rule_based_toolchain:__subpackages__"], ) +util.helper_target( + cc_toolchain_config, + name = "tool_env_toolchain_config", + args = [":c_compile_env_args"], + tool_map = ":tool_env_tool_map", +) + analysis_test_suite( name = "test_suite", targets = TARGETS, 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 1047203c4..9cca3df74 100644 --- a/tests/rule_based_toolchain/toolchain_config/toolchain_config_test.bzl +++ b/tests/rule_based_toolchain/toolchain_config/toolchain_config_test.bzl @@ -16,6 +16,8 @@ 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", @@ -258,6 +260,32 @@ def _toolchain_collects_files_test(env, targets): ), ]).in_order() +def _tool_env_wires_into_toolchain_test(env, targets): + tc = env.expect.that_target(targets.tool_env_toolchain_config).provider(ToolchainConfigInfo) + legacy = convert_toolchain(tc.actual) + + env.expect.that_collection(legacy.features).contains_exactly([ + legacy_feature( + name = "implied_by_always_enabled_env_sets", + enabled = True, + env_sets = [ + legacy_env_set( + actions = ["c_compile"], + env_entries = [ + legacy_env_entry(key = "STATIC", value = "value"), + legacy_env_entry(key = "TOOL_ENV", value = "tests/rule_based_toolchain/testdata/file1"), + ], + ), + legacy_env_set( + actions = ["c_compile"], + env_entries = [ + legacy_env_entry(key = "FROM_ARGS", value = "args-env"), + ], + ), + ], + ), + ]) + TARGETS = [ "//tests/rule_based_toolchain/actions:c_compile", "//tests/rule_based_toolchain/actions:cpp_compile", @@ -269,6 +297,9 @@ TARGETS = [ ":compile_feature", ":c_compile_args", ":c_compile_tool_map", + ":tool_env_toolchain_config", + ":tool_env_tool_map", + ":c_compile_env_args", ":empty_tool_map", ":implies_simple_feature", ":overrides_feature", @@ -289,4 +320,5 @@ TESTS = { "feature_missing_requirements_invalid_test": _feature_missing_requirements_invalid_test, "args_missing_requirements_invalid_test": _args_missing_requirements_invalid_test, "toolchain_collects_files_test": _toolchain_collects_files_test, + "tool_env_wires_into_toolchain_test": _tool_env_wires_into_toolchain_test, }