Work around https://github.com/bazelbuild/bazel/issues/3828 #130
Conversation
There was a problem hiding this comment.
Thanks for the patch!
Are you able to reproduce this issue just with
bazel build @llvm-project//llvm:NVPTXCodeGen ?
I can't on Linux Debian, although I have heard reports from people building on Windows. Note that I'm building with --config=generic_clang. At least some config is required, since that sets the C++ standard and such and I don't think there's anything in generic_clang that would somehow fix this issue.
|
I am able to reproduce on the Linux work desktop using that build command. Will apply patch manually and validate, after a the remaining meetings for today. Thank you very much for the patch. |
Huh. What config are you using? I don't get the issue with either |
|
This seems to make @GMNGeoffrey it is through TF, via Bazel. I can send more details via chat. |
Ok well I don't understand why that would make a difference, but that at least explains what's different about how you're building :-D @chandlerc, is adding these includes back in going to mess up the build on OSX? Can you test it out? |
|
Any updates? I cannot apply the patch via Bazel patching when fetching the repository :( |
|
@chandlerc waiting for you input here
Why not? |
|
Wait, the problem was on my side, I was patching the wrong project. Seems to be working now. |
|
So I think we no longer need this as I can patch TF. In fact, it seems we need to patch in several more places, depending on environment (Windows build fails on a different place) |
If this is indeed a fix for a Bazel bug, we should fix it here. I just want to check that it doesn't break other things in the process |
|
Bazel 4.0 that has been released in the past 24 hours no longer has the issue (it has the proper fix inside Bazel) |
|
Actually it seems Bazel 4.0 still has the same issue, after testing with the TF patch removed |
I don't see anything about this fix in the release notes. Maybe it's not included? |
|
@scentini can you comment on whether this is supposed to be in Bazel 4.0? |
|
No, the 4.0 baseline is a couple of days older than the fix, we'll need to wait for the next release. |
Current Bazel has a bug (bazelbuild/bazel#3828) when using `strip_include_prefix` that causes Bazel to erroneously report a missing dependency declaration. This bug shows up when trying to `bazel build @llvm-project//llvm:NVPTXCodeGen`: ``` undeclared inclusion(s) in rule '@llvm-project//llvm:NVPTXCodeGen': this rule is missing dependency declarations for the following files included by 'external/llvm-project/llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp': 'external/llvm-project/llvm/lib/Target/NVPTX/TargetInfo/NVPTXTargetInfo.h' ``` This has been fixed in bazelbuild/bazel@e3b7e17. This PR is a workaround until we have a Bazel version that contains the fix.
Current Bazel has a bug (bazelbuild/bazel#3828) when using
strip_include_prefixthat causes Bazel to erroneosly report a missing dependency declaration.This bug shows up when trying to
bazel build @llvm-project//llvm:NVPTXCodeGen:This has been fixed in bazelbuild/bazel@e3b7e17. This PR is a workaround until we have a Bazel version that contains the fix.