Просмотр исходного кода

Don't assume llvm-ar and clang are adjacent. (#1879)

The toolchain embeds the assumption that `clang` and `llvm-ar` are
adjacent, which may not be true on all host platforms. Moreover,
in #1842 and #1843, we test if Homebrow LLVM is in `PATH` by checking
that `llvm-ar` is adjacent to `clang` and if it isn't, we `fail()` the
build, even if `llvm-ar` *is* in `PATH`.

Instead, actually check `PATH` with `repository_ctx.which`. This however
necessitates the assumption that `llvm-ar` and other LLVM binutils
are adjacent, and subsequently that `clang` and `ld.lld` are adjacent.

It appears that we don't seem to always be using these tools, but we
should avoid embedding wrong assumptions regardless.

Update docs to reflect this change.
3405691582 3 лет назад
Родитель
Сommit
72d3d3c75c

+ 7 - 6
bazel/cc_toolchains/clang_cc_toolchain_config.bzl

@@ -21,6 +21,7 @@ load(
     ":clang_detected_variables.bzl",
     "clang_include_dirs_list",
     "clang_resource_dir",
+    "clang_bindir",
     "llvm_bindir",
     "sysroot_dir",
 )
@@ -68,9 +69,9 @@ all_link_actions = [
 def _impl(ctx):
     tool_paths = [
         tool_path(name = "ar", path = llvm_bindir + "/llvm-ar"),
-        tool_path(name = "ld", path = llvm_bindir + "/ld.lld"),
-        tool_path(name = "cpp", path = llvm_bindir + "/clang-cpp"),
-        tool_path(name = "gcc", path = llvm_bindir + "/clang++"),
+        tool_path(name = "ld", path = clang_bindir + "/ld.lld"),
+        tool_path(name = "cpp", path = clang_bindir + "/clang-cpp"),
+        tool_path(name = "gcc", path = clang_bindir + "/clang++"),
         tool_path(name = "dwp", path = llvm_bindir + "/llvm-dwp"),
         tool_path(name = "gcov", path = llvm_bindir + "/llvm-cov"),
         tool_path(name = "nm", path = llvm_bindir + "/llvm-nm"),
@@ -80,13 +81,13 @@ def _impl(ctx):
     ]
 
     action_configs = [
-        action_config(action_name = name, enabled = True, tools = [tool(path = llvm_bindir + "/clang")])
+        action_config(action_name = name, enabled = True, tools = [tool(path = clang_bindir + "/clang")])
         for name in all_c_compile_actions
     ] + [
-        action_config(action_name = name, enabled = True, tools = [tool(path = llvm_bindir + "/clang++")])
+        action_config(action_name = name, enabled = True, tools = [tool(path = clang_bindir + "/clang++")])
         for name in all_cpp_compile_actions
     ] + [
-        action_config(action_name = name, enabled = True, tools = [tool(path = llvm_bindir + "/clang++")])
+        action_config(action_name = name, enabled = True, tools = [tool(path = clang_bindir + "/clang++")])
         for name in all_link_actions
     ] + [
         action_config(action_name = name, enabled = True, tools = [tool(path = llvm_bindir + "/llvm-ar")])

+ 11 - 10
bazel/cc_toolchains/clang_configuration.bzl

@@ -129,21 +129,22 @@ def _configure_clang_toolchain_impl(repository_ctx):
         sysroot_dir,
     )
 
-    # Ensure the detected clang path has some LLVM tools in it, in attempt to
-    # warn the user if they don't have these necessary LLVM tools in their PATH.
-    # This check isn't exhaustive, only a best attempt at warning the developer.
-    llvm_check_result = repository_ctx.execute([
-        clang.dirname.get_child("llvm-ar"),
-        "--version"
-    ])
-    if llvm_check_result.return_code != 0:
-        fail("`llvm-ar` not found beside clang: is LLVM in PATH?")
+    # We expect that the LLVM binutils live adjacent to llvm-ar.
+    # First look for llvm-ar adjacent to clang, so that if found,
+    # it is most likely to match the same version as clang.
+    # Otherwise, try PATH.
+    arpath = clang.dirname.get_child("llvm-ar")
+    if not arpath.exists:
+        arpath = repository_ctx.which("llvm-ar")
+        if not arpath:
+            fail("`llvm-ar` not found in PATH or adjacent to clang")
 
     repository_ctx.template(
         "clang_detected_variables.bzl",
         repository_ctx.attr._clang_detected_variables_template,
         substitutions = {
-            "{LLVM_BINDIR}": str(clang.dirname),
+            "{LLVM_BINDIR}": str(arpath.dirname),
+            "{CLANG_BINDIR}": str(clang.dirname),
             "{CLANG_RESOURCE_DIR}": resource_dir,
             "{CLANG_INCLUDE_DIRS_LIST}": str(
                 [str(path) for path in include_dirs],

+ 1 - 0
bazel/cc_toolchains/clang_detected_variables.tpl.bzl

@@ -9,6 +9,7 @@ This file gets processed by a repository rule, substituting the
 """
 
 llvm_bindir = "{LLVM_BINDIR}"
+clang_bindir = "{CLANG_BINDIR}"
 clang_resource_dir = "{CLANG_RESOURCE_DIR}"
 clang_include_dirs_list = {CLANG_INCLUDE_DIRS_LIST}
 sysroot_dir = "{SYSROOT}"

+ 1 - 1
docs/project/contribution_tools.md

@@ -153,7 +153,7 @@ export PATH="$(brew --prefix llvm)/bin:${PATH}"
 Carbon expects the `PATH` to include the installed tooling. If set, `CC` should
 also point at `clang`. Our build environment will detect the `clang` binary
 using `CC` then `PATH`, and will expect the rest of the LLVM toolchain to be
-available in the same directory as `clang`. However, various scripts and tools
+available in the same directory as `llvm-ar`. However, various scripts and tools
 assume that the LLVM toolchain will be in `PATH`, particularly for tools like
 `clang-format` and `clang-tidy`.