pw_toolchain_bazel: Pull file collection into config rule
This allows us to actually write tests for the file collection logic.
It also allows us to significantly simplify our logic, since it's now
centralized.
Bug: 322872628
Test: bazel build //cc_toolchain/tests/...
Change-Id: I31ab518158dabceb65aeaf57f40a57fcd1544546
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/189971
Reviewed-by: Armando Montanez <amontanez@google.com>
Commit-Queue: Matt Stark <msta@google.com>
diff --git a/pw_toolchain_bazel/cc_toolchain/private/action_config.bzl b/pw_toolchain_bazel/cc_toolchain/private/action_config.bzl
index dbc08b5..1ca47d0 100644
--- a/pw_toolchain_bazel/cc_toolchain/private/action_config.bzl
+++ b/pw_toolchain_bazel/cc_toolchain/private/action_config.bzl
@@ -35,8 +35,10 @@
path = ctx.attr.path or None
files = ctx.files.additional_files
+ transitive_files = []
if tool != None:
files = files + [tool]
+ transitive_files = [ctx.attr.tool[DefaultInfo].data_runfiles.files]
return [
config_lib_tool(
@@ -44,14 +46,14 @@
path = path,
execution_requirements = ctx.attr.execution_requirements,
),
- DefaultInfo(files = depset(files)),
+ DefaultInfo(files = depset(files, transitive = transitive_files)),
]
pw_cc_tool = rule(
implementation = _pw_cc_tool_impl,
attrs = {
"tool": attr.label(
- allow_single_file = True,
+ allow_files = True,
executable = True,
cfg = "exec",
doc = """The underlying tool that this rule represents.
@@ -157,6 +159,7 @@
))
tools = tuple([tool[PwToolInfo] for tool in ctx.attr.tools])
+ files = depset(transitive = [dep[DefaultInfo].files for dep in ctx.attr.tools])
common_kwargs = dict(
label = ctx.label,
tools = tools,
@@ -166,6 +169,7 @@
]),
implies_action_configs = depset([]),
enabled = ctx.attr.enabled,
+ files = files,
)
action_configs = [
_generate_action_config(ctx, action, **common_kwargs)
@@ -177,9 +181,7 @@
label = ctx.label,
action_configs = depset(action_configs),
),
- DefaultInfo(
- files = depset(None, transitive = [dep[DefaultInfo].files for dep in ctx.attr.tools]),
- ),
+ DefaultInfo(files = files),
]
pw_cc_action_config = rule(
diff --git a/pw_toolchain_bazel/cc_toolchain/private/action_config_files.bzl b/pw_toolchain_bazel/cc_toolchain/private/action_config_files.bzl
deleted file mode 100644
index 61c24d9..0000000
--- a/pw_toolchain_bazel/cc_toolchain/private/action_config_files.bzl
+++ /dev/null
@@ -1,91 +0,0 @@
-# Copyright 2023 The Pigweed Authors
-#
-# 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
-#
-# https://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 `pw_cc_action_config_file_collector`.
-
-This library is intended to be a private implementation detail of
-pw_toolchain_bazel, DO NOT export the contents of this file to be used publicly.
-"""
-
-load(
- ":providers.bzl",
- "PwActionConfigSetInfo",
- "PwActionNameSetInfo",
-)
-
-def _get_action_names(action_sets):
- return depset(transitive = [
- action_set[PwActionNameSetInfo].actions
- for action_set in action_sets
- ]).to_list()
-
-def _pw_cc_action_config_file_collector_impl(ctx):
- if ctx.attr.collect_files_from_actions and ctx.attr.collect_files_not_from_actions:
- fail("{} attempted to specify `collect_files_from_actions` and `collect_files_not_from_actions` simultaneously".format(ctx.label))
- collect_files_from_actions = _get_action_names(ctx.attr.collect_files_from_actions)
- collect_files_not_from_actions = _get_action_names(ctx.attr.collect_files_not_from_actions)
-
- all_file_depsets = []
- for dep in ctx.attr.all_action_configs:
- action_names = [ac.action_name for ac in dep[PwActionConfigSetInfo].action_configs.to_list()]
-
- # NOTE: This intentionally doesn't do a check to ensure that the
- # items in `action_names` are `pw_cc_action_config`s because the
- # `pw_cc_toolchain_config` does the same check and will surface a more
- # contextually useful error. Instead, we silently but safely continue
- # even if `action_names` ends up empty.
-
- for action_name in action_names:
- if (action_name in collect_files_from_actions and
- DefaultInfo in dep):
- all_file_depsets.append(dep[DefaultInfo].files)
- elif (collect_files_not_from_actions and
- action_name not in collect_files_not_from_actions and
- DefaultInfo in dep):
- all_file_depsets.append(dep[DefaultInfo].files)
-
- # Tag on the `filegroup` specified in `extra_files` if specified.
- if ctx.attr.extra_files:
- if DefaultInfo not in ctx.attr.extra_files:
- fail(
- "{} in `extra_files` does not provide any files".format(
- ctx.attr.extra_files.label,
- ),
- )
- all_file_depsets.append(ctx.attr.extra_files[DefaultInfo].files)
-
- return [DefaultInfo(files = depset(None, transitive = all_file_depsets))]
-
-pw_cc_action_config_file_collector = rule(
- implementation = _pw_cc_action_config_file_collector_impl,
- attrs = {
- "all_action_configs": attr.label_list(default = [], providers = [PwActionConfigSetInfo]),
- "collect_files_from_actions": attr.label_list(
- providers = [PwActionNameSetInfo],
- doc = """Collects files from tools that apply to the listed action names.
-
-Note: `collect_files_from_actions` and `collect_files_not_from_actions` are
-mutually exclusive.
-""",
- ),
- "collect_files_not_from_actions": attr.label_list(
- providers = [PwActionNameSetInfo],
- doc = """Collects files from tools that DO NOT apply to the listed action names.
-
-Note: `collect_files_from_actions` and `collect_files_not_from_actions` are
-mutually exclusive.
-""",
- ),
- "extra_files": attr.label(),
- },
-)
diff --git a/pw_toolchain_bazel/cc_toolchain/private/cc_toolchain.bzl b/pw_toolchain_bazel/cc_toolchain/private/cc_toolchain.bzl
index b316a2d..48b4e15 100644
--- a/pw_toolchain_bazel/cc_toolchain/private/cc_toolchain.bzl
+++ b/pw_toolchain_bazel/cc_toolchain/private/cc_toolchain.bzl
@@ -21,17 +21,15 @@
"flag_set",
"variable_with_value",
)
-load(
- "//cc_toolchain/private:action_config_files.bzl",
- "pw_cc_action_config_file_collector",
-)
load("//features:builtin_features.bzl", "BUILTIN_FEATURES")
load(
":providers.bzl",
"PwActionConfigSetInfo",
+ "PwActionNameSetInfo",
"PwFeatureInfo",
"PwFeatureSetInfo",
"PwFlagSetInfo",
+ "PwToolchainConfigInfo",
)
load(
":utils.bzl",
@@ -192,22 +190,29 @@
# TODO: b/297413805 - This could be externalized.
out.features.append(_archiver_flags_feature(ctx.attr.target_libc == "macosx"))
- return cc_common.create_cc_toolchain_config_info(
- ctx = ctx,
- action_configs = out.action_configs,
- features = out.features,
- cxx_builtin_include_directories = builtin_include_dirs,
- toolchain_identifier = ctx.attr.toolchain_identifier,
- host_system_name = ctx.attr.host_system_name,
- target_system_name = ctx.attr.target_system_name,
- target_cpu = ctx.attr.target_cpu,
- target_libc = ctx.attr.target_libc,
- compiler = ctx.attr.compiler,
- abi_version = ctx.attr.abi_version,
- abi_libc_version = ctx.attr.abi_libc_version,
- builtin_sysroot = sysroot_dir,
- cc_target_os = ctx.attr.cc_target_os,
- )
+ extra = []
+ if ctx.attr.extra_files:
+ extra.append(ctx.attr.extra_files[DefaultInfo].files)
+ return [
+ cc_common.create_cc_toolchain_config_info(
+ ctx = ctx,
+ action_configs = out.action_configs,
+ features = out.features,
+ cxx_builtin_include_directories = builtin_include_dirs,
+ toolchain_identifier = ctx.attr.toolchain_identifier,
+ host_system_name = ctx.attr.host_system_name,
+ target_system_name = ctx.attr.target_system_name,
+ target_cpu = ctx.attr.target_cpu,
+ target_libc = ctx.attr.target_libc,
+ compiler = ctx.attr.compiler,
+ abi_version = ctx.attr.abi_version,
+ abi_libc_version = ctx.attr.abi_libc_version,
+ builtin_sysroot = sysroot_dir,
+ cc_target_os = ctx.attr.cc_target_os,
+ ),
+ PwToolchainConfigInfo(action_to_files = out.action_to_files),
+ DefaultInfo(files = depset(transitive = extra + out.action_to_files.values())),
+ ]
pw_cc_toolchain_config = rule(
implementation = _pw_cc_toolchain_config_impl,
@@ -216,6 +221,7 @@
"action_configs": attr.label_list(providers = [PwActionConfigSetInfo]),
"unconditional_flag_sets": attr.label_list(providers = [PwFlagSetInfo]),
"toolchain_features": attr.label_list(providers = [PwFeatureSetInfo]),
+ "extra_files": attr.label(),
# Attributes from create_cc_toolchain_config_info.
"toolchain_identifier": attr.string(),
@@ -231,7 +237,7 @@
"cxx_builtin_include_directories": attr.string_list(),
"_builtin_features": attr.label_list(default = BUILTIN_FEATURES),
},
- provides = [CcToolchainConfigInfo],
+ provides = [CcToolchainConfigInfo, PwToolchainConfigInfo],
)
def _check_args(rule_label, kwargs):
@@ -280,75 +286,43 @@
return filtered_args, remainder
-def _generate_file_group(kwargs, attr_name, action_names):
- """Generates rules to collect files from pw_cc_action_config rules.
+def _cc_file_collector_impl(ctx):
+ actions = depset(transitive = [
+ names[PwActionNameSetInfo].actions
+ for names in ctx.attr.actions
+ ]).to_list()
+ action_to_files = ctx.attr.config[PwToolchainConfigInfo].action_to_files
- All items in the kwargs dictionary whose keys are present in the filter
- dictionary are returned as a new dictionary as the first item in the tuple.
- All remaining arguments are returned as a dictionary in the second item of
- the tuple.
+ extra = []
+ if ctx.attr.extra_files:
+ extra.append(ctx.attr.extra_files[DefaultInfo].files)
+ return [DefaultInfo(files = depset(transitive = [
+ action_to_files[action]
+ for action in actions
+ ] + extra))]
- Args:
- kwargs: Dictionary of all pw_cc_toolchain arguments.
- attr_name: The attr name of the file group to collect files for.
- action_names: The actions that apply to the `attr_name` group.
+_cc_file_collector = rule(
+ implementation = _cc_file_collector_impl,
+ attrs = {
+ "config": attr.label(providers = [PwToolchainConfigInfo], mandatory = True),
+ "actions": attr.label_list(providers = [PwActionNameSetInfo], mandatory = True),
+ "extra_files": attr.label(),
+ },
+)
- Returns:
- Name of the generated filegroup rule.
- """
- file_group_name = "{}_{}".format(kwargs["name"], attr_name)
- pw_cc_action_config_file_collector(
- name = file_group_name,
- all_action_configs = kwargs["action_configs"],
- extra_files = kwargs[attr_name] if attr_name in kwargs else None,
- collect_files_from_actions = action_names,
- visibility = ["//visibility:private"],
- )
- return file_group_name
-
-def _generate_misc_file_group(kwargs):
- """Generate the misc_files group.
-
- Some actions aren't enumerated in ALL_FILE_GROUPS because they don't
- necessarily have an associated *_files group. This group collects
- all the other files and enumerates them in a group so they still appear in
- all_files.
-
- Args:
- kwargs: Dictionary of all pw_cc_toolchain arguments.
-
- Returns:
- Name of the generated filegroup rule.
- """
- file_group_name = "{}_misc_files".format(kwargs["name"])
-
- all_known_actions = []
- for action_names in ALL_FILE_GROUPS.values():
- all_known_actions.extend(action_names)
-
- pw_cc_action_config_file_collector(
- name = file_group_name,
- all_action_configs = kwargs["action_configs"],
- collect_files_not_from_actions = all_known_actions,
- visibility = ["//visibility:private"],
- )
- return file_group_name
-
-def pw_cc_toolchain(action_config_flag_sets = None, **kwargs):
+def pw_cc_toolchain(name, action_config_flag_sets = None, **kwargs):
"""A suite of cc_toolchain, pw_cc_toolchain_config, and *_files rules.
Generated rules:
{name}: A `cc_toolchain` for this toolchain.
- {name}_config: A `pw_cc_toolchain_config` for this toolchain.
- {name}_*_files: Generated rules that group together files for
- "all_files", "ar_files", "as_files", "compiler_files",
- "coverage_files", "dwp_files", "linker_files", "objcopy_files", and
- "strip_files" normally enumerated as part of the `cc_toolchain`
- rule.
- {name}_misc_files: Generated rule that groups together files for action
- configs not associated with any other *_files group.
+ _{name}_config: A `pw_cc_toolchain_config` for this toolchain.
+ _{name}_*_files: Generated rules that group together files for
+ "ar_files", "as_files", "compiler_files", "coverage_files",
+ "dwp_files", "linker_files", "objcopy_files", and "strip_files"
+ normally enumerated as part of the `cc_toolchain` rule.
Args:
+ name: str: The name of the label for the toolchain.
action_config_flag_sets: Deprecated. Do not use.
**kwargs: All attributes supported by either cc_toolchain or pw_cc_toolchain_config.
"""
@@ -357,43 +331,52 @@
if action_config_flag_sets != None:
kwargs["unconditional_flag_sets"] = action_config_flag_sets
- _check_args(native.package_relative_label(kwargs["name"]), kwargs)
-
- # Generate *_files groups.
- # `all_files` is skipped here because it is handled differently below.
- for group_name, action_names in ALL_FILE_GROUPS.items():
- kwargs[group_name] = _generate_file_group(kwargs, group_name, action_names)
-
- # The `all_files` group must be a superset of all the smaller file groups.
- all_files_name = "{}_all_files".format(kwargs["name"])
- all_file_inputs = [":{}".format(kwargs[file_group]) for file_group in ALL_FILE_GROUPS.keys()]
-
- all_file_inputs.append(":{}".format(_generate_misc_file_group(kwargs)))
-
- if "all_files" in kwargs:
- all_file_inputs.append(kwargs["all_files"])
- native.filegroup(
- name = all_files_name,
- srcs = all_file_inputs,
- visibility = ["//visibility:private"],
- )
- kwargs["all_files"] = ":{}".format(all_files_name)
+ _check_args(native.package_relative_label(name), kwargs)
# Split args between `pw_cc_toolchain_config` and `native.cc_toolchain`.
cc_toolchain_config_args, cc_toolchain_args = _split_args(kwargs, PW_CC_TOOLCHAIN_CONFIG_ATTRS | PW_CC_TOOLCHAIN_DEPRECATED_TOOL_ATTRS)
# Bind pw_cc_toolchain_config and the cc_toolchain.
- config_name = "{}_config".format(cc_toolchain_args["name"])
- cc_toolchain_config_args["name"] = config_name
- cc_toolchain_args["toolchain_config"] = ":{}".format(config_name)
+ config_name = "_{}_config".format(name)
+ pw_cc_toolchain_config(
+ name = config_name,
+ extra_files = cc_toolchain_args.pop("all_files", None),
+ visibility = ["//visibility:private"],
+ **cc_toolchain_config_args
+ )
- # TODO: b/321268080 - Remove after transition of this option is complete.
- cc_toolchain_args["exec_transition_for_inputs"] = False
+ all_files_srcs = [config_name]
+ for group, actions in ALL_FILE_GROUPS.items():
+ group_name = "_{}_{}".format(name, group)
+ _cc_file_collector(
+ name = group_name,
+ config = config_name,
+ actions = actions,
+ extra_files = cc_toolchain_args.pop(group, None),
+ visibility = ["//visibility:private"],
+ )
+ cc_toolchain_args[group] = group_name
+ all_files_srcs.append(group_name)
# Copy over arguments that should be shared by both rules.
for arg_name in PW_CC_TOOLCHAIN_SHARED_ATTRS:
if arg_name in cc_toolchain_config_args:
cc_toolchain_args[arg_name] = cc_toolchain_config_args[arg_name]
- pw_cc_toolchain_config(**cc_toolchain_config_args)
- native.cc_toolchain(**cc_toolchain_args)
+ all_files_name = "_{}_all_files".format(name)
+ native.filegroup(
+ name = all_files_name,
+ srcs = all_files_srcs,
+ )
+
+ native.cc_toolchain(
+ name = name,
+ toolchain_config = config_name,
+ # TODO: b/321268080 - Remove after transition of this option is complete.
+ exec_transition_for_inputs = False,
+ # TODO(b/323448214): Replace with `all_files = config_name`.
+ # This is currently required in case the user passes in compiler_files,
+ # for example (since it won't propagate to the config.
+ all_files = all_files_name,
+ **cc_toolchain_args
+ )
diff --git a/pw_toolchain_bazel/cc_toolchain/private/providers.bzl b/pw_toolchain_bazel/cc_toolchain/private/providers.bzl
index f1765e0..a0c009b 100644
--- a/pw_toolchain_bazel/cc_toolchain/private/providers.bzl
+++ b/pw_toolchain_bazel/cc_toolchain/private/providers.bzl
@@ -97,6 +97,7 @@
"flag_sets": "Sequence[FlagSetInfo]: Set of flag sets the action sets",
"implies_features": "depset[FeatureInfo]: Set of features implied by this action config",
"implies_action_configs": "depset[ActionConfigInfo]: Set of action configs enabled by this action config",
+ "files": "depset[File]: The files required to run these actions",
},
)
@@ -107,4 +108,12 @@
"action_configs": "depset[ActionConfigInfo]: A set of action configs",
},
)
+
PwToolInfo = ToolInfo
+
+PwToolchainConfigInfo = provider(
+ doc = "Additional metadata about the config of the pigweed toolchain.",
+ fields = {
+ "action_to_files": "Dict[str, depset[File]]: A set of files required to execute a given action",
+ },
+)
diff --git a/pw_toolchain_bazel/cc_toolchain/private/utils.bzl b/pw_toolchain_bazel/cc_toolchain/private/utils.bzl
index c0d5624..eeccace 100644
--- a/pw_toolchain_bazel/cc_toolchain/private/utils.bzl
+++ b/pw_toolchain_bazel/cc_toolchain/private/utils.bzl
@@ -204,7 +204,13 @@
fail = fail,
))
+ action_to_files = {
+ ac.action_name: ac.files
+ for ac in acs
+ }
+
return struct(
features = untyped_features,
action_configs = untyped_acs,
+ action_to_files = action_to_files,
)
diff --git a/pw_toolchain_bazel/cc_toolchain/tests/action_configs/BUILD.bazel b/pw_toolchain_bazel/cc_toolchain/tests/action_configs/BUILD.bazel
index aae465a..5531ff4 100644
--- a/pw_toolchain_bazel/cc_toolchain/tests/action_configs/BUILD.bazel
+++ b/pw_toolchain_bazel/cc_toolchain/tests/action_configs/BUILD.bazel
@@ -12,6 +12,7 @@
# License for the specific language governing permissions and limitations under
# the License.
+load("@bazel_skylib//rules:native_binary.bzl", "native_binary")
load(
"//cc_toolchain:defs.bzl",
"pw_cc_action_config",
@@ -26,6 +27,19 @@
path = "/usr/bin/clang",
)
+native_binary(
+ name = "clang_wrapper",
+ src = "clang_wrapper.sh",
+ out = "clang_wrapper",
+ data = [":real_clang"],
+)
+
+pw_cc_tool(
+ name = "clang_wrapper_tool",
+ additional_files = ["data.txt"],
+ tool = ":clang_wrapper",
+)
+
pw_cc_action_config(
name = "all_c_compile",
action_names = ["//actions:all_c_compiler_actions"],
@@ -39,6 +53,12 @@
tools = [":system_clang"],
)
+pw_cc_action_config(
+ name = "cpp_compile_from_tool",
+ action_names = ["//actions:cpp_compile"],
+ tools = [":clang_wrapper_tool"],
+)
+
test_action_configs(
name = "test_action_configs",
)
diff --git a/pw_toolchain_bazel/cc_toolchain/tests/action_configs/clang_wrapper.sh b/pw_toolchain_bazel/cc_toolchain/tests/action_configs/clang_wrapper.sh
new file mode 100755
index 0000000..e69de29
--- /dev/null
+++ b/pw_toolchain_bazel/cc_toolchain/tests/action_configs/clang_wrapper.sh
diff --git a/pw_toolchain_bazel/cc_toolchain/tests/action_configs/data.txt b/pw_toolchain_bazel/cc_toolchain/tests/action_configs/data.txt
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/pw_toolchain_bazel/cc_toolchain/tests/action_configs/data.txt
diff --git a/pw_toolchain_bazel/cc_toolchain/tests/action_configs/real_clang b/pw_toolchain_bazel/cc_toolchain/tests/action_configs/real_clang
new file mode 100755
index 0000000..e69de29
--- /dev/null
+++ b/pw_toolchain_bazel/cc_toolchain/tests/action_configs/real_clang
diff --git a/pw_toolchain_bazel/cc_toolchain/tests/action_configs/test_action_configs.bzl b/pw_toolchain_bazel/cc_toolchain/tests/action_configs/test_action_configs.bzl
index bbfa3af..ae46dc1 100644
--- a/pw_toolchain_bazel/cc_toolchain/tests/action_configs/test_action_configs.bzl
+++ b/pw_toolchain_bazel/cc_toolchain/tests/action_configs/test_action_configs.bzl
@@ -32,6 +32,12 @@
assert_eq(sorted([x[0] for x in actions]), sorted(deduped_actions))
return deduped_actions
+ def get_files(**kwargs):
+ return {
+ k: sorted([f.basename for f in v.to_list()])
+ for k, v in to_untyped_config(**kwargs).action_to_files.items()
+ }
+
# Verify that we validate that features with duplicate action names are not
# permitted
assert_fail(
@@ -65,4 +71,17 @@
},
)
+ assert_eq(
+ get_files(
+ action_configs = [
+ action_configs.c_compile,
+ action_configs.cpp_compile_from_tool,
+ ],
+ ),
+ {
+ "c-compile": [],
+ "c++-compile": ["clang_wrapper", "data.txt", "real_clang"],
+ },
+ )
+
test_action_configs = generate_test_rule(_test_action_configs_impl)
diff --git a/pw_toolchain_bazel/cc_toolchain/tests/utils.bzl b/pw_toolchain_bazel/cc_toolchain/tests/utils.bzl
index 1e32ea3..dad8f6a 100644
--- a/pw_toolchain_bazel/cc_toolchain/tests/utils.bzl
+++ b/pw_toolchain_bazel/cc_toolchain/tests/utils.bzl
@@ -99,6 +99,7 @@
_PROVIDERS = {
"//cc_toolchain/tests/action_configs:all_c_compile": [PwActionConfigSetInfo],
"//cc_toolchain/tests/action_configs:c_compile": [PwActionConfigSetInfo],
+ "//cc_toolchain/tests/action_configs:cpp_compile_from_tool": [PwActionConfigSetInfo],
"//cc_toolchain/tests/features:bar": [PwFeatureInfo, PwFeatureSetInfo],
"//cc_toolchain/tests/features:baz": [PwFeatureInfo, PwFeatureSetInfo],
"//cc_toolchain/tests/features:conflict": [PwFeatureInfo, PwFeatureSetInfo],
