Use source directories for toolchain includes and wrapper tools

Author: dzbarsky

.github/CODEOWNERS

@@ -1 +1 @@
-* @rrbutani @jsharpe @fmeum @helly25
+* @rrbutani @jsharpe @fmeum @helly25 @dzbarsky

toolchain/BUILD.llvm_repo renamed to toolchain/BUILD.llvm_repo.tpl

@@ -61,27 +61,30 @@ filegroup(
     ),
 )
 
+filegroup(
+    name = "cxx_builtin_include",
+    srcs = [
+        "include/c++",
+        "lib/clang/{LLVM_VERSION}/include",
+    ],
+)
+
 filegroup(
     name = "bin",
     srcs = glob(["bin/**"]),
 )
 
 filegroup(
     name = "lib",
-    srcs = glob(
-        [
-            "lib/**/libc++*.a",
-            "lib/**/libunwind.a",
-            # TODO(zbarsky): `lib/clang/<VERSION>` can be a source directory
-            "lib/clang/*/lib/**",
-            # clang_rt.*.o supply crtbegin and crtend sections.
-            "lib/**/clang_rt.*.o",
-        ],
-        allow_empty = True,
-    ),
-    # Include the .dylib files in the linker sandbox even though they will
-    # not be available at runtime to allow sanitizers to work locally.
-    # Any library linked from the toolchain to be released should be linked statically.
+    srcs = [
+        # Include the .dylib files in the linker sandbox even though they will
+        # not be available at runtime to allow sanitizers to work locally.
+        # Any library linked from the toolchain to be released should be linked statically.
+        "lib/clang/{LLVM_VERSION}/lib",
+    ] + glob([
+        "lib/**/libc++*.a",
+        "lib/**/libunwind.a",
+    ]),
 )
 
 filegroup(

toolchain/BUILD.toolchain.tpl

@@ -19,33 +19,13 @@ load("@rules_cc//cc:defs.bzl", "cc_toolchain", "cc_toolchain_suite")
 load("@toolchains_llvm//toolchain/internal:system_module_map.bzl", "system_module_map")
 load("%{cc_toolchain_config_bzl}", "cc_toolchain_config")
 
-# Following filegroup targets are used when not using absolute paths and shared
+# This filegroup target is used when not using absolute paths and shared
 # between different toolchains.
 
-# Tools symlinked through this repo. This target is for internal use in the toolchain only.
+# Tools wrapped and symlinked through this repo. This target is for internal use in the toolchain only.
 filegroup(
-    name = "internal-use-symlinked-tools",
-    srcs = [%{symlinked_tools}
-    ],
-    visibility = ["//visibility:private"],
-)
-
-# Tools wrapped through this repo. This target is for internal use in the toolchain only.
-filegroup(
-    name = "internal-use-wrapped-tools",
-    srcs = [
-        "%{wrapper_bin_prefix}cc_wrapper.sh",
-    ],
-    visibility = ["//visibility:private"],
-)
-
-# All internal use files.
-filegroup(
-    name = "internal-use-files",
-    srcs = [
-        ":internal-use-symlinked-tools",
-        ":internal-use-wrapped-tools",
-    ],
+    name = "internal-use-tools",
+    srcs = ["%{tools_dir}"],
     visibility = ["//visibility:private"],
 )
 

toolchain/internal/configure.bzl

@@ -135,10 +135,6 @@ def llvm_config_impl(rctx):
         tools = _toolchain_tools(os)
         for tool_name, symlink_name in tools.items():
             rctx.symlink(llvm_dist_rel_path + "bin/" + tool_name, tools_path_prefix + symlink_name)
-        symlinked_tools_str = "".join([
-            "\n" + (" " * 8) + "\"" + tools_path_prefix + symlink_name + "\","
-            for symlink_name in tools.values()
-        ])
     else:
         llvm_dist_rel_path = llvm_dist_path_prefix
         llvm_dist_label_prefix = llvm_dist_path_prefix
@@ -147,7 +143,6 @@ def llvm_config_impl(rctx):
         # No symlinking necessary when using absolute paths.
         wrapper_bin_prefix = "bin/"
         tools_path_prefix = llvm_dist_path_prefix + "bin/"
-        symlinked_tools_str = ""
 
     sysroot_paths_dict, sysroot_labels_dict = _sysroot_paths_dict(
         rctx,
@@ -230,8 +225,7 @@ def llvm_config_impl(rctx):
         {
             "%{cc_toolchain_config_bzl}": str(rctx.attr._cc_toolchain_config_bzl),
             "%{cc_toolchains}": cc_toolchains_str,
-            "%{symlinked_tools}": symlinked_tools_str,
-            "%{wrapper_bin_prefix}": wrapper_bin_prefix,
+            "%{tools_dir}": wrapper_bin_prefix.removesuffix("/"),
             "%{convenience_targets}": convenience_targets_str,
         },
     )
@@ -335,7 +329,7 @@ def _cc_toolchain_str(
             # TODO: Are there other situations where we can continue?
             return ""
 
-    extra_files_str = "\":internal-use-files\""
+    extra_files_str = repr(":internal-use-tools")
 
     # C++ built-in include directories.
     # This contains both the includes shipped with the compiler as well as the sysroot (or host)
@@ -360,15 +354,12 @@ def _cc_toolchain_str(
         "wasip1-wasm32": "wasm32-wasip1",
         "wasip1-wasm64": "wasm64-wasip1",
     }[target_pair]
+
     cxx_builtin_include_directories = [
         toolchain_path_prefix + "include/c++/v1",
-        toolchain_path_prefix + "include/{}/c++/v1".format(target_system_name),
-        toolchain_path_prefix + "lib/clang/{}/include".format(llvm_version),
-        toolchain_path_prefix + "lib/clang/{}/share".format(llvm_version),
-        toolchain_path_prefix + "lib64/clang/{}/include".format(llvm_version),
-        toolchain_path_prefix + "lib/clang/{}/include".format(major_llvm_version),
-        toolchain_path_prefix + "lib/clang/{}/share".format(major_llvm_version),
-        toolchain_path_prefix + "lib64/clang/{}/include".format(major_llvm_version),
+        toolchain_path_prefix + "lib/clang/{}/include".format(
+            major_llvm_version if major_llvm_version >= 16 else llvm_version,
+        ),
     ]
 
     sysroot_prefix = ""
@@ -505,17 +496,15 @@ filegroup(name = "strip-files-{suffix}", srcs = [{extra_files_str}])
         template = template + """
 filegroup(
     name = "cxx_builtin_include_files-{suffix}",
-    srcs = [
-        "{llvm_dist_label_prefix}clang",
-        "{llvm_dist_label_prefix}include",
-    ],
+    srcs = ["{llvm_dist_label_prefix}cxx_builtin_include"],
 )
 
 filegroup(
     name = "compiler-components-{suffix}",
     srcs = [
         ":cxx_builtin_include_files-{suffix}",
         ":sysroot-components-{suffix}",
+        "{llvm_dist_label_prefix}clang",
         {extra_compiler_files}
     ],
 )

toolchain/internal/repo.bzl

@@ -14,6 +14,7 @@
 load(
     "//toolchain/internal:common.bzl",
     _attr_dict = "attr_dict",
+    _exec_os_arch_dict_value = "exec_os_arch_dict_value",
     _os = "os",
     _supported_os_arch_keys = "supported_os_arch_keys",
 )
@@ -422,9 +423,16 @@ def llvm_repo_impl(rctx):
         rctx.file("BUILD.bazel", executable = False)
         return None
 
+    (_key, llvm_version) = _exec_os_arch_dict_value(rctx, "llvm_versions")
+
+    major_llvm_version = int(llvm_version.split(".")[0])
+
     rctx.file(
         "BUILD.bazel",
-        content = rctx.read(Label("//toolchain:BUILD.llvm_repo")),
+        content = rctx.read(Label("//toolchain:BUILD.llvm_repo.tpl")).format(
+            # The versioning changed.
+            LLVM_VERSION = major_llvm_version if major_llvm_version >= 16 else llvm_version,
+        ),
         executable = False,
     )
 

toolchain/internal/system_module_map.bzl

@@ -55,20 +55,29 @@ def _system_module_map(ctx):
         execroot_prefix = execroot_prefix,
     )
 
+    umbrella_submodule_closure = lambda file: _umbrella_submodule(
+        execroot_prefix + paths.normalize(file.path).replace("//", "/"),
+    )
+
     template_dict = ctx.actions.template_dict()
+
+    cxx_builtin_files = ctx.attr.cxx_builtin_include_files[DefaultInfo].files
+    if len(cxx_builtin_files.to_list()) == 2:
+        cxx_builtin_include_files_closure = umbrella_submodule_closure
+    else:
+        cxx_builtin_include_files_closure = textual_header_closure
     template_dict.add_joined(
         "%cxx_builtin_include_files%",
-        ctx.attr.cxx_builtin_include_files[DefaultInfo].files,
+        cxx_builtin_files,
         join_with = "\n",
-        map_each = textual_header_closure,
+        map_each = cxx_builtin_include_files_closure,
         allow_closure = True,
     )
 
     # We don't have a good way to detect a source directory, so check if it's a single File...
     sysroot_files = ctx.attr.sysroot_files[DefaultInfo].files.to_list()
     if len(sysroot_files) == 1:
-        path = paths.normalize(sysroot_files[0].path).replace("//", "/")
-        template_dict.add("%sysroot%", _umbrella_submodule(execroot_prefix + path))
+        template_dict.add("%sysroot%", umbrella_submodule_closure(sysroot_files[0]))
     else:
         if sysroot_files:
             # buildifier: disable=print