From 66613ac5d92d8d29e5d59db7bebefa4eea32469b Mon Sep 17 00:00:00 2001 From: Googler Date: Thu, 5 Sep 2024 09:05:23 -0700 Subject: [PATCH] Add support for implicit include directories to rule-based toolchains BEGIN_PUBLIC Add support for implicit include directories to rule-based toolchains Reorients the `cc_toolchain.cxx_builtin_include_directories` attribute so it is expressed as an attribute on `cc_args` and `cc_tool` to signify that the tools or arguments imply include directories that Bazel's include path checker should allowlist. This moves the allowlist to the source of truth that implies these directories. END_PUBLIC PiperOrigin-RevId: 671393376 Change-Id: Ide8cae548783726835168adcd3f705028a1f4308 --- cc/toolchains/args.bzl | 17 +++++++++- cc/toolchains/cc_toolchain_info.bzl | 5 +++ cc/toolchains/feature.bzl | 4 ++- cc/toolchains/impl/collect.bzl | 4 +++ cc/toolchains/impl/external_feature.bzl | 1 + cc/toolchains/impl/legacy_converter.bzl | 9 ++++- cc/toolchains/impl/nested_args.bzl | 2 +- cc/toolchains/impl/toolchain_config.bzl | 11 +------ cc/toolchains/impl/toolchain_config_info.bzl | 8 ++++- cc/toolchains/tool.bzl | 16 ++++++++- tests/rule_based_toolchain/args/BUILD | 8 +++++ tests/rule_based_toolchain/args/args_test.bzl | 13 ++++++++ tests/rule_based_toolchain/args_list/BUILD | 28 ++++++++++++++++ .../args_list/args_list_test.bzl | 33 +++++++++++++++++++ tests/rule_based_toolchain/features/BUILD | 29 ++++++++++++---- .../features/features_test.bzl | 24 +++++++++++--- tests/rule_based_toolchain/subjects.bzl | 9 +++++ tests/rule_based_toolchain/testdata/BUILD | 21 +++++++++++- .../testdata/subdir1/file_foo | 0 .../testdata/subdir2/file_bar | 0 .../testdata/subdir3/file_baz | 0 tests/rule_based_toolchain/tool/BUILD | 7 ++++ tests/rule_based_toolchain/tool/tool_test.bzl | 12 +++++++ .../toolchain_config/BUILD | 15 ++++++--- .../toolchain_config_test.bzl | 6 ++++ 25 files changed, 250 insertions(+), 32 deletions(-) create mode 100644 tests/rule_based_toolchain/testdata/subdir1/file_foo create mode 100644 tests/rule_based_toolchain/testdata/subdir2/file_bar create mode 100644 tests/rule_based_toolchain/testdata/subdir3/file_baz diff --git a/cc/toolchains/args.bzl b/cc/toolchains/args.bzl index 2f624f3..282fa04 100644 --- a/cc/toolchains/args.bzl +++ b/cc/toolchains/args.bzl @@ -13,6 +13,7 @@ # limitations under the License. """All providers for rule-based bazel toolchain config.""" +load("@bazel_skylib//rules/directory:providers.bzl", "DirectoryInfo") load("//cc/toolchains/impl:args_utils.bzl", "validate_nested_args") load( "//cc/toolchains/impl:collect.bzl", @@ -50,7 +51,7 @@ def _cc_args_impl(ctx): ) files = nested.files else: - files = collect_files(ctx.attr.data) + files = collect_files(ctx.attr.data + ctx.attr.allowlist_include_directories) requires = collect_provider(ctx.attr.requires_any_of, FeatureConstraintInfo) @@ -61,6 +62,9 @@ def _cc_args_impl(ctx): nested = nested, env = ctx.attr.env, files = files, + allowlist_include_directories = depset( + direct = [d[DirectoryInfo] for d in ctx.attr.allowlist_include_directories], + ), ) return [ args, @@ -72,6 +76,7 @@ def _cc_args_impl(ctx): struct(action = action, args = tuple([args]), files = files) for action in actions.to_list() ]), + allowlist_include_directories = args.allowlist_include_directories, ), ] @@ -89,6 +94,16 @@ See @rules_cc//cc/toolchains/actions:all for valid options. "env": attr.string_dict( doc = "Environment variables to be added to the command-line.", ), + "allowlist_include_directories": attr.label_list( + providers = [DirectoryInfo], + doc = """Include paths implied by using this rule. + +Some flags (e.g. --sysroot) imply certain include paths are available despite +not explicitly specifying a normal include path flag (`-I`, `-isystem`, etc.). +Bazel checks that all included headers are properly provided by a dependency or +allowlisted through this mechanism. +""", + ), "requires_any_of": attr.label_list( providers = [FeatureConstraintInfo], doc = """This will be enabled when any of the constraints are met. diff --git a/cc/toolchains/cc_toolchain_info.bzl b/cc/toolchains/cc_toolchain_info.bzl index 36e922b..2325ddb 100644 --- a/cc/toolchains/cc_toolchain_info.bzl +++ b/cc/toolchains/cc_toolchain_info.bzl @@ -87,6 +87,7 @@ ArgsInfo = provider( "nested": "(Optional[NestedArgsInfo]) The args expand. Equivalent to a flag group.", "files": "(depset[File]) Files required for the args", "env": "(dict[str, str]) Environment variables to apply", + "allowlist_include_directories": "(depset[DirectoryInfo]) Include directories implied by these arguments that should be allowlisted in Bazel's include checker", }, ) ArgsListInfo = provider( @@ -97,6 +98,7 @@ ArgsListInfo = provider( "args": "(Sequence[ArgsInfo]) The flag sets contained within", "files": "(depset[File]) The files required for all of the arguments", "by_action": "(Sequence[struct(action=ActionTypeInfo, args=List[ArgsInfo], files=depset[Files])]) Relevant information about the args keyed by the action type.", + "allowlist_include_directories": "(depset[DirectoryInfo]) Include directories implied by these arguments that should be allowlisted in Bazel's include checker", }, ) @@ -114,6 +116,7 @@ FeatureInfo = provider( "external": "(bool) Whether a feature is defined elsewhere.", "overridable": "(bool) Whether the feature is an overridable feature.", "overrides": "(Optional[FeatureInfo]) The feature that this overrides. Must be a known feature", + "allowlist_include_directories": "(depset[DirectoryInfo]) Include directories implied by this feature that should be allowlisted in Bazel's include checker", }, ) FeatureSetInfo = provider( @@ -152,6 +155,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", + "allowlist_include_directories": "(depset[DirectoryInfo]) Built-in include directories implied by this tool that should be allowlisted in Bazel's include checker", }, ) @@ -174,5 +178,6 @@ ToolchainConfigInfo = provider( "tool_map": "(ToolConfigInfo) A provider mapping toolchain action types to tools.", "args": "(Sequence[ArgsInfo]) A list of arguments to be unconditionally applied to the toolchain.", "files": "(dict[ActionTypeInfo, depset[File]]) Files required for the toolchain, keyed by the action type.", + "allowlist_include_directories": "(depset[DirectoryInfo]) Built-in include directories implied by this toolchain's args and tools that should be allowlisted in Bazel's include checker", }, ) diff --git a/cc/toolchains/feature.bzl b/cc/toolchains/feature.bzl index 075ff07..a282762 100644 --- a/cc/toolchains/feature.bzl +++ b/cc/toolchains/feature.bzl @@ -62,13 +62,14 @@ def _cc_feature_impl(ctx): if name.startswith("implied_by_"): fail("Feature names starting with 'implied_by' are reserved") + args = collect_args_lists(ctx.attr.args, ctx.label) feature = FeatureInfo( label = ctx.label, name = name, # Unused field, but leave it just in case we want to reuse it in the # future. enabled = False, - args = collect_args_lists(ctx.attr.args, ctx.label), + args = args, implies = collect_features(ctx.attr.implies), requires_any_of = tuple(collect_provider( ctx.attr.requires_any_of, @@ -81,6 +82,7 @@ def _cc_feature_impl(ctx): external = False, overridable = False, overrides = overrides, + allowlist_include_directories = args.allowlist_include_directories, ) return [ diff --git a/cc/toolchains/impl/collect.bzl b/cc/toolchains/impl/collect.bzl index a1daa23..f41aa7d 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(), + allowlist_include_directories = depset(), )) else: fail("Expected %s to be a cc_tool or a binary rule" % target.label) @@ -141,6 +142,9 @@ def collect_args_lists(targets, label): label = label, args = tuple(args), files = depset(transitive = transitive_files), + allowlist_include_directories = depset( + transitive = [a.allowlist_include_directories for a in args], + ), by_action = tuple([ struct( action = k, diff --git a/cc/toolchains/impl/external_feature.bzl b/cc/toolchains/impl/external_feature.bzl index 0853b32..1e11bc9 100644 --- a/cc/toolchains/impl/external_feature.bzl +++ b/cc/toolchains/impl/external_feature.bzl @@ -43,6 +43,7 @@ def _cc_external_feature_impl(ctx): external = True, overridable = ctx.attr.overridable, overrides = None, + allowlist_include_directories = depset(), ) providers = [ feature, diff --git a/cc/toolchains/impl/legacy_converter.bzl b/cc/toolchains/impl/legacy_converter.bzl index 99cc353..7197716 100644 --- a/cc/toolchains/impl/legacy_converter.bzl +++ b/cc/toolchains/impl/legacy_converter.bzl @@ -146,7 +146,7 @@ def convert_toolchain(toolchain): """Converts a rule-based toolchain into the legacy providers. Args: - toolchain: CcToolchainConfigInfo: The toolchain config to convert. + toolchain: (ToolchainConfigInfo) The toolchain config to convert. Returns: A struct containing parameters suitable to pass to cc_common.create_cc_toolchain_config_info. @@ -165,10 +165,17 @@ def convert_toolchain(toolchain): requires_any_of = [], mutually_exclusive = [], external = False, + allowlist_include_directories = depset(), ))) action_configs = _convert_tool_map(toolchain.tool_map) + cxx_builtin_include_directories = [ + d.path + for d in toolchain.allowlist_include_directories.to_list() + ] + return struct( features = [ft for ft in features if ft != None], action_configs = sorted(action_configs, key = lambda ac: ac.action_name), + cxx_builtin_include_directories = cxx_builtin_include_directories, ) diff --git a/cc/toolchains/impl/nested_args.bzl b/cc/toolchains/impl/nested_args.bzl index 2a5fb07..17ebb77 100644 --- a/cc/toolchains/impl/nested_args.bzl +++ b/cc/toolchains/impl/nested_args.bzl @@ -97,7 +97,7 @@ def nested_args_provider_from_ctx(ctx): args = ctx.attr.args, format = ctx.attr.format, nested = collect_provider(ctx.attr.nested, NestedArgsInfo), - files = collect_files(ctx.attr.data), + files = collect_files(ctx.attr.data + getattr(ctx.attr, "allowlist_include_directories", [])), iterate_over = ctx.attr.iterate_over, requires_not_none = _var(ctx.attr.requires_not_none), requires_none = _var(ctx.attr.requires_none), diff --git a/cc/toolchains/impl/toolchain_config.bzl b/cc/toolchains/impl/toolchain_config.bzl index 282588b..5c8d69c 100644 --- a/cc/toolchains/impl/toolchain_config.bzl +++ b/cc/toolchains/impl/toolchain_config.bzl @@ -14,7 +14,6 @@ """Implementation of the cc_toolchain rule.""" load("@bazel_skylib//rules:common_settings.bzl", "BuildSettingInfo") -load("@bazel_skylib//rules/directory:providers.bzl", "DirectoryInfo") load( "//cc/toolchains:cc_toolchain_info.bzl", "ActionTypeSetInfo", @@ -66,18 +65,13 @@ def _cc_toolchain_config_impl(ctx): legacy = convert_toolchain(toolchain_config) - cxx_builtin_include_directories = [ - d[DirectoryInfo].path - for d in ctx.attr.cxx_builtin_include_directories - ] - return [ toolchain_config, cc_common.create_cc_toolchain_config_info( ctx = ctx, action_configs = legacy.action_configs, features = legacy.features, - cxx_builtin_include_directories = cxx_builtin_include_directories, + cxx_builtin_include_directories = legacy.cxx_builtin_include_directories, # toolchain_identifier is deprecated, but setting it to None results # in an error that it expected a string, and for safety's sake, I'd # prefer to provide something unique. @@ -110,9 +104,6 @@ cc_toolchain_config = rule( "skip_experimental_flag_validation_for_test": attr.bool(default = False), "_builtin_features": attr.label(default = "//cc/toolchains/features:all_builtin_features"), "_enabled": attr.label(default = "//cc/toolchains:experimental_enable_rule_based_toolchains"), - - # Attributes translated from legacy cc toolchains. - "cxx_builtin_include_directories": attr.label_list(providers = [DirectoryInfo]), }, provides = [ToolchainConfigInfo], ) diff --git a/cc/toolchains/impl/toolchain_config_info.bzl b/cc/toolchains/impl/toolchain_config_info.bzl index a2c6bf1..7e68b15 100644 --- a/cc/toolchains/impl/toolchain_config_info.bzl +++ b/cc/toolchains/impl/toolchain_config_info.bzl @@ -162,7 +162,12 @@ def toolchain_config_info(label, known_features = [], enabled_features = [], arg action_type: _collect_files_for_action_type(action_type, tools, features, args) for action_type in tools.keys() } - + allowlist_include_directories = depset( + transitive = [ + src.allowlist_include_directories + for src in features + tools.values() + ] + [args.allowlist_include_directories], + ) toolchain_config = ToolchainConfigInfo( label = label, features = features, @@ -170,6 +175,7 @@ def toolchain_config_info(label, known_features = [], enabled_features = [], arg tool_map = tool_map[ToolConfigInfo], args = args.args, files = files, + allowlist_include_directories = allowlist_include_directories, ) _validate_toolchain(toolchain_config, fail = fail) return toolchain_config diff --git a/cc/toolchains/tool.bzl b/cc/toolchains/tool.bzl index f07af32..159a9d6 100644 --- a/cc/toolchains/tool.bzl +++ b/cc/toolchains/tool.bzl @@ -13,6 +13,7 @@ # limitations under the License. """Implementation of cc_tool""" +load("@bazel_skylib//rules/directory:providers.bzl", "DirectoryInfo") load("//cc/toolchains/impl:collect.bzl", "collect_data") load( ":cc_toolchain_info.bzl", @@ -28,12 +29,15 @@ def _cc_tool_impl(ctx): else: fail("Expected cc_tool's src attribute to be either an executable or a single file") - runfiles = collect_data(ctx, ctx.attr.data + [ctx.attr.src]) + runfiles = collect_data(ctx, ctx.attr.data + [ctx.attr.src] + ctx.attr.allowlist_include_directories) tool = ToolInfo( label = ctx.label, exe = exe, runfiles = runfiles, execution_requirements = tuple(ctx.attr.tags), + allowlist_include_directories = depset( + direct = [d[DirectoryInfo] for d in ctx.attr.allowlist_include_directories], + ), ) link = ctx.actions.declare_file(ctx.label.name) @@ -70,6 +74,16 @@ executable label. allow_files = True, doc = "Additional files that are required for this tool to run.", ), + "allowlist_include_directories": attr.label_list( + providers = [DirectoryInfo], + doc = """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. +""", + ), }, provides = [ToolInfo], doc = """Declares a tool that can be bound to action configs. diff --git a/tests/rule_based_toolchain/args/BUILD b/tests/rule_based_toolchain/args/BUILD index 585ec91..3b8e1ab 100644 --- a/tests/rule_based_toolchain/args/BUILD +++ b/tests/rule_based_toolchain/args/BUILD @@ -29,6 +29,14 @@ util.helper_target( env = {"BAR": "bar"}, ) +util.helper_target( + cc_args, + name = "with_dir", + actions = ["//tests/rule_based_toolchain/actions:all_compile"], + allowlist_include_directories = ["//tests/rule_based_toolchain/testdata:directory"], + args = ["--secret-builtin-include-dir"], +) + analysis_test_suite( name = "test_suite", targets = TARGETS, diff --git a/tests/rule_based_toolchain/args/args_test.bzl b/tests/rule_based_toolchain/args/args_test.bzl index 226aadb..46ab7f9 100644 --- a/tests/rule_based_toolchain/args/args_test.bzl +++ b/tests/rule_based_toolchain/args/args_test.bzl @@ -39,6 +39,7 @@ _SIMPLE_FILES = [ "tests/rule_based_toolchain/testdata/multiple1", "tests/rule_based_toolchain/testdata/multiple2", ] +_TOOL_DIRECTORY = "tests/rule_based_toolchain/testdata" _CONVERTED_ARGS = subjects.struct( flag_sets = subjects.collection, @@ -99,9 +100,20 @@ def _env_only_test(env, targets): converted.flag_sets().contains_exactly([]) +def _with_dir_test(env, targets): + with_dir = env.expect.that_target(targets.with_dir).provider(ArgsInfo) + with_dir.allowlist_include_directories().contains_exactly([_TOOL_DIRECTORY]) + with_dir.files().contains_at_least(_SIMPLE_FILES) + + c_compile = env.expect.that_target(targets.with_dir).provider(ArgsListInfo).by_action().get( + targets.c_compile[ActionTypeInfo], + ) + c_compile.files().contains_at_least(_SIMPLE_FILES) + TARGETS = [ ":simple", ":env_only", + ":with_dir", "//tests/rule_based_toolchain/actions:c_compile", "//tests/rule_based_toolchain/actions:cpp_compile", ] @@ -110,4 +122,5 @@ TARGETS = [ TESTS = { "simple_test": _simple_test, "env_only_test": _env_only_test, + "with_dir_test": _with_dir_test, } diff --git a/tests/rule_based_toolchain/args_list/BUILD b/tests/rule_based_toolchain/args_list/BUILD index 9fc9f88..64f0fbb 100644 --- a/tests/rule_based_toolchain/args_list/BUILD +++ b/tests/rule_based_toolchain/args_list/BUILD @@ -42,6 +42,34 @@ util.helper_target( visibility = ["//tests/rule_based_toolchain:__subpackages__"], ) +util.helper_target( + cc_args, + name = "args_with_dir_1", + actions = ["//tests/rule_based_toolchain/actions:c_compile"], + allowlist_include_directories = ["//tests/rule_based_toolchain/testdata:subdirectory_1"], + args = ["dir1"], + visibility = ["//tests/rule_based_toolchain:__subpackages__"], +) + +util.helper_target( + cc_args, + name = "args_with_dir_2", + actions = ["//tests/rule_based_toolchain/actions:cpp_compile"], + allowlist_include_directories = ["//tests/rule_based_toolchain/testdata:subdirectory_2"], + args = ["dir2"], + visibility = ["//tests/rule_based_toolchain:__subpackages__"], +) + +util.helper_target( + cc_args_list, + name = "args_list_with_dir", + args = [ + ":args_with_dir_1", + ":args_with_dir_2", + ], + visibility = ["//tests/rule_based_toolchain:__subpackages__"], +) + analysis_test_suite( name = "test_suite", targets = TARGETS, diff --git a/tests/rule_based_toolchain/args_list/args_list_test.bzl b/tests/rule_based_toolchain/args_list/args_list_test.bzl index 1d37145..c811673 100644 --- a/tests/rule_based_toolchain/args_list/args_list_test.bzl +++ b/tests/rule_based_toolchain/args_list/args_list_test.bzl @@ -26,6 +26,20 @@ _C_COMPILE_FILE = "tests/rule_based_toolchain/testdata/file1" _CPP_COMPILE_FILE = "tests/rule_based_toolchain/testdata/file2" _BOTH_FILE = "tests/rule_based_toolchain/testdata/multiple1" +_TEST_DIR_1 = "tests/rule_based_toolchain/testdata/subdir1" +_TEST_DIR_2 = "tests/rule_based_toolchain/testdata/subdir2" +_ALL_TEST_DIRS = [ + _TEST_DIR_1, + _TEST_DIR_2, +] +_TEST_DIR_1_FILES = [ + "tests/rule_based_toolchain/testdata/subdir1/file_foo", +] +_TEST_DIR_2_FILES = [ + "tests/rule_based_toolchain/testdata/subdir2/file_bar", +] +_ALL_TEST_DIRS_FILES = _TEST_DIR_1_FILES + _TEST_DIR_2_FILES + def _collect_args_lists_test(env, targets): args = env.expect.that_target(targets.args_list).provider(ArgsListInfo) args.args().contains_exactly([ @@ -53,15 +67,34 @@ def _collect_args_lists_test(env, targets): targets.all_compile_args[ArgsInfo], ]) +def _collect_args_list_dirs_test(env, targets): + args = env.expect.that_target(targets.args_list_with_dir).provider(ArgsListInfo) + args.allowlist_include_directories().contains_exactly(_ALL_TEST_DIRS) + args.files().contains_exactly(_ALL_TEST_DIRS_FILES) + + c_compile = env.expect.that_target(targets.args_list_with_dir).provider(ArgsListInfo).by_action().get( + targets.c_compile[ActionTypeInfo], + ) + c_compile.files().contains_exactly(_TEST_DIR_1_FILES) + + cpp_compile = env.expect.that_target(targets.args_list_with_dir).provider(ArgsListInfo).by_action().get( + targets.cpp_compile[ActionTypeInfo], + ) + cpp_compile.files().contains_exactly(_TEST_DIR_2_FILES) + TARGETS = [ ":c_compile_args", ":cpp_compile_args", ":all_compile_args", ":args_list", + ":args_with_dir_1", + ":args_with_dir_2", + ":args_list_with_dir", "//tests/rule_based_toolchain/actions:c_compile", "//tests/rule_based_toolchain/actions:cpp_compile", ] TESTS = { + "collect_args_list_dirs_test": _collect_args_list_dirs_test, "collect_args_lists_test": _collect_args_lists_test, } diff --git a/tests/rule_based_toolchain/features/BUILD b/tests/rule_based_toolchain/features/BUILD index 9a142be..c982318 100644 --- a/tests/rule_based_toolchain/features/BUILD +++ b/tests/rule_based_toolchain/features/BUILD @@ -10,7 +10,7 @@ load(":features_test.bzl", "TARGETS", "TESTS") util.helper_target( cc_args, - name = "c_compile", + name = "c_compile_args", actions = ["//tests/rule_based_toolchain/actions:c_compile"], args = ["c"], data = ["//tests/rule_based_toolchain/testdata:file1"], @@ -19,7 +19,7 @@ util.helper_target( util.helper_target( cc_feature, name = "simple", - args = [":c_compile"], + args = [":c_compile_args"], feature_name = "feature_name", visibility = ["//tests/rule_based_toolchain:__subpackages__"], ) @@ -27,7 +27,7 @@ util.helper_target( util.helper_target( cc_feature, name = "simple2", - args = [":c_compile"], + args = [":c_compile_args"], feature_name = "simple2", ) @@ -43,7 +43,7 @@ util.helper_target( util.helper_target( cc_feature, name = "requires", - args = [":c_compile"], + args = [":c_compile_args"], feature_name = "requires", requires_any_of = [":feature_set"], ) @@ -51,7 +51,7 @@ util.helper_target( util.helper_target( cc_feature, name = "implies", - args = [":c_compile"], + args = [":c_compile_args"], feature_name = "implies", implies = [":simple"], ) @@ -63,7 +63,7 @@ cc_mutually_exclusive_category( util.helper_target( cc_feature, name = "mutual_exclusion_feature", - args = [":c_compile"], + args = [":c_compile_args"], feature_name = "mutual_exclusion", mutually_exclusive = [ ":simple", @@ -99,7 +99,7 @@ util.helper_target( util.helper_target( cc_feature, name = "overrides", - args = [":c_compile"], + args = [":c_compile_args"], overrides = ":builtin_feature", ) @@ -109,6 +109,21 @@ util.helper_target( feature_name = "sentinel_feature_name", ) +util.helper_target( + cc_args, + name = "args_with_dir", + actions = ["//tests/rule_based_toolchain/actions:c_compile"], + allowlist_include_directories = ["//tests/rule_based_toolchain/testdata:subdirectory_1"], + args = ["--include-builtin-dirs"], +) + +util.helper_target( + cc_feature, + name = "feature_with_dir", + args = [":args_with_dir"], + feature_name = "feature_with_dir", +) + analysis_test_suite( name = "test_suite", targets = TARGETS, diff --git a/tests/rule_based_toolchain/features/features_test.bzl b/tests/rule_based_toolchain/features/features_test.bzl index 332d27e..a0d479a 100644 --- a/tests/rule_based_toolchain/features/features_test.bzl +++ b/tests/rule_based_toolchain/features/features_test.bzl @@ -21,6 +21,7 @@ load( ) load( "//cc/toolchains:cc_toolchain_info.bzl", + "ActionTypeInfo", "ArgsInfo", "FeatureConstraintInfo", "FeatureInfo", @@ -36,6 +37,8 @@ load( visibility("private") _C_COMPILE_FILE = "tests/rule_based_toolchain/testdata/file1" +_SUBDIR1 = "tests/rule_based_toolchain/testdata/subdir1" +_SUBDIR1_FILES = ["tests/rule_based_toolchain/testdata/subdir1/file_foo"] def _sentinel_feature_test(env, targets): sentinel_feature = env.expect.that_target(targets.sentinel_feature).provider(FeatureInfo) @@ -45,17 +48,17 @@ def _sentinel_feature_test(env, targets): def _simple_feature_test(env, targets): simple = env.expect.that_target(targets.simple).provider(FeatureInfo) simple.name().equals("feature_name") - simple.args().args().contains_exactly([targets.c_compile.label]) + simple.args().args().contains_exactly([targets.c_compile_args.label]) simple.enabled().equals(False) simple.overrides().is_none() simple.overridable().equals(False) simple.args().files().contains_exactly([_C_COMPILE_FILE]) c_compile_action = simple.args().by_action().get( - targets.c_compile[ArgsInfo].actions.to_list()[0], + targets.c_compile_args[ArgsInfo].actions.to_list()[0], ) c_compile_action.files().contains_exactly([_C_COMPILE_FILE]) - c_compile_action.args().contains_exactly([targets.c_compile[ArgsInfo]]) + c_compile_action.args().contains_exactly([targets.c_compile_args[ArgsInfo]]) legacy = convert_feature(simple.actual) env.expect.that_str(legacy.name).equals("feature_name") @@ -149,12 +152,23 @@ def _feature_can_be_overridden_test(env, targets): overrides.name().equals("builtin_feature") overrides.overrides().some().label().equals(targets.builtin_feature.label) +def _feature_with_directory_test(env, targets): + with_dir = env.expect.that_target(targets.feature_with_dir).provider(FeatureInfo) + with_dir.allowlist_include_directories().contains_exactly([_SUBDIR1]) + + c_compile = env.expect.that_target(targets.feature_with_dir).provider(FeatureInfo).args().by_action().get( + targets.c_compile[ActionTypeInfo], + ) + c_compile.files().contains_at_least(_SUBDIR1_FILES) + TARGETS = [ + ":args_with_dir", ":builtin_feature", - ":c_compile", + ":c_compile_args", ":category", ":direct_constraint", ":feature_set", + ":feature_with_dir", ":implies", ":mutual_exclusion_feature", ":overrides", @@ -163,6 +177,7 @@ TARGETS = [ ":simple", ":simple2", ":transitive_constraint", + "//tests/rule_based_toolchain/actions:c_compile", ] # @unsorted-dict-items @@ -177,4 +192,5 @@ TESTS = { "feature_constraint_collects_transitive_features_test": _feature_constraint_collects_transitive_features_test, "external_feature_is_a_feature_test": _external_feature_is_a_feature_test, "feature_can_be_overridden_test": _feature_can_be_overridden_test, + "feature_with_directory_test": _feature_with_directory_test, } diff --git a/tests/rule_based_toolchain/subjects.bzl b/tests/rule_based_toolchain/subjects.bzl index 92f6fc8..e741d67 100644 --- a/tests/rule_based_toolchain/subjects.bzl +++ b/tests/rule_based_toolchain/subjects.bzl @@ -43,6 +43,10 @@ runfiles_subject = lambda value, meta: _subjects.depset_file(value.files, meta = # type. unknown_subject = _subjects.str +# Directory depsets are quite complex, so just simplify them as a list of paths. +# buildifier: disable=name-conventions +_FakeDirectoryDepset = lambda value, *, meta: _subjects.collection([v.path for v in value.to_list()], meta = meta) + # buildifier: disable=name-conventions _ActionTypeFactory = generate_factory( ActionTypeInfo, @@ -78,6 +82,7 @@ _FEATURE_FLAGS = dict( overridable = _subjects.bool, external = _subjects.bool, overrides = None, + allowlist_include_directories = _FakeDirectoryDepset, ) # Break the dependency loop. @@ -141,6 +146,7 @@ _ArgsFactory = generate_factory( # Use .factory so it's not inlined. nested = optional_subject(_NestedArgsFactory.factory), requires_any_of = ProviderSequence(_FeatureConstraintFactory), + allowlist_include_directories = _FakeDirectoryDepset, ), ) @@ -155,6 +161,7 @@ _ArgsListFactory = generate_factory( files = _subjects.depset_file, ))({value.action: value for value in values}, meta = meta), files = _subjects.depset_file, + allowlist_include_directories = _FakeDirectoryDepset, ), ) @@ -179,6 +186,7 @@ _ToolFactory = generate_factory( exe = _subjects.file, runfiles = runfiles_subject, execution_requirements = _subjects.collection, + allowlist_include_directories = _FakeDirectoryDepset, ), ) @@ -201,6 +209,7 @@ _ToolchainConfigFactory = generate_factory( tool_map = optional_subject(_ToolConfigFactory.factory), args = ProviderSequence(_ArgsFactory), files = dict_key_subject(_subjects.depset_file), + allowlist_include_directories = _FakeDirectoryDepset, ), ) diff --git a/tests/rule_based_toolchain/testdata/BUILD b/tests/rule_based_toolchain/testdata/BUILD index 950751c..876834a 100644 --- a/tests/rule_based_toolchain/testdata/BUILD +++ b/tests/rule_based_toolchain/testdata/BUILD @@ -1,16 +1,35 @@ load("@bazel_skylib//rules:native_binary.bzl", "native_binary") load("@bazel_skylib//rules/directory:directory.bzl", "directory") +load("@bazel_skylib//rules/directory:subdirectory.bzl", "subdirectory") package(default_visibility = ["//tests/rule_based_toolchain:__subpackages__"]) directory( name = "directory", srcs = glob( - ["*"], + ["**"], exclude = ["BUILD"], ), ) +subdirectory( + name = "subdirectory_1", + parent = ":directory", + path = "subdir1", +) + +subdirectory( + name = "subdirectory_2", + parent = ":directory", + path = "subdir2", +) + +subdirectory( + name = "subdirectory_3", + parent = ":directory", + path = "subdir3", +) + exports_files( glob( ["*"], diff --git a/tests/rule_based_toolchain/testdata/subdir1/file_foo b/tests/rule_based_toolchain/testdata/subdir1/file_foo new file mode 100644 index 0000000..e69de29 diff --git a/tests/rule_based_toolchain/testdata/subdir2/file_bar b/tests/rule_based_toolchain/testdata/subdir2/file_bar new file mode 100644 index 0000000..e69de29 diff --git a/tests/rule_based_toolchain/testdata/subdir3/file_baz b/tests/rule_based_toolchain/testdata/subdir3/file_baz new file mode 100644 index 0000000..e69de29 diff --git a/tests/rule_based_toolchain/tool/BUILD b/tests/rule_based_toolchain/tool/BUILD index 2d58852..67ce625 100644 --- a/tests/rule_based_toolchain/tool/BUILD +++ b/tests/rule_based_toolchain/tool/BUILD @@ -16,6 +16,13 @@ cc_tool( visibility = ["//tests/rule_based_toolchain:__subpackages__"], ) +cc_tool( + name = "tool_with_allowlist_include_directories", + src = "//tests/rule_based_toolchain/testdata:bin_wrapper.sh", + allowlist_include_directories = ["//tests/rule_based_toolchain/testdata:directory"], + 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 81b62b6..ebc5164 100644 --- a/tests/rule_based_toolchain/tool/tool_test.bzl +++ b/tests/rule_based_toolchain/tool/tool_test.bzl @@ -32,6 +32,8 @@ tool_result = subjects.result(subjects.ToolInfo) _BIN_WRAPPER_SYMLINK = "tests/rule_based_toolchain/testdata/bin_wrapper" _BIN_WRAPPER = "tests/rule_based_toolchain/testdata/bin_wrapper.sh" _BIN = "tests/rule_based_toolchain/testdata/bin" +_FILE1 = "tests/rule_based_toolchain/testdata/file1" +_TOOL_DIRECTORY = "tests/rule_based_toolchain/testdata" def _tool_test(env, target): tool = env.expect.that_target(target).provider(ToolInfo) @@ -55,6 +57,14 @@ def _wrapped_tool_includes_runfiles_test(env, targets): _BIN, ]) +def _tool_with_allowlist_include_directories_test(env, targets): + tool = env.expect.that_target(targets.tool_with_allowlist_include_directories).provider(ToolInfo) + tool.allowlist_include_directories().contains_exactly([_TOOL_DIRECTORY]) + tool.runfiles().contains_at_least([ + _BIN, + _FILE1, + ]) + def _collect_tools_collects_tools_test(env, targets): env.expect.that_value( value = collect_tools(env.ctx, [targets.tool, targets.wrapped_tool]), @@ -97,6 +107,7 @@ TARGETS = [ "//tests/rule_based_toolchain/tool:tool", "//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/testdata:bin_wrapper", "//tests/rule_based_toolchain/testdata:multiple", "//tests/rule_based_toolchain/testdata:bin_filegroup", @@ -112,4 +123,5 @@ TESTS = { "collect_tools_collects_binaries_test": _collect_tools_collects_binaries_test, "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, } diff --git a/tests/rule_based_toolchain/toolchain_config/BUILD b/tests/rule_based_toolchain/toolchain_config/BUILD index 6d18357..981758e 100644 --- a/tests/rule_based_toolchain/toolchain_config/BUILD +++ b/tests/rule_based_toolchain/toolchain_config/BUILD @@ -2,6 +2,7 @@ load("@rules_testing//lib:util.bzl", "util") load("//cc/toolchains:args.bzl", "cc_args") load("//cc/toolchains:feature.bzl", "cc_feature") load("//cc/toolchains:feature_set.bzl", "cc_feature_set") +load("//cc/toolchains:tool.bzl", "cc_tool") load("//cc/toolchains:tool_map.bzl", "cc_tool_map") load("//cc/toolchains/args:sysroot.bzl", "cc_sysroot") load("//cc/toolchains/impl:external_feature.bzl", "cc_external_feature") @@ -29,6 +30,7 @@ util.helper_target( cc_args, name = "c_compile_args", actions = ["//tests/rule_based_toolchain/actions:c_compile"], + allowlist_include_directories = ["//tests/rule_based_toolchain/testdata:subdirectory_1"], args = ["c_compile_args"], data = ["//tests/rule_based_toolchain/testdata:file1"], ) @@ -41,6 +43,12 @@ util.helper_target( env = {"CPP_COMPILE": "1"}, ) +cc_tool( + name = "c_compile_tool", + src = "//tests/rule_based_toolchain/testdata:bin_wrapper", + allowlist_include_directories = ["//tests/rule_based_toolchain/testdata:subdirectory_3"], +) + cc_sysroot( name = "sysroot", sysroot = "//tests/rule_based_toolchain/testdata:directory", @@ -53,9 +61,6 @@ util.helper_target( ":sysroot", ":c_compile_args", ], - cxx_builtin_include_directories = [ - "//tests/rule_based_toolchain/testdata:directory", - ], enabled_features = [":simple_feature"], known_features = [":compile_feature"], skip_experimental_flag_validation_for_test = True, @@ -80,6 +85,7 @@ util.helper_target( cc_args, name = "compile_args", actions = ["//tests/rule_based_toolchain/actions:all_compile"], + allowlist_include_directories = ["//tests/rule_based_toolchain/testdata:subdirectory_2"], args = ["compile_args"], data = ["//tests/rule_based_toolchain/testdata:file2"], ) @@ -88,7 +94,8 @@ util.helper_target( cc_tool_map, name = "compile_tool_map", tools = { - "//tests/rule_based_toolchain/actions:all_compile": "//tests/rule_based_toolchain/tool:wrapped_tool", + "//tests/rule_based_toolchain/actions:c_compile": ":c_compile_tool", + "//tests/rule_based_toolchain/actions:cpp_compile": "//tests/rule_based_toolchain/tool:wrapped_tool", }, ) 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 215d3fe..71a05cf 100644 --- a/tests/rule_based_toolchain/toolchain_config/toolchain_config_test.bzl +++ b/tests/rule_based_toolchain/toolchain_config/toolchain_config_test.bzl @@ -36,11 +36,17 @@ _COLLECTED_CPP_COMPILE_FILES = [ "tests/rule_based_toolchain/testdata/bin_wrapper", # From :compile_feature's args "tests/rule_based_toolchain/testdata/file2", + # From :compile_feature's args' allowlist_include_directories + "tests/rule_based_toolchain/testdata/subdir2/file_bar", ] _COLLECTED_C_COMPILE_FILES = _COLLECTED_CPP_COMPILE_FILES + [ # From :c_compile_args "tests/rule_based_toolchain/testdata/file1", + # From :c_compile_args's allowlist_include_directories + "tests/rule_based_toolchain/testdata/subdir1/file_foo", + # From :c_compile_tool's allowlist_include_directories + "tests/rule_based_toolchain/testdata/subdir3/file_baz", ] def _expect_that_toolchain(env, expr = None, **kwargs):