[bazel] Add llvm_zlib to system include path #121374
Conversation
LLVM codebase expects to find zlib using (angle brackets) `<zlib.h>`. This means llvm_zlib needs to be exposed on the system include path (`-isystem` not `-iquote`). Tell bazel to do that.
|
Reviewers: I don't have merge permissions - so please mash the merge button yourself when you feel appropriate. |
| includes = select({ | ||
| ":llvm_zlib_enabled": ["."], | ||
| "//conditions:default": [], | ||
| }), | ||
| # Clang includes zlib with angled instead of quoted includes, so we need |
There was a problem hiding this comment.
This comment suggests angled bracket includes should already be working, so I'm not sure why an additional change is needed. Looks like @aaronmondal set this up a long time ago; do you know what's going on here?
There was a problem hiding this comment.
Hmm IIIRC entire target should only be instantiated if "llvm_zlib_enabled" is set. Maybe some downstream target is missing an explicit dep on zlib? The strip_include_prefix should already be doing the include path shifting for the angled include. But this code is old and Bazel changed a bunch of internals around rule_cc so that might also be related here.
For reference, this was originally upstreamed from here: https://github.com/eomii/bazel-eomii-registry/blob/main/modules/llvm-project-overlay/17-init-bcr.3/patches/llvm_use_zlib-ng.patch
I have a newer version here which I originally wrote against llvm which works without custom includes or the strip prefix here: https://github.com/bazelbuild/bazel-central-registry/pull/1539/files. BCR CI is too slow moving to fix this for windows, but I can probably bump it in llvm and fix the windows support here.
LLVM codebase expects to find zlib using (angle brackets)
<zlib.h>.This means llvm_zlib needs to be exposed on the system include path
(
-isystemnot-iquote). Tell bazel to do that.