Provide a way to create a compilation DB in the bazel project #9122
Conversation
|
clang-tidy review says "All clean, LGTM! 👍" |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces useful scripts for generating a compilation database and running clang-tidy, which is a great improvement for developer workflow and code quality. The scripts are well-structured. My review includes several suggestions to enhance their robustness and maintainability by following shell scripting best practices. These include adding error handling (set -e), quoting variables and command substitutions to prevent word splitting issues, and using more robust methods like bazel cquery to locate build artifacts instead of relying on fragile path globbing. I've also pointed out a potential bug in a loop that could occur if a glob pattern doesn't match any files.
hzeller
force-pushed
the
feature-20251222-add-compdb
branch
from
ead2044 to
930e60b
Compare
|
clang-tidy review says "All clean, LGTM! 👍" |
hzeller
force-pushed
the
feature-20251222-add-compdb
branch
from
930e60b to
5eb9f4a
Compare
|
clang-tidy review says "All clean, LGTM! 👍" |
hzeller
force-pushed
the
feature-20251222-add-compdb
branch
from
5eb9f4a to
3a228ad
Compare
|
clang-tidy review says "All clean, LGTM! 👍" |
This uses http://bant.build/ to create a compilation DB. The change provides two useful scripts: * `etc/bazel-make-compilation-db.sh` creates a compilation db. To avoid a huge compile_commands.json, this generates the compile_flags.txt which is a much shorter file applied to all files. This keeps memory usage low for tools such as clang-tidy * `etc/run-clang-tidy.sh` a convenience script that creates the compilation DB and then calls `run-clang-tidy-cached.cc`. It uses `clang-tidy` from the llvm-toolchain configured in the MODULE.bazel Issues The-OpenROAD-Project#8586 Signed-off-by: Henner Zeller <[email protected]>
hzeller
force-pushed
the
feature-20251222-add-compdb
branch
from
3a228ad to
1a2d93e
Compare
|
clang-tidy review says "All clean, LGTM! 👍" |
|
(forgot to make it a separate commit, so it is |
etc/bazel-make-compilation-db.sh
Outdated
| # Important to run with --remote_download_outputs=all to make sure generated | ||
| # files are actually visible locally in case a remote cache (that includes | ||
| # --disk_cache) is used ( https://github.com/hzeller/bazel-gen-file-issue ) | ||
| BAZEL_OPTS="-c opt --remote_download_outputs=all" |
There was a problem hiding this comment.
It doesn't seem to hit the cache. I see the CI builds with
bazelisk test --config=ci --show_timestamps --test_output=errors --curses=no --force_pic --profile=build.profile --remote_header=Authorization=Basic **** ...
which I'm guessing isn't a match
There was a problem hiding this comment.
Since I can not run the ci configuration as a regular user (no permissions), I've now kept that default, but made it overridable with an environment variable, so that the CI or personal set-ups can set that to desired values.
|
compile_flags.txt seems to try to be a universal set of includes rather than the specific includes used by a given translation unit. I don't see how that will work in general. For example: build/compile_commands.json is 3.4M so larger but not terrible. cmake is much faster at this job as it doesn't require a build. I think you only need to generate the swig wrappers and tcl encoding files. |
Signed-off-by: Henner Zeller <[email protected]>
|
clang-tidy review says "All clean, LGTM! 👍" |
1 similar comment
|
clang-tidy review says "All clean, LGTM! 👍" |
Yeah, that is a general problem of the project layout which I have said many times before. Once we can abandon CMake, we should fix the paths to always use paths relative to project root.
For bazel, it would be several hundred megabytes, as it needs to encode all the paths. With that, running clang-tidy on 128 cores will require several hundred gigabytes of RAM. So unfortunately, a compile_commands.json is not an option right now.
We do that now also but build the relevant generated files. Ideally,
Yep, changed that now, so only the fetch of the dependencies is done plus the handful of generated things (much faster) |
Signed-off-by: Henner Zeller <[email protected]>
hzeller
force-pushed
the
feature-20251222-add-compdb
branch
from
080d3a9 to
2ad0129
Compare
|
clang-tidy review says "All clean, LGTM! 👍" |
2 participants
This uses http://bant.build/ to create a compilation DB.
The change provides two useful scripts:
etc/bazel-make-compilation-db.shcreates a compilation db. To avoid a huge compile_commands.json, this generates the compile_flags.txt which is a much shorter file applied to all files. This keeps memory usage low for tools such as clang-tidyetc/run-clang-tidy.sha convenience script that creates the compilation DB and then callsrun-clang-tidy-cached.cc. It usesclang-tidyfrom the llvm-toolchain configured in the MODULE.bazelIssues #8586