-
Notifications
You must be signed in to change notification settings - Fork 168
Fix for header-only compile actions by Bazel #219
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
base: main
Are you sure you want to change the base?
Fix for header-only compile actions by Bazel #219
Conversation
|
Is ready to be committed? Can someone please review this? |
xFile3160
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
Is this ready to be merged? We are running into the same issue... |
|
This worked fine to produce a compilation database on a Abseil library. But I had to patch the |
|
@cpsauer could you take a look at this when you have a chance? It would be great to finally get this merged. Thanks in advance. |
|
Simply using @mikael-s-persson 's branch in your MODULE.bazel is a workaround until this gets merged into the main branch. |
|
When is this going to be merged? Keep facing the same problems and figuring out these work-arounds. |
|
Please consider merging this fix. Thanks! |
Please merge this. |
hhkblogi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Co-authored-by: Shyheme Days <shyhemedays@gmail.com>
|
FYI, people on this thread might be interested in checking out my new repo "bazel_cc_meta" which combines the functionality of this compile-commands-extractor and the depend-on-what-you-use tool into a much more efficient solution (benefits from bazel caching through an aspect-based implementation). It is still highly experimental, needless to say, but we've been using it for a few weeks in a relatively complicated cross-compilation setup. And it should be less brittle to issues like the one that spurred this PR because it can rely on Bazel's own logic to classify sources and headers rather than inferring them externally. |
I tried it and unfortunately it does not generate the proper compile db for the simplest abseil hello world example, at least for clangd based LSP in vim. I.e. firing up vim would immediately report plenty of undefined symbols. |
|
When the project relies on |
|
we good to merge? |
|
I apologise if this comes across as rude, but: This PR was raised more than a full year ago and has not yet been merged. The last non-bot commit to this repository was 1e08f8e in July 2024. I know the README says:
But it's been more than an entire year and this PR, which multiple people have confirmed works, has not been merged. Despite the above, this project is clearly not stable, myself and others (#271, #265, #258, #249, #242) run into this bug immediately on relatively basic projects. I'm aware of the internal issues (#232), but it has been a full year since that comment with no update. If this PR cannot be merged timely and this repository is indeed abandoned, perhaps it is time to fork. I would offer to do so myself but I'm only a downstream user of Bazel, I don't know enough about the tool itself to maintain a tool of this complexity. |
Fix for header-only compile actions by Bazel
This PR fixes, as far as I can tell, the issues related to #199 where Bazel adds header-only / syntax-only compilation actions that produce dummy output files (.processed). I believe this is part of Bazel open-sourcing more of its capabilities for preprocessing headers and handling pre-compiled modules.
The fix is based on #209 and makes the following changes:
This fix was tested in this repo: https://github.com/mikael-s-persson/evdevpp
And also tested and used for a while in other private repos.
This seems to produce clean, non-racy outputs. AFAICT, the resulting
compile_commands.jsonfile contains everything it should, including the headers.Fixes #199