-
Notifications
You must be signed in to change notification settings - Fork 4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Include checking fails on windows for generated C header files (undeclared inclusion(s)) #7030
Comments
Have you added |
I tried, and it doesn't help, but will it really help the include checking? The compilation is fine, the object file is built, but when the includes are to be validated, it fails. |
Please create a minimal repro. From face value the bug sounds like it might be a duplicate of #5640. @ozio85, does #5640 (comment) sound plausible in this case? |
An easy repro is to use buildfarm to build an extremely simple project, which passes on linux, but fails on windows, see this build farm issue (unclear if it is the same cause, but it triggers a similar behavior in a reproducible way): It might be one way to trigger this bug, but I am running in to it every now and then. |
@meteorcloudy does this bug look familiar to you? Look at this first: https://github.com/laszlocsomor/projects/tree/44c9f3a601986ee8f01768f80deff1ce2835e88f/bazel/gh-7030-buildfarm-includecheck-fails/README.md -- it shows the bug and a repro. |
@laszlocsomor Thanks for the repo, I am looking into this! Could be a similar bug to #6847 |
Great! Thank you! |
I debugged on this a bit, it indeed is a similar issue with #6847. The MSVC compiler always output the absolute path of the header files it detected, but of course a file under the remote worker's execution directory is not declared as dependency, so it failed at header check. a1aa5c1 provided a work around for this problem. When Bazel sees an absolute header file path, it first tries to find But the execroot used for actions in buildfarm worker doesn't contain To solve this problem, we have to append
should become
The idea is the buildfarm worker's execution directory should mimic the actual Bazel execution root. |
Not explicitly. It would be capable of observing output paths and making a reasonable guess, but it would be predicated on knowing that a client is [a particular version of] bazel due to semantics. There is no explicit naming for an execution above the action, except in opaque IDs in metadata, which likely does not contain anything specific to a workspace or it's name. |
please don't :-) |
I can confirm that #6931 fixes my include problem. However, i haven't been able to try it on the small build-farm example (seems like bazel and docker on windows does not work well together at the moment). |
@ozio85 : Cool, glad to hear that! Can we close this bug or is there anything else to do? |
No lets close it. I open i new if the problem remains in bazel build-farm. |
Description of the problem / feature request:
Include checking fails on windows for generated C/CPP header files. (the compilation is OK, but when Bazel checks the files afterwards, the header file is not recognized.)
Feature requests: what underlying problem are you trying to solve with this feature?
The include checking fails in CppCompileAction.java, in validateInclusions.
The line which is supposed to say the header file is ok is (around line 807):
The problem is that the artifact execroot differs on Windows and Linux for all generated files:
Edit: There seem to be a more intricate problem then just "all generated files". It might have something to do with circular dependencies? I am trying to get a minimal example rolling.
Linux:
Windows:
which causes the include check to fail.
A quick fix would be to add yet another check:
But it is not the clean way forward.
Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.
Edit: The problem is more intricate than this, i am trying to create a minimal example.
What operating system are you running Bazel on?
Windows
What's the output of
bazel info release
?release 0.21.0
Have you found anything relevant by searching the web?
Might be the same cause as to #3828 and #6337
Any other information, logs, or outputs that you want to share?
No
The text was updated successfully, but these errors were encountered: