Properly avoiding external sources (clang-tidy)

[fzakaria]

First off -- thank you for the project.

I am trying to integrate it with my Bazel project to get clang-tidy working.

With no changes to the setup I have noticed that clang-tidy operates on all sources even including third-party sources. Obviously I don't control that -- so I see that there is a exclude_external_sources = True, option.

When I run that -- I still seem to get some errors which are surprising. Is the compile_commands.json database missing something?

clang-tidy-14 --use-color -p=/usr/local/home/fmzakari/code/github.com/fzakaria/elf2sql /usr/local/home/fmzakari/code/github.com/fzakaria/elf2sql/src/cli/main.cc
/usr/local/home/fmzakari/code/github.com/fzakaria/elf2sql/bazel-out/k8-fastbuild/bin/external/com_github_google_glog/_virtual_includes/glog/glog/logging.h:66:10: error: 'glog/platform.h' file not found [clang-diagnostic-error]
#include <glog/platform.h>
         ^~~~~~~~~~~~~~~~~
1 error generated.
Error while processing /usr/local/home/fmzakari/code/github.com/fzakaria/elf2sql/src/cli/main.cc.
Found compiler error(s).

platform.h definitely exists in the glog codebase https://github.com/google/glog/blob/888f93947b60045f674f2c67a2c7e18505079050/src/glog/platform.h

relevant compile commands:

 {
    "file": "src/cli/main.cc",
    "arguments": [
      "external/buildbuddy_toolchain/bin/clang",
      "-xc++",
      "-U_FORTIFY_SOURCE",
      "-fstack-protector",
      "-fno-omit-frame-pointer",
      "-fcolor-diagnostics",
      "-Wall",
      "-Wthread-safety",
      "-Wself-assign",
      "-std=c++17",
      "-stdlib=libc++",
      "-MD",
      "-MF",
      "bazel-out/k8-fastbuild/bin/src/cli/_objs/cli/main.pic.d",
      "-fPIC",
      "-DGLOG_DEPRECATED=__attribute__((deprecated))",
      "-DGLOG_EXPORT=__attribute__((visibility(\"default\")))",
      "-DBAZEL_CURRENT_REPOSITORY=\"\"",
      "-iquote",
      ".",
      "-iquote",
      "bazel-out/k8-fastbuild/bin",
      "-iquote",
      "external/com_github_gflags_gflags",
      "-iquote",
      "bazel-out/k8-fastbuild/bin/external/com_github_gflags_gflags",
      "-iquote",
      "external/com_github_google_glog",
      "-iquote",
      "bazel-out/k8-fastbuild/bin/external/com_github_google_glog",
      "-iquote",
      "external/bazel_tools",
      "-iquote",
      "bazel-out/k8-fastbuild/bin/external/bazel_tools",
      "-Ibazel-out/k8-fastbuild/bin/external/com_github_gflags_gflags/_virtual_includes/gflags",
      "-Ibazel-out/k8-fastbuild/bin/external/com_github_google_glog/_virtual_includes/glog",
      "-isystem",
      "bazel-out/k8-fastbuild/bin/third_party/lief/include",
      "-no-canonical-prefixes",
      "-Wno-builtin-macro-redefined",
      "-D__DATE__=\"redacted\"",
      "-D__TIMESTAMP__=\"redacted\"",
      "-D__TIME__=\"redacted\"",
      "-fdebug-prefix-map=external/buildbuddy_toolchain/=external/buildbuddy_toolchain/",
      "-std=c++14",
      "-c",
      "src/cli/main.cc",
      "-o",
      "bazel-out/k8-fastbuild/bin/src/cli/_objs/cli/main.pic.o"
    ],
    "directory": "/usr/local/google/home/fmzakari/code/github.com/fzakaria/elf2sql"
  },

My guess is that it's because its using iquote and these 3rd party are using system library format "< >" (angle bracket).

[cpsauer]

Hey, Farid! Great to meet you. You're very welcome. Sorry to be a little slow--just catching up after getting back from a trip.

I'm pretty sure I know what the issue is. I'm going to guess the error goes away if you run a build (with the same flags), yeah?

If so, what's going on is that glog is included via that -Ibazel-out/k8-fastbuild/bin/external/com_github_google_glog/_virtual_includes/glog, from the strip_include_prefix here, but the _virtual_incude symlinks aren't created until build time. Sad times. I'll toss up a quick PR to glog to move them over, since that strip_include_prefix could just be an includes and seems to also be causing them some pain on Windows. Edit: They're actually using it to enforce header privacy w/o layering_check, and have headers that collide with system ones, so I've closed that PR.

This general issue been causing a lot of trouble for folks lately. #102 is probably the best read if you're curious but #123 is the same. I think we warn people to run a build or switch to includes if we detect a missing _virtual_include link? (I assume we didn't currently print out any warnings that would have guided you on the right path.) If you're feeling especially crafty, I'd love it if you'd be willing to work on some code as a PR! (Thinking more, we'll probably want to cache the existence check--for speed--but that's super easy.)

Cheers,
Chris

[cpsauer]

I put up a PR that should fix this the missing-virtual-includes-without-rebuild issue on bazel's-side over at bazelbuild/bazel#20540. Any amount you'd like to chime in over there to encourage merging would be much appreciated :)

#140 and #147 and #133 and #123 are the same underlying issue, I think.

Note that I'd expect that clang-tidy will still try to include external headers imported by your files, including transitively. Let me know if that's unexpected.

Since it's been quite a while since I last heard from you, I'll probably close this when that PR lands unless you say otherwise. If you come back to it after that, just give a holler, and we can open it back up.

[fzakaria]

Thanks for following up and the fix! I'm traveling at the moment so please close this issue once it lands 👍

…

On Thu, Dec 14, 2023, 1:54 AM Chris Sauer ***@***.***> wrote: I put up a PR that should fix this the missing-virtual-includes-without-rebuild issue on bazel's-side over at bazelbuild/bazel#20540 <bazelbuild/bazel#20540>. Any amount you'd like to chime in over there to encourage merging would be much appreciated :) #140 <#140> and #147 <#147> and #133 <#133> and #123 <#123> are the same underlying issue, I think. Note that I'd expect that clang-tidy will still try to include external headers imported by your files, including transitively. Let me know if that's unexpected. Since it's been quite a while since I last heard from you, I'll probably close this when that PR lands unless you say otherwise. If you come back to it after that, just give a holler, and we can open it back up. — Reply to this email directly, view it on GitHub <#121 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAETXDQAKATHTOM62BLNW7DYJLEHBAVCNFSM6AAAAAAYG4ZJOKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNJVGUZDSNJZGU> . You are receiving this because you authored the thread.Message ID: <hedronvision/bazel-compile-commands-extractor/issues/121/1855529595@ github.com>

[cpsauer]

My pleasure. For my learning (and for issue closing timing), would you switch to bazel rolling for the fix or would you need it to be in a versioned release?

[fzakaria]

I haven't been doing C/C++ development in a bit for this project; I don't think I can provide details on the fix :(

[cpsauer]

The fix has made it into the bazel mainline!

Thanks for helping track this down and get it in--should be released as part of Bazel 7.1 and the next rolling release!