[ci] Patch gRPC BUILD files to disable layering_check#61623
[ci] Patch gRPC BUILD files to disable layering_check#61623andrew-anyscale wants to merge 1 commit intomasterfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds a patch to disable the layering_check feature in gRPC's BUILD files. This is a workaround for a Bazel bug that appears with clang-14. The change is well-motivated and the explanation in the code is excellent. I have one suggestion to make the patch file itself more minimal and consistent.
| - features = [ | ||
| - "layering_check", | ||
| - ], |
There was a problem hiding this comment.
This part of the patch removes the entire features attribute from src/core/BUILD. For consistency with the patch for the root BUILD file (which only removes the layering_check entry), and to make the patch more minimal, consider changing this part of the patch to only remove the "layering_check" line. This would leave features = [] in the target file, which is valid and still achieves the goal. A more minimal and consistent patch is easier to maintain.
clang-14's Bazel toolchain enables the layering_check feature, which generates per-rule .cppmap module-map files and enforces that headers are only included through declared BUILD dependencies. gRPC v1.57.1 explicitly opts into this feature via package(features = ["layering_check"]) in two BUILD files (BUILD and src/core/BUILD), but has undeclared cross-rule transitive header inclusions that pass with clang-12 but fail with clang-14. With --spawn_strategy=local (used in test:ci), actions run unsandboxed and the compiler can access .cppmap files present in the output directory but not declared as inputs, producing "undeclared inclusion" errors across ~200 gRPC targets. Rather than disabling layering_check globally, patch the two gRPC BUILD files to remove the explicit feature opt-in. This keeps layering_check active for Ray's own C++ code. Topic: layering-check Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: andrew <andrew@anyscale.com>
a8a13fe to
aa6c579
Compare
|
I am also doing a similar thing at #61499 |
clang-14's Bazel toolchain enables the layering_check feature, which generates per-rule .cppmap module-map files and enforces that headers are only included through declared BUILD dependencies. gRPC v1.57.1 explicitly opts into this feature via package(features = ["layering_check"]) in two BUILD files (BUILD and src/core/BUILD), but has undeclared cross-rule transitive header inclusions that pass with clang-12 but fail with clang-14.
With --spawn_strategy=local (used in test:ci), actions run unsandboxed and the compiler can access .cppmap files present in the output directory but not declared as inputs, producing "undeclared inclusion" errors across ~200 gRPC targets.
Rather than disabling layering_check globally, patch the two gRPC BUILD files to remove the explicit feature opt-in. This keeps layering_check active for Ray's own C++ code.
Topic: layering-check
Co-Authored-By: Claude Sonnet 4.6 noreply@anthropic.com
Signed-off-by: andrew andrew@anyscale.com