Use source directories for toolchain includes and wrapper tools (#580)

Compile and linker action inputs should now be fully optimized:
```
➜  tests git:(zbarsky/toolchain) bazel cquery --output=files @llvm_toolchain_with_sysroot//:compiler-files-aarch64-darwin
INFO: Found 1 target...
external/toolchains_llvm++llvm+llvm_toolchain_llvm/include/c++
external/toolchains_llvm++llvm+llvm_toolchain_llvm/lib/clang/16/include
external/toolchains_llvm++llvm+llvm_toolchain_llvm/bin/clang
external/toolchains_llvm++llvm+llvm_toolchain_llvm/bin/clang++
external/toolchains_llvm++llvm+llvm_toolchain_llvm/bin/clang-cpp
external/toolchains_llvm++llvm+llvm_toolchain_with_sysroot/bin
INFO: Elapsed time: 0.308s, Critical Path: 0.00s
INFO: 0 processes.
INFO: Build completed successfully, 0 total actions
```

I'm not sure why `ar` is in the linker files, perhaps it can be removed.

```
➜  tests git:(zbarsky/toolchain) bazel cquery --output=files @llvm_toolchain_with_sysroot//:linker-files-aarch64-darwin
INFO: Found 1 target...
➜  tests git:(zbarsky/toolchain) ✗ bazel cquery --output=files @llvm_toolchain_with_sysroot//:linker-files-aarch64-darwin        
INFO: Invocation ID: d6b673ec-b86d-413d-808b-1e8eda884ad0
INFO: Analyzed target @@toolchains_llvm++llvm+llvm_toolchain_with_sysroot//:linker-files-aarch64-darwin (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
external/toolchains_llvm++llvm+llvm_toolchain_llvm/bin/clang
external/toolchains_llvm++llvm+llvm_toolchain_llvm/bin/clang++
external/toolchains_llvm++llvm+llvm_toolchain_llvm/bin/clang-cpp
external/toolchains_llvm++llvm+llvm_toolchain_llvm/bin/ld.lld
external/toolchains_llvm++llvm+llvm_toolchain_llvm/bin/ld64.lld
external/toolchains_llvm++llvm+llvm_toolchain_llvm/bin/wasm-ld
external/toolchains_llvm++llvm+llvm_toolchain_llvm/bin/llvm-ar
external/toolchains_llvm++llvm+llvm_toolchain_llvm/lib/clang/16/lib
external/toolchains_llvm++llvm+llvm_toolchain_llvm/lib/libc++.a
external/toolchains_llvm++llvm+llvm_toolchain_llvm/lib/libc++abi.a
external/toolchains_llvm++llvm+llvm_toolchain_llvm/lib/libc++experimental.a
external/toolchains_llvm++llvm+llvm_toolchain_llvm/lib/libunwind.a
external/toolchains_llvm++llvm+llvm_toolchain_with_sysroot/bin
INFO: Elapsed time: 0.108s, Critical Path: 0.00s
```

Author: dzbarsky

.github/CODEOWNERS

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

MODULE.bazel

@@ -19,7 +19,7 @@ module(
     compatibility_level = 0,
 )
 
-bazel_dep(name = "bazel_features", version = "1.36.0")
+bazel_dep(name = "bazel_features", version = "1.38.0")
 bazel_dep(name = "bazel_skylib", version = "1.5.0")
 bazel_dep(name = "aspect_bazel_lib", version = "2.0.0")
 bazel_dep(name = "rules_cc", version = "0.2.2")

tests/WORKSPACE

@@ -21,6 +21,17 @@ local_repository(
 
 load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
 
+http_archive(
+    name = "bazel_features",
+    sha256 = "07271d0f6b12633777b69020c4cb1eb67b1939c0cf84bb3944dc85cc250c0c01",
+    strip_prefix = "bazel_features-1.38.0",
+    url = "https://github.com/bazel-contrib/bazel_features/releases/download/v1.38.0/bazel_features-v1.38.0.tar.gz",
+)
+
+load("@bazel_features//:deps.bzl", "bazel_features_deps")
+
+bazel_features_deps()
+
 http_archive(
     name = "aspect_bazel_lib",
     sha256 = "6fd3b1e1a38ca744f9664be4627ced80895c7d2ee353891c172f1ab61309c933",
@@ -337,10 +348,6 @@ load("@rules_foreign_cc//foreign_cc:repositories.bzl", "rules_foreign_cc_depende
 
 rules_foreign_cc_dependencies()
 
-load("@bazel_features//:deps.bzl", "bazel_features_deps")
-
-bazel_features_deps()
-
 load("@rules_cc//cc:extensions.bzl", "compatibility_proxy_repo")
 
 compatibility_proxy_repo()

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

@@ -61,27 +61,49 @@ filegroup(
     ),
 )
 
+# This filegroup should only have source directories, not individual files.
+# We rely on this assumption in system_module_map.bzl.
+filegroup(
+    name = "cxx_builtin_include",
+    srcs = [
+        "include/c++",
+        "lib/clang/{LLVM_VERSION}/include",
+    ],
+)
+
+filegroup(
+    name = "extra_config_site",
+    srcs = glob(["include/*/c++/v1/__config_site"], allow_empty = True)
+)
+
 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(
+    name = "lib_legacy",
+    srcs = glob([
+        # 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/**",
+        "lib/**/libc++*.a",
+        "lib/**/libunwind.a",
+    ]),
 )
 
 filegroup(

toolchain/BUILD.toolchain.tpl

@@ -19,32 +19,20 @@ 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",
-    ],
+    name = "internal-use-tools",
+    srcs = ["%{tools_dir}"],
     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-legacy",
+    srcs = [%{symlinked_tools}
+        "%{tools_dir}/cc_wrapper.sh",
     ],
     visibility = ["//visibility:private"],
 )

toolchain/cc_toolchain_config.bzl

@@ -339,6 +339,7 @@ def cc_toolchain_config(
         # https://github.com/llvm/llvm-project/commit/0556138624edf48621dd49a463dbe12e7101f17d
         cxx_flags.append("-Xclang")
         cxx_flags.append("-fno-cxx-modules")
+        cxx_flags.append("-Wno-module-import-in-extern-c")
 
     opt_link_flags = ["-Wl,--gc-sections"] if target_os == "linux" else []
 

toolchain/internal/configure.bzl

@@ -12,6 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+load("@bazel_features//:features.bzl", "bazel_features")
 load("@bazel_skylib//lib:paths.bzl", "paths")
 load(
     "//toolchain:aliases.bzl",
@@ -76,11 +77,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) = _exec_os_arch_dict_value(rctx, "toolchain_roots")
+        _, 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) = _exec_os_arch_dict_value(rctx, "llvm_versions")
+    _, llvm_version = _exec_os_arch_dict_value(rctx, "llvm_versions")
     if is_requirement(llvm_version):
         llvm_version, distribution, error = required_llvm_release_name_rctx(rctx, llvm_version)
         if error:
@@ -231,7 +232,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 +336,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" if bazel_features.rules.merkle_cache_v2 else ":internal-use-tools-legacy")
 
     # C++ built-in include directories.
     # This contains both the includes shipped with the compiler as well as the sysroot (or host)
@@ -360,17 +361,24 @@ 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 + "lib/clang/{}/include".format(
+            major_llvm_version if major_llvm_version >= 16 else llvm_version,
+        ),
+        # Note(zbarsky): We could avoid this path if we renamed `include/{target_system_name}/c++/v1/__config_site` to `include/c++/v1/__config_site` in the LLVM repo.
+        # However, that would preclude sharing it across multiple toolchain definitions.
         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),
     ]
 
+    # TODO(zbarsky): Not sure if these lib64 paths are actually needed for system toolchains?
+    if use_absolute_paths_llvm:
+        cxx_builtin_include_directories.extend([
+            toolchain_path_prefix + "lib64/clang/{}/include".format(llvm_version),
+            toolchain_path_prefix + "lib64/clang/{}/include".format(major_llvm_version),
+        ])
+
     sysroot_prefix = ""
     if sysroot_path:
         sysroot_prefix = "%sysroot%"
@@ -505,17 +513,16 @@ 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_label}"],
 )
 
 filegroup(
     name = "compiler-components-{suffix}",
     srcs = [
         ":cxx_builtin_include_files-{suffix}",
         ":sysroot-components-{suffix}",
+        "{llvm_dist_label_prefix}extra_config_site",
+        "{llvm_dist_label_prefix}clang",
         {extra_compiler_files}
     ],
 )
@@ -526,7 +533,7 @@ filegroup(
         "{llvm_dist_label_prefix}clang",
         "{llvm_dist_label_prefix}ld",
         "{llvm_dist_label_prefix}ar",
-        "{llvm_dist_label_prefix}lib",
+        "{llvm_dist_label_prefix}{lib_label}",
         ":sysroot-components-{suffix}",
     ],
 )
@@ -629,6 +636,8 @@ cc_toolchain(
         extra_unfiltered_compile_flags = _list_to_string(_dict_value(toolchain_info.extra_unfiltered_compile_flags_dict, target_pair)),
         extra_files_str = extra_files_str,
         cxx_builtin_include_directories = _list_to_string(filtered_cxx_builtin_include_directories),
+        cxx_builtin_include_label = "cxx_builtin_include" if bazel_features.rules.merkle_cache_v2 else "include",
+        lib_label = "lib" if bazel_features.rules.merkle_cache_v2 else "lib_legacy",
         extra_compiler_files = ("\"%s\"," % str(toolchain_info.extra_compiler_files)) if toolchain_info.extra_compiler_files else "",
         major_llvm_version = major_llvm_version,
         extra_exec_compatible_with_specific = toolchain_info.extra_exec_compatible_with.get(target_pair, []),

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
 
+    _, 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

@@ -12,6 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+load("@bazel_features//:features.bzl", "bazel_features")
 load("@bazel_skylib//lib:paths.bzl", "paths")
 
 def _textual_header(file, *, include_prefixes, execroot_prefix):
@@ -55,20 +56,32 @@ 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()
+
+    if bazel_features.rules.merkle_cache_v2:
+        # If provided, cxx_builtin_files should be a filegroup with 2 source directory entries:
+        #  - include/c++
+        #  - lib/clang/<VERSION>/include
+        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,
         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