Merge remote-tracking branch 'origin/master' into bzlmod

.github/workflows/tests.yml

@@ -13,11 +13,19 @@ jobs:
       fail-fast: false
       matrix:
         os: [macos-latest, ubuntu-latest]
+<<<<<<< HEAD
         bazel_version: [latest, 6.2.0, 5.1.0]
         bzlmod: [yes, no]
         exclude:
           - bzlmod: yes
             bazel_version: 5.1.0
+=======
+        script: [run_tests.sh, run_external_tests.sh]
+        bazel_version: [latest, 6.0.0]
+        exclude:
+          - script: run_external_tests.sh
+            bazel_version: 6.0.0
+>>>>>>> origin/master
     steps:
     - uses: actions/checkout@v3
     - name: test

README.md

@@ -20,7 +20,7 @@ implementation, please let me know and I can redirect people there.
 
 ## Quickstart
 
-Minimum bazel version: **4.2.1**
+Minimum bazel version: **6.0.0**
 
 If you're using `bzlmod`, add the following to `MODULE.bazel`:
 
@@ -70,10 +70,14 @@ load("@llvm_toolchain//:toolchains.bzl", "llvm_register_toolchains")
 llvm_register_toolchains()
 ```
 
-And add the following section to your .bazelrc file (not needed after
-this [issue](https://github.com/bazelbuild/bazel/issues/7260) is closed):
+And add the following section to your .bazelrc file:
 ```
+# Not needed after https://github.com/bazelbuild/bazel/issues/7260 is closed
 build --incompatible_enable_cc_toolchain_resolution
+
+# Tell Bazel to pass the right flags for llvm-ar, not libtool. Only needed if you are building on darwin.
+# See https://github.com/bazelbuild/bazel/blob/5c75d0acec21459bbb13520817e3806e1507e907/tools/cpp/unix_cc_toolchain_config.bzl#L1000-L1024
+build --features=-libtool
 ```
 
 ## Basic Usage

tests/.bazelrc

@@ -1 +1,2 @@
 build --incompatible_enable_cc_toolchain_resolution
+build --features=-libtool

tests/MODULE.bazel

@@ -3,9 +3,9 @@ module(name = "com_grail_bazel_toolchain_tests")
 bazel_dep(name = "grail_llvm_toolchain", version = "0.8.2")
 local_path_override(module_name = "grail_llvm_toolchain", path = "..")
 
-bazel_dep(name = "bazel_skylib", version = "1.4.1")
+bazel_dep(name = "bazel_skylib", version = "1.4.2")
 bazel_dep(name = "platforms", version = "0.0.6")
-bazel_dep(name = "rules_cc", version = "0.0.6")
+bazel_dep(name = "rules_cc", version = "0.0.8")
 bazel_dep(name = "rules_go", version = "0.40.1", repo_name = "io_bazel_rules_go")
 bazel_dep(name = "rules_foreign_cc", version = "0.9.0")
 bazel_dep(name = "rules_rust", version = "0.25.1")

toolchain/BUILD.toolchain.tpl

@@ -34,7 +34,6 @@ filegroup(
     name = "internal-use-wrapped-tools",
     srcs = [
         "%{wrapper_bin_prefix}cc_wrapper.sh",
-        "%{wrapper_bin_prefix}host_libtool_wrapper.sh",
     ],
     visibility = ["//visibility:private"],
 )

toolchain/cc_toolchain_config.bzl

@@ -19,7 +19,6 @@ load(
 load(
     "//toolchain/internal:common.bzl",
     _check_os_arch_keys = "check_os_arch_keys",
-    _host_tool_features = "host_tool_features",
     _host_tools = "host_tools",
     _os_arch_pair = "os_arch_pair",
 )
@@ -295,29 +294,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`.
 
-    # Tool paths:
-    # `llvm-strip` was introduced in V7 (https://reviews.llvm.org/D46407):
-    llvm_version = llvm_version.split(".")
-    llvm_major_ver = int(llvm_version[0]) if len(llvm_version) else 0
-    strip_binary = (tools_path_prefix + "llvm-strip") if llvm_major_ver >= 7 else _host_tools.get_and_assert(host_tools_info, "strip")
-
-    # TODO: The command line formed on darwin does not work with llvm-ar.
-    ar_binary = tools_path_prefix + "llvm-ar"
-    if host_os == "darwin":
-        # Bazel uses arg files for longer commands; some old macOS `libtool`
-        # versions do not support this.
-        #
-        # In these cases we want to use `libtool_wrapper.sh` which translates
-        # the arg file back into command line arguments.
-        if not _host_tools.tool_supports(host_tools_info, "libtool", features = [_host_tool_features.SUPPORTS_ARG_FILE]):
-            ar_binary = wrapper_bin_prefix + "host_libtool_wrapper.sh"
-        else:
-            ar_binary = host_tools_info["libtool"]["path"]
-
     # 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):
     # NOTE: Ensure these are listed in toolchain_tools in toolchain/internal/common.bzl.
     tool_paths = {
-        "ar": ar_binary,
+        "ar": tools_path_prefix + "llvm-ar",
         "cpp": tools_path_prefix + "clang-cpp",
         "dwp": tools_path_prefix + "llvm-dwp",
         "gcc": wrapper_bin_prefix + "cc_wrapper.sh",
@@ -328,7 +308,7 @@ def cc_toolchain_config(
         "nm": tools_path_prefix + "llvm-nm",
         "objcopy": tools_path_prefix + "llvm-objcopy",
         "objdump": tools_path_prefix + "llvm-objdump",
-        "strip": strip_binary,
+        "strip": tools_path_prefix + "llvm-strip",
     }
 
     # Start-end group linker support:

toolchain/internal/common.bzl

@@ -14,10 +14,6 @@
 
 SUPPORTED_TARGETS = [("linux", "x86_64"), ("linux", "aarch64"), ("darwin", "x86_64"), ("darwin", "aarch64")]
 
-host_tool_features = struct(
-    SUPPORTS_ARG_FILE = "supports_arg_file",
-)
-
 toolchain_tools = [
     "clang-cpp",
     "ld.lld",
@@ -181,103 +177,31 @@ def attr_dict(attr):
 
     return dict(tuples)
 
-# Tries to figure out if a tool supports newline separated arg files (i.e.
-# `@file`).
-def _tool_supports_arg_file(rctx, tool_path):
-    # We assume nothing other than that `tool_path` is an executable.
-    #
-    # First we have to find out what command line flag gets the tool to just
-    # print out some text and exit successfully.
-    #
-    # Most tools support `-v` or `--version` or (for `libtool`) `-V` but some
-    # tools don't have such an option (BSD `ranlib` and `ar`, for example).
-    #
-    # We just try all the options we know of until one works and if none work
-    # we return "None" indicating an indeterminate result.
-    opts = (
-        ["-v", "--version", "-version", "-V"] +
-        ["-h", "--help", "-help", "-H"]
-    )
-
-    no_op_opt = None
-    for opt in opts:
-        if rctx.execute([tool_path, opt], timeout = 2).return_code == 0:
-            no_op_opt = opt
-            break
-
-    if no_op_opt == None:
-        return None
-
-    # Okay! Once we have an opt that we *know* does nothing but make the
-    # executable exit successfully, we'll stick that opt in a file and try
-    # again:
-    tmp_file = "tmp-arg-file"
-    rctx.file(tmp_file, content = "{}\n".format(no_op_opt), executable = False)
-
-    res = rctx.execute([tool_path, "@{}".format(tmp_file)]).return_code == 0
-    rctx.delete(tmp_file)
-
-    return res
-
-def _get_host_tool_info(rctx, tool_path, features_to_test = [], tool_key = None):
+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 {}
 
-    f = host_tool_features
-    features = {}
-    for feature in features_to_test:
-        features[feature] = {
-            f.SUPPORTS_ARG_FILE: _tool_supports_arg_file,
-        }[feature](rctx, tool_path)
-
     return {
         tool_key: struct(
             path = tool_path,
-            features = features,
+            features = [],
         ),
     }
 
-def _extract_tool_path_and_features(tool_info):
+def _extract_tool_path(tool_info):
     # Have to support structs or dicts:
-    tool_path = tool_info.path if type(tool_info) == "struct" else tool_info["path"]
-    tool_features = tool_info.features if type(tool_info) == "struct" else tool_info["features"]
+    return tool_info.path if type(tool_info) == "struct" else tool_info["path"]
 
-    return (tool_path, tool_features)
-
-def _check_host_tool_supports(host_tool_info, tool_key, features = []):
+def _get_host_tool(host_tool_info, tool_key):
     if tool_key in host_tool_info:
-        _, tool_features = _extract_tool_path_and_features(host_tool_info[tool_key])
-
-        for f in features:
-            if not f in tool_features or not tool_features[f]:
-                return False
-
-        return True
+        return _extract_tool_path(host_tool_info[tool_key])
     else:
-        return False
-
-def _get_host_tool_and_assert_supports(host_tool_info, tool_key, features = []):
-    if tool_key in host_tool_info:
-        tool_path, tool_features = _extract_tool_path_and_features(host_tool_info[tool_key])
-
-        missing = [f for f in features if not f in tool_features or not tool_features[f]]
-
-        if missing:
-            fail("Host tool `{key}` (`{path}`) is missing these features: `{missing}`.".format(
-                key = tool_key,
-                path = tool_path,
-                missing = missing,
-            ))
-
-        return tool_path
-    else:
-        return False
+        return None
 
 host_tools = struct(
     get_tool_info = _get_host_tool_info,
-    tool_supports = _check_host_tool_supports,
-    get_and_assert = _get_host_tool_and_assert_supports,
+    get_and_assert = _get_host_tool,
 )

toolchain/internal/configure.bzl

@@ -18,13 +18,11 @@ load(
     _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_tool_features = "host_tool_features",
     _host_tools = "host_tools",
     _list_to_string = "list_to_string",
     _os = "os",
     _os_arch_pair = "os_arch_pair",
     _os_bzl = "os_bzl",
-    _pkg_name_from_label = "pkg_name_from_label",
     _pkg_path_from_label = "pkg_path_from_label",
     _supported_targets = "SUPPORTED_TARGETS",
     _toolchain_tools = "toolchain_tools",
@@ -166,16 +164,12 @@ def llvm_register_toolchains():
     host_dl_ext = "dylib" if os == "darwin" else "so"
     host_tools_info = dict([
         pair
-        for (key, tool_path, features) in [
-            # This is used for macOS hosts:
-            ("libtool", "/usr/bin/libtool", [_host_tool_features.SUPPORTS_ARG_FILE]),
-            # This is used with old (pre 7) LLVM versions:
-            ("strip", "/usr/bin/strip", []),
+        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", []),
+            ("ld", "/usr/bin/ld"),
         ]
-        for pair in _host_tools.get_tool_info(rctx, tool_path, features, key).items()
+        for pair in _host_tools.get_tool_info(rctx, tool_path, key).items()
     ])
     cc_toolchains_str, toolchain_labels_str = _cc_toolchains_str(
         workspace_name,
@@ -227,15 +221,6 @@ def llvm_register_toolchains():
         },
     )
 
-    # libtool wrapper; used if the host libtool doesn't support arg files:
-    rctx.template(
-        "bin/host_libtool_wrapper.sh",
-        rctx.attr._host_libtool_wrapper_sh_tpl,
-        {
-            "%{libtool_path}": "/usr/bin/libtool",
-        },
-    )
-
 def _cc_toolchains_str(
         workspace_name,
         toolchain_info,