[bazel] Add filegroup for builtin_headers

[dzbarsky]

I'd like to package these files into a distribution tar as part of https://github.com/dzbarsky/static-clang. I'm currently applying patches to the llvm repo but figured this bit could be upstreamed.

(Also open to ideas what to do about the config.bzl change in https://github.com/dzbarsky/static-clang/blob/master/llvm.patch - it's needed to link with musl libc)

[dzbarsky]

@jathu I saw you were touching the same area

[jathu]

Why can't you directly use the outs of the genrule?

[dzbarsky]

I could. It would require the following change on my side:

 pkg_files(
     name = "builtin_headers_pkg_files",
     srcs = [
-        "@llvm-project//clang:builtin_headers_files",
+        "@llvm-project//clang:builtin_headers_gen",
     ],
-    strip_prefix = "lib/Headers",
+    strip_prefix = strip_prefix.from_pkg() + "staging/include/",
     prefix = "lib/clang/%s/include" % LLVM_VERSION_MAJOR,

This seems more brittle, it will break with your PR. Also that genrule takes 17 seconds in my setup to copy the files around, which is a waste since I'm just going to package them up anyway. I'd probably stick with my local patch in that case.

Any reason not to add this?

[dzbarsky]

Oh I guess I misread and it won't change with your PR, I would probably need to update to use the const though. Either way, I think the point is the filegroup is simpler and faster.

[jathu]

Do you think you will still need this once we land #67626? The clang-tidy binary will have the header files as runfiles, which you can just package directly

Also that genrule takes 17 seconds in my setup to copy the files around, which is a waste since I'm just going to package them up anyway. I'd probably stick with my local patch in that case. Any reason not to add this?

No, I don't see any reason not to add this. I assumed the filegroup was redundant since the genrule would cache the subsequent uses anyways

[dzbarsky]

Do you think you will still need this once we land #67626? The clang-tidy binary will have the header files as runfiles, which you can just package directly

I'm building the entire tar package within Bazel and I'm not packaging clang-tidy (you don't need it for a minimal toolchain) so I don't think it helps.

Also that genrule takes 17 seconds in my setup to copy the files around, which is a waste since I'm just going to package them up anyway. I'd probably stick with my local patch in that case. Any reason not to add this?

No, I don't see any reason not to add this. I assumed the filegroup was redundant since the genrule would cache the subsequent uses anyways

I think something about my compilation setup is non-hermetic, because when executing from a github actions runner (with RBE/remote caching) I am seeing tons of remote misses. When running from a minimal docker image on my laptop (also against RBE) I am seeing everything cached, so I suspect we are somehow leaking the repo path, which is probably randomized on github. Anyway, this is a total aside, but the caching doesn't work as well as I'd like :)