Proto generated headers (*.pb.h) not included in compile_commands.json output

[eschumacher-s]

First, thanks for the tool. Extremely useful.

My issue is that my bazel build includes some .proto files, which at compile time generate .pb.h headers in the bazel cache.

These .pb.h files in compile_commands.json appear to have the wrong directory, which leads to clangd not finding them. For example, compile_commands.json lists directory as /path/to/bazel/repo/ instead of /path/to/.cache/path/to/header.pb.h.

Am I missing something in my configuration to support this scenario? Or is there a suggested workaround I can attempt to implement?

[cpsauer]

Hey, @eschumacher-s! Thanks for using the tool and writing in. I'm delighted to hear it's useful and honored to be the recipient of your account's first ever issue.

Let's get this fixed. Generated files (including these) are definitely a case we want to have work without any additional configuration. I know we use this case, that we've got code that tries to handle it, and that it was working last time I was in that part of the codebase, so I've got to figure out what's going on differently here.

Could I get you to send me the actual compile_commands.json entry for the proto generated header? And could you check to see if a file at that path exists--and if not, try to find the path where the file actually is? (Also if you're on Windows, please lmk.)

For reference, here's what I'd expect to be happening--and the behavior that (by default) I'll be trying to restore us to:

• I'd expect to see the directory as /path/to/bazel/repo/, like you're seeing.
• But I'd expect the generated file to be pointed to via a bazel symlink: bazel-out/<whatever>/header.pb.h
• I'd expect the generated file to exist at that path if you've run a build that needs it and it's in cache. If not, ideally we'd have printed a warning, but I don't think we can promise that always, because people can, e.g., turn header finding off. So hopefully it's also messaged prominently enough in the readme.
• And hopefully, that'd make things work fairly automagically. But it sounds like not, so let's figure out where things went wrong.

Cheers,
Chris

[eschumacher-s]

Chris,

Thanks for the quick reply. After digging in further, it appears each *.pb.h file is actually present twice in the generated compile_commands.json. One of the entries looks mostly correct, as you included in your response's details, in terms of the symlink'd bazel-out path.

There is a small problem with the includes within compile_commands.json, and I believe this is likely an error on how I've configured my refresh_compile_commands definition. I noticed that the includes within compile_commands.json often specify k8-opt (ex: bazel-out/k8-opt/bin/...), but my filesystem only has an arm related path and a host path. As a quick test, I did a sed to replace k8-opt with host in compile-commands.json, and it resolved the includes error.

So now I am revisiting my refresh_compile_commands and properly generating. I am cross compiling for an arm target, which may be part of the problem: I omitted three flags that I use while building, as the extractor tool complained that they specify targets: -c opt --cpu=armv7a --compiler=gcc.

Could the omission of these flags be related?

For context, my bazel build command:

bazel build --verbose_failures --symlink_prefix=/home/gbsbuild/mediapipe-t/ --experimental_cc_shared_library --crosstool_top=@crosstool//:toolchains -c opt --cpu=armv7a --compiler=gcc --copt=-DMEDIAPIPE_OMIT_EGL_WINDOW_BIT=1 --copt=-DT_VERSION=65 --define MEDIAPIPE_DISABLE_GPU=1 --compilation_mode=opt mediapipe/examples/desktop/$application_name:$application_name

[cpsauer]

Oh! Oh. yeah, important to end up with the same set of flags that you use to build.

[Why? Bazel will generate the files once per platform, so if you analyze for a different platform than you build for, it'll point to different files, which don't exist.]

[cpsauer]

So you can test quickly by adding a -- before the flags when you run the extractor.

[cpsauer]

But long term, better to add them to a refresh_compile_commands rule. The README has more details and examples on this than I should duplicate in this thread, but I'm here for questions if you need me.

[cpsauer]

Please do lmk if that fixes!

[eschumacher-s]

I included my build command in my previous comment (posting here as well):
bazel build --verbose_failures --symlink_prefix=/home/gbsbuild/mediapipe-t/ --experimental_cc_shared_library --crosstool_top=@crosstool//:toolchains -c opt --cpu=armv7a --compiler=gcc --copt=-DMEDIAPIPE_OMIT_EGL_WINDOW_BIT=1 --copt=-DT_VERSION=65 --define MEDIAPIPE_DISABLE_GPU=1 --compilation_mode=opt mediapipe/examples/desktop/$application_name:$application_name

My issue is that if I include -c opt --cpu=armv7a --compiler=gcc, I get an error message regarding targets and my compile_commands.json only contains []. Is there a way to properly specify these flags somehow? I use refresh_compile_commands to build for my target application, but even in there I get the same error message if these flags are included.

[cpsauer]

Yep! (As above.) See this section of the README https://github.com/hedronvision/bazel-compile-commands-extractor#2-are-there-flags-eg---configdebug---compilation_modedbg-that-you-apply-manually-apply-to-all-your-builds-while-developing

[eschumacher-s]

Maybe I am not explaining properly... I am following the suggestions in the linked section (2 and 3) of the README. Even with these suggestions, if I include -c opt --cpu=armv7a --compiler=gcc the extractor complains about targets and quits.

If I omit those flags only (-c opt --cpu=armv7a --compiler=gcc), but include the other flags, everything succeeds.

Maybe I am missing something. Do I have to somehow specify these specific flags as an additional target somehow?

Currently my BUILD is modified with the following rule:

load("@hedron_compile_commands//:refresh_compile_commands.bzl", "refresh_compile_commands")

refresh_compile_commands(
        name = "refresh_compile_commands",

        # Specify the targets of interest and any flags required to build
        targets = {
                        "mediapipe/examples/desktop/hybrid_app_example:hybrid_app_example": "--verbose_failures --symlink_prefix=/home/gbsbuild/mediapipe-t/ --experimental_cc_shared_library --crosstool_top=@crosstool//:toolchains --copt=-DMEDIAPIPE_OMIT_EGL_WINDOW_BIT=1 --copt=-DT_VERSION=65 --compilation_mode=opt",
        },
)

I want it to be as below, but get a failure when it is:

load("@hedron_compile_commands//:refresh_compile_commands.bzl", "refresh_compile_commands")

refresh_compile_commands(
        name = "refresh_compile_commands",

        # Specify the targets of interest and any flags required to build
        targets = {
                        "mediapipe/examples/desktop/hybrid_app_example:hybrid_app_example": "--verbose_failures --symlink_prefix=/home/gbsbuild/mediapipe-t/ --experimental_cc_shared_library --crosstool_top=@crosstool//:toolchains -c opt --cpu=armv7a --compiler=gcc --copt=-DMEDIAPIPE_OMIT_EGL_WINDOW_BIT=1 --copt=-DT_VERSION=65 --compilation_mode=opt",
        },
)

[cpsauer]

Oh! Sorry. Okay.

Just as an experiment, could I ask you to try adding build --cpu=armv7a --compiler=gcc to your .bazelrc and running with the command that works?

Could I also get that error message?

[This is definitely a case I've tried to make work, so I'm still trying to figure out where exactly things are going wrong.]

[cpsauer]

[We're probably going to need you to cut symlink-prefix out, but it should give you an error message about that, too. Sadly, that one is just not worth handling, since it breaks the (greatly) simplifying assumption that the build sandbox mirrors the workspace.)

[cpsauer]

One other thing that might be handy is trying to figure out which flag is breaking things by adding --cpu and --compiler individually?

(-c opt should just be a duplicate of --compilation_mode=opt, which you're already specifying.)

[cpsauer]

Aha! I think I've got a hypothesis for where in the code things are going wrong.

Is the warning you see "The flags you passed seem to contain targets..."
And does the full command work if you just remove "-c opt"?

[Again, sorry about the miscommunication.]

[eschumacher-s]

Sorry for the delay in response. First, to answer your questions:

1. The full warning is:

Analyzing commands used in mediapipe/examples/desktop/hybrid_app_example:hybrid_app_example
The flags you passed seem to contain targets.
Try adding them as targets in your refresh_compile_commands rather than flags.
[Specifying targets at runtime isn't supported yet, and in a moment, Bazel will likely fail to parse without our help. If you need to be able to specify targets at runtime, and can't easily just add them to your refresh_compile_commands, please open an issue or file a PR. You may also want to refer to https://github.com//issues/62.]

2. If I insert any of the 3 commands, whether individually or together, I receive the same warning. (i.e. adding -c opt gives the warning, adding -c opt --cpu=armv7a gives the warning, adding --cpu=armv7a gives the warning, etc through all iterations of the 3).

Secondly, a bit of good news I suppose! Looks as though I put too much faith in the warning message! I was initially receiving "[]" in compile_commands.json, so I assumed the message would lead to an eventual failure.

But I believe that was when I had tested with no refresh_compile_commands rule, and instead purely used the command line. Now that I retest with refresh_compile_commands and let it continue despite the warning, compile_commands.json looks correct!

Sorry for prematurely canceling and not treating the warning as only a warning. Thank you so much for the help.

[eschumacher-s]

Well, now I am getting issues with system includes that were previously working. I may need to bang my head against it for a while. But if you know of a quick fix, I'm all ears.

My compile_commands.json does contain a --sysroot flag pointing to my cross compiler root, within which ./usr/include/ contains the system headers that are failing. I'm experimenting with manually specifying include path and seeing if that resolves.

Anything ring a bell here?

[eschumacher-s]

FYI manually switching --sysroot=/path/to/sysroot with -isysroot /path/to/sysroot resolves the systems includes issues. Is there a way to configure this within my refresh_compile_commands rule? I can add a --copt flag to specify -isysroot, but this leads a stream of errors for toolchain headers, each apparently the same:

error: missing binary operator before token "("
44 | #if __GLIBC_PREREQ(2,15) && defined(_GNU_SOURCE)

[cpsauer]

Not a problem. The overeager warning is definitely my fault. Sorry. I'll start by fixing that.

[cpsauer]

Just pushed a commit attempting to make that warning only appear in problem cases.
(This was the hunch I had last night.)
Though I still don't know how, for example, --cpu=armv7a alone could cause the warning to be printed.

Could I ask you to check that the latest commit correctly silences the warning for you? If you do get any warnings or error messages, please do (always) send them. They're super helpful!

Similarly, if you ever recreate the blank compile_commands.json, please do let me know what led to it, including any warnings or errors.

[cpsauer]

Re isysroot/sysroot:

Just to confirm, there is not also another -isysroot in the arguments, right? (GCC docs say that sysroot should function for header search paths iff there's not an isysroot. source)

Assuming there's not another isysroot, then this is a bug coming up from clangd, unfortunately. Maybe clangd/clangd#1305. Does that look related to you? Would love it if you'd chime in there or file a new issue over there if you think it's sufficiently different.

Until they get it fixed, we should push a workaround. How about if we swap every "--sysroot" prefix for an "-isysroot"? This assumes you're also finding that -isysroot=/path/to/sysroot works, not just -isysroot /path/to/sysroot. I'll push a commit with that in a sec. I'm hoping I can ask you to test.

[eschumacher-s]

Confirmed that the warning no longer shows up with your latest commit. Awesome!

Regarding isysroot: No, no other isysroot. I did not use an =, so let me retry real quick using that instead of a space.

Also, amazingly helpful that you linked the exact clangd issue I'm hitting. Starting to regain a bit of my sanity knowing other clangd users are hitting this and its not just my setup.

[cpsauer]

Yay. Thanks for checking in on all of these.

New commit is also out for you to test, doing the substitution automatically.

[cpsauer]

And sorry for the tax on your sanity. Honestly, we're hitting (and working around) enough clangd bugs that I'm kinda tempted to recommend people migrate to ccls. But really, I wish we'd had things just work out of the box for you.

[cpsauer]

Also, note that that clangd issue may not be quite exactly what you're seeing. There, it looks like clangd is automatically changing the sysroot to isysroot internally and it's still not working.

Would love it if you'd chime or file a new issue over there if you think it's sufficiently different from your case or if I missed anything.

And again, sorry this has been such an adventure.

[eschumacher-s]

No need to apologize, you've been more than helpful and your tool has already been a huge help. Yes, I will post at clangd as well.

I confirmed that a manual swap of --sysroot= to -isysroot= works properly. I'm actually a bit shocked at how well everything comes together after that change.

One more run in progress with the newest commit - will post back momentarily.

[cpsauer]

Trying! Thanks for being great yourself :)

[cpsauer]

If that works for you, could you close this down?
But if other issues crop up, please do let me know. Don't settle!

[eschumacher-s]

Confirming issue as resolved & closed. Thanks again for the quick support.

[cpsauer]

Wahoo! My pleasure. Good luck! Curious what you're using media pipe for!

[cpsauer]

@eschumacher-s, we were revisiting this in #179. Do you know if clangd ended up fixing the underlying issue here? If you're still using this tool and bazel and gcc, would you be down to patch the --isysroot's back to --sysroots and see if things work?

[eschumacher-s]

I modified isysroot back to sysroot and things look to be working properly still - so maybe this was fixed on clangd end?

[cpsauer]

Thanks so much for checking :) I'll delete the workaround then. Please let me know if the issue comes back!

[kon72]

Hi, I don't know if it's related, but I just want to let you know that I recently pushed the change to clangd regarding sysroot flag parsing.
llvm/llvm-project#75694

[cpsauer]

Sounds pretty related to me :) thanks for the fix--and for chiming in!