[PATCH] D158942: [bazel] Include builtin headers with clang-tidy

[Jathu Satkunarajah via Phabricator via llvm-commits]

jathu added a comment.

@rnk @rupprecht thanks for the review!

> I'm not sure the approach here makes sense, since it only fixes the case of using this tool directly from the blaze tree since now the directory will be placed in the right spot adjacent to clang-tidy. As soon as you copy the clang-tidy binary to install it somewhere, it will fail to find the headers if you don't also know that you have to copy that too.

It is true that if we copy just the binary outside of the bazel sandbox, the tool will not work. However, this is the case with installing clang-tidy from any package manager. For example, try moving your installed clang-tidy anywhere in your system — it will fail to run if the adjacent headers are not found. So, if someone does copy the binary out, I guess they will have to follow the standard of bundling the runtime headers too. Note that we are making the headers a runtime data file, so I'm assuming if you use Bazel to package the binary, it will include the headers?

> But, I'm also not aware of any kind of well lit path for this kind of problem. The build file tree defines lots of targets to build and produce a bunch of build artifacts, but there isn't really an equivalent to running install targets that place all those build artifacts together and in the right configuration (e.g. clang needs to go next to clang headers). That's usually done by external means.

I assumed the existence of a runtime data attribute in cc_binary meant this is an accepted pattern. I've configured clang-tidy to run using Bazel and the data files are successfully included in the runfiles when it runs



================
Comment at: utils/bazel/llvm-project-overlay/clang-tools-extra/BUILD.bazel:19
+    srcs = ["//clang:builtin_headers_gen"],
+    destination = "lib/clang/{0}/include".format(LLVM_VERSION_MAJOR),
+    partition = BUILTIN_HEADERS_GEN_DIR,
----------------
rupprecht wrote:
> Why does the symlink destination need the version in it?
Clang seems to look for it in this pattern `../lib/clang/{major version}/include`. You can see something similar in the docs here: https://clang.llvm.org/docs/LibTooling.html#builtin-includes.

Running clang-tidy with verbose logging also includes this error:

```
ignoring nonexistent directory "/private/var/tmp/_bazel_jathu/d7c345aded7a3e75ef6039df0c27203f/execroot/starfig/bazel-out/darwin_arm64-opt-exec-2B5CBBC6/bin/external/llvm-project/clang-tools-extra/lib/clang/17/include
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158942/new/

https://reviews.llvm.org/D158942