From 329910897f3114f3f5d24407d9abf49b244056d2 Mon Sep 17 00:00:00 2001 From: Siddhartha Bagaria Date: Thu, 14 Mar 2024 14:50:25 -0700 Subject: [PATCH] Replace references of "host" with "exec". (#294) This properly captures that we actually mean the exec platform in these contexts. Also remove host_tools utils because we can not really do that for the exec platform. --- toolchain/cc_toolchain_config.bzl | 26 ++++----- toolchain/internal/common.bzl | 36 ++----------- toolchain/internal/configure.bzl | 66 ++++++++--------------- toolchain/internal/llvm_distributions.bzl | 6 +-- 4 files changed, 43 insertions(+), 91 deletions(-) diff --git a/toolchain/cc_toolchain_config.bzl b/toolchain/cc_toolchain_config.bzl index 00927ac7..434345fa 100644 --- a/toolchain/cc_toolchain_config.bzl +++ b/toolchain/cc_toolchain_config.bzl @@ -19,7 +19,6 @@ load( load( "//toolchain/internal:common.bzl", _check_os_arch_keys = "check_os_arch_keys", - _host_tools = "host_tools", _os_arch_pair = "os_arch_pair", ) @@ -32,8 +31,8 @@ def _fmt_flags(flags, toolchain_path_prefix): # right paths and flags for the tools. def cc_toolchain_config( name, - host_arch, - host_os, + exec_arch, + exec_os, target_arch, target_os, target_system_name, @@ -41,16 +40,14 @@ def cc_toolchain_config( tools_path_prefix, wrapper_bin_prefix, compiler_configuration, - cxx_builtin_include_directories, - host_tools_info = {}): - host_os_arch_key = _os_arch_pair(host_os, host_arch) + cxx_builtin_include_directories): + exec_os_arch_key = _os_arch_pair(exec_os, exec_arch) target_os_arch_key = _os_arch_pair(target_os, target_arch) - _check_os_arch_keys([host_os_arch_key, target_os_arch_key]) + _check_os_arch_keys([exec_os_arch_key, target_os_arch_key]) # A bunch of variables that get passed straight through to # `create_cc_toolchain_config_info`. # TODO: What do these values mean, and are they actually all correct? - host_system_name = host_arch ( toolchain_identifier, target_cpu, @@ -107,7 +104,7 @@ def cc_toolchain_config( "-fdebug-prefix-map={}=__bazel_toolchain_llvm_repo__/".format(toolchain_path_prefix), ] - is_xcompile = not (host_os == target_os and host_arch == target_arch) + is_xcompile = not (exec_os == target_os and exec_arch == target_arch) # Default compiler flags: compile_flags = [ @@ -148,7 +145,7 @@ def cc_toolchain_config( archive_flags = [] # Linker flags: - if host_os == "darwin" and not is_xcompile: + if exec_os == "darwin" and not is_xcompile: # lld is experimental for Mach-O, so we use the native ld64 linker. # TODO: How do we cross-compile from Linux to Darwin? use_lld = False @@ -264,7 +261,10 @@ def cc_toolchain_config( ## NOTE: make variables are missing here; unix_cc_toolchain_config doesn't ## pass these to `create_cc_toolchain_config_info`. - # The tool names come from [here](https://github.com/bazelbuild/bazel/blob/c7e58e6ce0a78fdaff2d716b4864a5ace8917626/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java#L76-L90): + # The requirements here come from + # https://cs.opensource.google/bazel/bazel/+/master:src/main/starlark/builtins_bzl/common/cc/cc_toolchain_provider_helper.bzl;l=75;drc=f0150efd1cca473640269caaf92b5a23c288089d + # https://cs.opensource.google/bazel/bazel/+/master:src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java;l=1257;drc=6743d76f9ecde726d592e88d8914b9db007b1c43 + # https://cs.opensource.google/bazel/bazel/+/refs/tags/7.0.0:tools/cpp/unix_cc_toolchain_config.bzl;l=192,201;drc=044a14cca2747aeff258fc71eaeb153c08cb34d5 # NOTE: Ensure these are listed in toolchain_tools in toolchain/internal/common.bzl. tool_paths = { "ar": tools_path_prefix + ("llvm-ar" if not use_libtool else "libtool"), @@ -272,7 +272,7 @@ def cc_toolchain_config( "dwp": tools_path_prefix + "llvm-dwp", "gcc": wrapper_bin_prefix + "cc_wrapper.sh", "gcov": tools_path_prefix + "llvm-profdata", - "ld": tools_path_prefix + "ld.lld" if use_lld else _host_tools.get_and_assert(host_tools_info, "ld"), + "ld": tools_path_prefix + "ld.lld" if use_lld else "/usr/bin/ld", "llvm-cov": tools_path_prefix + "llvm-cov", "llvm-profdata": tools_path_prefix + "llvm-profdata", "nm": tools_path_prefix + "llvm-nm", @@ -319,7 +319,7 @@ def cc_toolchain_config( cpu = target_cpu, compiler = compiler, toolchain_identifier = toolchain_identifier, - host_system_name = host_system_name, + host_system_name = exec_arch, target_system_name = target_system_name, target_libc = target_libc, abi_version = abi_version, diff --git a/toolchain/internal/common.bzl b/toolchain/internal/common.bzl index 19347a6b..ff94bfb2 100644 --- a/toolchain/internal/common.bzl +++ b/toolchain/internal/common.bzl @@ -15,6 +15,7 @@ SUPPORTED_TARGETS = [("linux", "x86_64"), ("linux", "aarch64"), ("darwin", "x86_64"), ("darwin", "aarch64")] # Map of tool name to its symlinked name in the tools directory. +# See tool_paths in toolchain/cc_toolchain_config.bzl. _toolchain_tools = { name: name for name in [ @@ -40,7 +41,7 @@ _toolchain_tools_darwin = { "llvm-libtool-darwin": "libtool", } -def host_os_key(rctx): +def exec_os_key(rctx): (os, version, arch) = os_version_arch(rctx) if version == "": return "%s-%s" % (os, arch) @@ -154,14 +155,14 @@ def check_os_arch_keys(keys): keys = ", ".join(_supported_os_arch), )) -def host_os_arch_dict_value(rctx, attr_name, debug = False): +def exec_os_arch_dict_value(rctx, attr_name, debug = False): # Gets a value from a dictionary keyed by host OS and arch. # Checks for the more specific key, then the less specific, # and finally the empty key as fallback. # Returns a tuple of the matching key and value. d = getattr(rctx.attr, attr_name) - key1 = host_os_key(rctx) + key1 = exec_os_key(rctx) if key1 in d: return (key1, d.get(key1)) @@ -230,32 +231,3 @@ def toolchain_tools(os): if os == "darwin": tools.update(_toolchain_tools_darwin) return tools - -def _get_host_tool_info(rctx, tool_path, tool_key = None): - if tool_key == None: - tool_key = tool_path - - if tool_path == None or not rctx.path(tool_path).exists: - return {} - - return { - tool_key: struct( - path = tool_path, - features = [], - ), - } - -def _extract_tool_path(tool_info): - # Have to support structs or dicts: - return tool_info.path if type(tool_info) == "struct" else tool_info["path"] - -def _get_host_tool(host_tool_info, tool_key): - if tool_key in host_tool_info: - return _extract_tool_path(host_tool_info[tool_key]) - else: - return None - -host_tools = struct( - get_tool_info = _get_host_tool_info, - get_and_assert = _get_host_tool, -) diff --git a/toolchain/internal/configure.bzl b/toolchain/internal/configure.bzl index e4c9e9bb..067e486d 100644 --- a/toolchain/internal/configure.bzl +++ b/toolchain/internal/configure.bzl @@ -22,8 +22,7 @@ load( _arch = "arch", _canonical_dir_path = "canonical_dir_path", _check_os_arch_keys = "check_os_arch_keys", - _host_os_arch_dict_value = "host_os_arch_dict_value", - _host_tools = "host_tools", + _exec_os_arch_dict_value = "exec_os_arch_dict_value", _is_absolute_path = "is_absolute_path", _list_to_string = "list_to_string", _os = "os", @@ -63,11 +62,11 @@ def llvm_config_impl(rctx): if not rctx.attr.toolchain_roots: toolchain_root = ("@" if BZLMOD_ENABLED else "") + "@%s_llvm//" % rctx.attr.name else: - (_key, toolchain_root) = _host_os_arch_dict_value(rctx, "toolchain_roots") + (_key, toolchain_root) = _exec_os_arch_dict_value(rctx, "toolchain_roots") if not toolchain_root: fail("LLVM toolchain root missing for ({}, {})".format(os, arch)) - (_key, llvm_version) = _host_os_arch_dict_value(rctx, "llvm_versions") + (_key, llvm_version) = _exec_os_arch_dict_value(rctx, "llvm_versions") if not llvm_version: # LLVM version missing for (os, arch) _empty_repository(rctx) @@ -161,22 +160,12 @@ def llvm_config_impl(rctx): llvm_version = llvm_version, extra_compiler_files = rctx.attr.extra_compiler_files, ) - host_dl_ext = "dylib" if os == "darwin" else "so" - host_tools_info = dict([ - pair - for (key, tool_path) in [ - # This is used when lld doesn't support the target platform (i.e. - # Mach-O for macOS): - ("ld", "/usr/bin/ld"), - ] - for pair in _host_tools.get_tool_info(rctx, tool_path, key).items() - ]) + exec_dl_ext = "dylib" if os == "darwin" else "so" cc_toolchains_str, toolchain_labels_str = _cc_toolchains_str( rctx, workspace_name, toolchain_info, use_absolute_paths_llvm, - host_tools_info, ) convenience_targets_str = _convenience_targets_str( @@ -184,7 +173,7 @@ def llvm_config_impl(rctx): use_absolute_paths_llvm, llvm_dist_rel_path, llvm_dist_label_prefix, - host_dl_ext, + exec_dl_ext, ) # Convenience macro to register all generated toolchains. @@ -226,8 +215,7 @@ def _cc_toolchains_str( rctx, workspace_name, toolchain_info, - use_absolute_paths_llvm, - host_tools_info): + use_absolute_paths_llvm): # Since all the toolchains rely on downloading the right LLVM toolchain for # the host architecture, we don't need to explicitly specify # `exec_compatible_with` attribute. If the host and execution platform are @@ -252,7 +240,6 @@ def _cc_toolchains_str( target_arch, toolchain_info, use_absolute_paths_llvm, - host_tools_info, ) if cc_toolchain_str: cc_toolchains_str = cc_toolchains_str + cc_toolchain_str @@ -275,12 +262,11 @@ def _cc_toolchain_str( target_os, target_arch, toolchain_info, - use_absolute_paths_llvm, - host_tools_info): - host_os = toolchain_info.os - host_arch = toolchain_info.arch + use_absolute_paths_llvm): + exec_os = toolchain_info.os + exec_arch = toolchain_info.arch - host_os_bzl = _os_bzl(host_os) + exec_os_bzl = _os_bzl(exec_os) target_os_bzl = _os_bzl(target_os) target_pair = _os_arch_pair(target_os, target_arch) @@ -293,9 +279,9 @@ def _cc_toolchain_str( sysroot_label_str = "" if not sysroot_path: - if host_os == target_os and host_arch == target_arch: + if exec_os == target_os and exec_arch == target_arch: # For darwin -> darwin, we can use the macOS SDK path. - sysroot_path = _default_sysroot_path(rctx, host_os) + sysroot_path = _default_sysroot_path(rctx, exec_os) else: # We are trying to cross-compile without a sysroot, let's bail. # TODO: Are there situations where we can continue? @@ -303,10 +289,6 @@ def _cc_toolchain_str( extra_files_str = "\":internal-use-files\"" - # `struct` isn't allowed in `BUILD` files so we JSON encode + decode to turn - # them into `dict`s. - host_tools_info = json.decode(json.encode(host_tools_info)) - # C++ built-in include directories. # This contains both the includes shipped with the compiler as well as the sysroot (or host) # include directories. While Bazel's default undeclared inclusions check does not seem to be @@ -358,8 +340,8 @@ def _cc_toolchain_str( cc_toolchain_config( name = "local-{suffix}", - host_arch = "{host_arch}", - host_os = "{host_os}", + exec_arch = "{exec_arch}", + exec_os = "{exec_os}", target_arch = "{target_arch}", target_os = "{target_os}", target_system_name = "{target_system_name}", @@ -382,15 +364,14 @@ cc_toolchain_config( "coverage_link_flags": {coverage_link_flags}, "unfiltered_compile_flags": {unfiltered_compile_flags}, }}, - host_tools_info = {host_tools_info}, cxx_builtin_include_directories = {cxx_builtin_include_directories}, ) toolchain( name = "cc-toolchain-{suffix}", exec_compatible_with = [ - "@platforms//cpu:{host_arch}", - "@platforms//os:{host_os_bzl}", + "@platforms//cpu:{exec_arch}", + "@platforms//os:{exec_os_bzl}", ], target_compatible_with = [ "@platforms//cpu:{target_arch}", @@ -518,12 +499,12 @@ cc_toolchain( suffix = suffix, target_os = target_os, target_arch = target_arch, - host_os = host_os, - host_arch = host_arch, + exec_os = exec_os, + exec_arch = exec_arch, target_settings = _list_to_string(_dict_value(toolchain_info.target_settings_dict, target_pair)), target_os_bzl = target_os_bzl, target_system_name = target_system_name, - host_os_bzl = host_os_bzl, + exec_os_bzl = exec_os_bzl, llvm_dist_label_prefix = toolchain_info.llvm_dist_label_prefix, llvm_dist_path_prefix = toolchain_info.llvm_dist_path_prefix, tools_path_prefix = toolchain_info.tools_path_prefix, @@ -544,7 +525,6 @@ cc_toolchain( coverage_link_flags = _list_to_string(_dict_value(toolchain_info.coverage_link_flags_dict, target_pair)), unfiltered_compile_flags = _list_to_string(_dict_value(toolchain_info.unfiltered_compile_flags_dict, target_pair)), extra_files_str = extra_files_str, - host_tools_info = host_tools_info, cxx_builtin_include_directories = _list_to_string([ # Filter out non-existing directories with absolute paths as they # result in a -Wincomplete-umbrella warning when mentioned in the @@ -556,12 +536,12 @@ cc_toolchain( extra_compiler_files = ("\"%s\"," % str(toolchain_info.extra_compiler_files)) if toolchain_info.extra_compiler_files else "", ) -def _convenience_targets_str(rctx, use_absolute_paths, llvm_dist_rel_path, llvm_dist_label_prefix, host_dl_ext): +def _convenience_targets_str(rctx, use_absolute_paths, llvm_dist_rel_path, llvm_dist_label_prefix, exec_dl_ext): if use_absolute_paths: llvm_dist_label_prefix = ":" filenames = [] for libname in _aliased_libs: - filename = "lib/{}.{}".format(libname, host_dl_ext) + filename = "lib/{}.{}".format(libname, exec_dl_ext) filenames.append(filename) for toolname in _aliased_tools: filename = "bin/{}".format(toolname) @@ -575,7 +555,7 @@ def _convenience_targets_str(rctx, use_absolute_paths, llvm_dist_rel_path, llvm_ template = """ cc_import( name = "{name}", - shared_library = "{{llvm_dist_label_prefix}}lib/lib{name}.{{host_dl_ext}}", + shared_library = "{{llvm_dist_label_prefix}}lib/lib{name}.{{exec_dl_ext}}", )""".format(name = name) lib_target_strs.append(template) @@ -591,7 +571,7 @@ native_binary( return "\n".join(lib_target_strs + tool_target_strs).format( llvm_dist_label_prefix = llvm_dist_label_prefix, - host_dl_ext = host_dl_ext, + exec_dl_ext = exec_dl_ext, ) def _is_hermetic_or_exists(rctx, path, sysroot_prefix): diff --git a/toolchain/internal/llvm_distributions.bzl b/toolchain/internal/llvm_distributions.bzl index 97eecfc9..5d99c6d6 100644 --- a/toolchain/internal/llvm_distributions.bzl +++ b/toolchain/internal/llvm_distributions.bzl @@ -13,7 +13,7 @@ # limitations under the License. load("@bazel_tools//tools/build_defs/repo:utils.bzl", "read_netrc", "use_netrc") -load("//toolchain/internal:common.bzl", _arch = "arch", _attr_dict = "attr_dict", _host_os_arch_dict_value = "host_os_arch_dict_value", _os = "os") +load("//toolchain/internal:common.bzl", _arch = "arch", _attr_dict = "attr_dict", _exec_os_arch_dict_value = "exec_os_arch_dict_value", _os = "os") load("//toolchain/internal:release_name.bzl", _llvm_release_name = "llvm_release_name") # If a new LLVM version is missing from this list, please add the shasums here @@ -547,7 +547,7 @@ def download_llvm(rctx): return updated_attrs def _urls(rctx): - (key, urls) = _host_os_arch_dict_value(rctx, "urls", debug = False) + (key, urls) = _exec_os_arch_dict_value(rctx, "urls", debug = False) if not urls: print("LLVM archive URLs missing and no default fallback provided; will try 'distribution' attribute") # buildifier: disable=print @@ -561,7 +561,7 @@ def _get_llvm_version(rctx): return rctx.attr.llvm_version if not rctx.attr.llvm_versions: fail("Neither 'llvm_version' nor 'llvm_versions' given.") - (_, llvm_version) = _host_os_arch_dict_value(rctx, "llvm_versions") + (_, llvm_version) = _exec_os_arch_dict_value(rctx, "llvm_versions") if not llvm_version: fail("LLVM version string missing for ({os}, {arch})", os = _os(rctx), arch = _arch(rctx)) return llvm_version