From 2480c905254f519bf41850ed7c3f94f8f5b26808 Mon Sep 17 00:00:00 2001 From: Googler Date: Thu, 22 Aug 2024 20:35:27 -0700 Subject: [PATCH] Replace sysroot with a cc_sysroot macro. BEGIN_PUBLIC Replace sysroot with a cc_sysroot macro. This is part of what amontanez@ and I discussed about making C++ toolchains less "magic". I'd like to do the same for cxx_builtin_include_directories, but that will require significantly more effort. END_PUBLIC PiperOrigin-RevId: 666607531 Change-Id: Ic9cfb157e892c05a9c875f240c0ed9a1048dea19 --- cc/toolchains/args/BUILD | 0 cc/toolchains/args/sysroot.bzl | 39 +++++++++++++++++++ cc/toolchains/features/legacy/BUILD | 8 +--- cc/toolchains/impl/toolchain_config.bzl | 6 --- cc/toolchains/variables/BUILD | 7 +--- .../toolchain_config/BUILD | 12 +++++- .../toolchain_config_test.bzl | 37 +++++++++++++++--- 7 files changed, 84 insertions(+), 25 deletions(-) create mode 100644 cc/toolchains/args/BUILD create mode 100644 cc/toolchains/args/sysroot.bzl diff --git a/cc/toolchains/args/BUILD b/cc/toolchains/args/BUILD new file mode 100644 index 0000000..e69de29 diff --git a/cc/toolchains/args/sysroot.bzl b/cc/toolchains/args/sysroot.bzl new file mode 100644 index 0000000..0f74d20 --- /dev/null +++ b/cc/toolchains/args/sysroot.bzl @@ -0,0 +1,39 @@ +# 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 the cc_sysroot macro.""" + +load("//cc/toolchains:args.bzl", "cc_args") + +visibility("public") + +def cc_sysroot(name, sysroot, **kwargs): + """Creates args for a sysroot. + + Args: + name: (str) The name of the target + sysroot: (bazel_skylib's directory rule) The directory that should be the + sysroot. + **kwargs: kwargs to pass to cc_args. + """ + cc_args( + name = name, + actions = [ + "//cc/toolchains/actions:cpp_compile_actions", + "//cc/toolchains/actions:c_compile", + "//cc/toolchains/actions:link_actions", + ], + args = ["--sysroot={sysroot}"], + format = {"sysroot": sysroot}, + **kwargs + ) diff --git a/cc/toolchains/features/legacy/BUILD b/cc/toolchains/features/legacy/BUILD index c7953a0..f7e4dd9 100644 --- a/cc/toolchains/features/legacy/BUILD +++ b/cc/toolchains/features/legacy/BUILD @@ -203,11 +203,8 @@ cc_external_feature( overridable = True, ) -cc_external_feature( - name = "sysroot", - feature_name = "sysroot", - overridable = True, -) +# Instead of the "sysroot" legacy flag, use the cc_sysroot macro in +# //cc/toolchains/args:sysroot.bzl cc_external_feature( name = "unfiltered_compile_flags", @@ -269,7 +266,6 @@ cc_feature_set( ":gcc_coverage_map_format", ":fully_static_link", ":user_compile_flags", - ":sysroot", ":unfiltered_compile_flags", ":linker_param_file", ":compiler_input_flags", diff --git a/cc/toolchains/impl/toolchain_config.bzl b/cc/toolchains/impl/toolchain_config.bzl index 24af9c0..282588b 100644 --- a/cc/toolchains/impl/toolchain_config.bzl +++ b/cc/toolchains/impl/toolchain_config.bzl @@ -66,10 +66,6 @@ def _cc_toolchain_config_impl(ctx): legacy = convert_toolchain(toolchain_config) - sysroot = None - if ctx.attr.sysroot: - sysroot = ctx.attr.sysroot[DirectoryInfo].path - cxx_builtin_include_directories = [ d[DirectoryInfo].path for d in ctx.attr.cxx_builtin_include_directories @@ -93,7 +89,6 @@ def _cc_toolchain_config_impl(ctx): compiler = "", abi_version = "", abi_libc_version = "", - builtin_sysroot = sysroot, ), # This allows us to support all_files. # If all_files was simply an alias to @@ -117,7 +112,6 @@ cc_toolchain_config = rule( "_enabled": attr.label(default = "//cc/toolchains:experimental_enable_rule_based_toolchains"), # Attributes translated from legacy cc toolchains. - "sysroot": attr.label(providers = [DirectoryInfo]), "cxx_builtin_include_directories": attr.label_list(providers = [DirectoryInfo]), }, provides = [ToolchainConfigInfo], diff --git a/cc/toolchains/variables/BUILD b/cc/toolchains/variables/BUILD index 3740ce7..d650708 100644 --- a/cc/toolchains/variables/BUILD +++ b/cc/toolchains/variables/BUILD @@ -400,10 +400,8 @@ cc_variable( type = types.list(types.string), ) -cc_variable( - name = "sysroot", - type = types.directory, -) +# Instead of the "sysroot" variable, use the cc_sysroot macro in +# //cc/toolchains/args:sysroot.bzl cc_variable( name = "system_include_paths", @@ -526,7 +524,6 @@ cc_builtin_variables( ":source_file", ":strip_debug_symbols", ":stripopts", - ":sysroot", ":system_include_paths", ":thinlto_index", ":thinlto_indexing_param_file", diff --git a/tests/rule_based_toolchain/toolchain_config/BUILD b/tests/rule_based_toolchain/toolchain_config/BUILD index c1199f8..6d18357 100644 --- a/tests/rule_based_toolchain/toolchain_config/BUILD +++ b/tests/rule_based_toolchain/toolchain_config/BUILD @@ -3,6 +3,7 @@ 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_map.bzl", "cc_tool_map") +load("//cc/toolchains/args:sysroot.bzl", "cc_sysroot") load("//cc/toolchains/impl:external_feature.bzl", "cc_external_feature") load("//cc/toolchains/impl:toolchain_config.bzl", "cc_legacy_file_group", "cc_toolchain_config") load("//tests/rule_based_toolchain:analysis_test_suite.bzl", "analysis_test_suite") @@ -40,17 +41,24 @@ util.helper_target( env = {"CPP_COMPILE": "1"}, ) +cc_sysroot( + name = "sysroot", + sysroot = "//tests/rule_based_toolchain/testdata:directory", +) + util.helper_target( cc_toolchain_config, name = "collects_files_toolchain_config", - args = [":c_compile_args"], + args = [ + ":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, - sysroot = "//tests/rule_based_toolchain/testdata:directory", tool_map = ":compile_tool_map", ) 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 e2c56be..215d3fe 100644 --- a/tests/rule_based_toolchain/toolchain_config/toolchain_config_test.bzl +++ b/tests/rule_based_toolchain/toolchain_config/toolchain_config_test.bzl @@ -204,12 +204,37 @@ def _toolchain_collects_files_test(env, targets): legacy_feature( name = "implied_by_always_enabled", enabled = True, - flag_sets = [legacy_flag_set( - actions = ["c_compile"], - flag_groups = [ - legacy_flag_group(flags = ["c_compile_args"]), - ], - )], + flag_sets = [ + legacy_flag_set( + actions = [ + "c++-compile", + "c++-header-parsing", + "c++-link-dynamic-library", + "c++-link-executable", + "c++-link-nodeps-dynamic-library", + "c++-module-codegen", + "c++-module-compile", + "c-compile", + "clif-match", + "linkstamp-compile", + "lto-backend", + "lto-index-for-dynamic-library", + "lto-index-for-executable", + "lto-index-for-nodeps-dynamic-library", + ], + flag_groups = [ + legacy_flag_group(flags = [ + "--sysroot=tests/rule_based_toolchain/testdata", + ]), + ], + ), + legacy_flag_set( + actions = ["c_compile"], + flag_groups = [ + legacy_flag_group(flags = ["c_compile_args"]), + ], + ), + ], ), ]).in_order()