mirror of
https://gitee.com/mirrors_PX4/PX4-Autopilot.git
synced 2026-04-14 10:07:39 +08:00
The pr-review-poster was flagging `gtest/gtest.h file not found` on any PR that added or modified a test file, because clang-tidy-diff-18.py ran against files that weren't in the compilation database. PR #27004 and PR #26233 both hit this. The root cause is that test TUs only enter compile_commands.json when BUILD_TESTING is ON, which the historical clang-tidy build does not enable. This PR fixes both halves of the problem: 1. Add a second make target `px4_sitl_default-clang-test` that configures a separate build dir with -DCMAKE_TESTING=ON. Test TUs land in its compile_commands.json with resolved gtest/fuzztest include paths. 2. Add an umbrella `clang-ci` target that depends on both `px4_sitl_default-clang` and `px4_sitl_default-clang-test` so the PR job prepares both build dirs with one make invocation. 3. On PR events the workflow uses `make clang-ci`, installs libclang-rt-18-dev (needed so fuzztest's FUZZTEST_FUZZING_MODE flags do not fail the abseil try_compile with a misleading "pthreads not found" error), and routes the clang-tidy-diff producer at the test-enabled build dir. 4. Push-to-main is left entirely alone: same single build dir, same `make px4_sitl_default-clang`, same `make clang-tidy`. Test files are not in that DB so run-clang-tidy.py keeps ignoring them exactly as before. This preserves green main while ~189 pre-existing clang-tidy issues in test files remain untouched; fixing those is out of scope for this change. 5. Replace the fragile `:!*/test/*` pathspec filter (which missed flat `*Test.cpp` files in module roots) with `Tools/ci/clang-tidy-diff-filter.py`, which reads the compilation database and drops any changed source file that is not a TU. Headers always pass through. Production code that happens to use test-like names (src/systemcmds/actuator_test, src/drivers/test_ppm, etc.) stays analyzed because those are real px4_add_module targets. Verified in the ghcr.io/px4/px4-dev:v1.17.0-rc2 container and on the real CI runner: - cmake configure with CMAKE_TESTING=ON succeeds after installing libclang-rt-18-dev (Found Threads: TRUE) - compile_commands.json grows from 1333 to 1521 TUs - Modifying HysteresisTest.cpp with a new `const char *p = NULL` correctly flags hicpp-use-nullptr and clang-diagnostic-unused-variable on the new line, while pre-existing issues on other lines of the same file stay suppressed by clang-tidy-diff-18.py's line filter ("Suppressed ... 1 due to line filter") - No gtest/gtest.h false positives - Push-to-main path unchanged, still green Signed-off-by: Ramon Roche <mrpollo@gmail.com>
112 lines
3.5 KiB
Python
112 lines
3.5 KiB
Python
#!/usr/bin/env python3
|
|
"""
|
|
Filter a git diff for consumption by clang-tidy-diff.
|
|
|
|
Produces a unified diff containing only files that clang-tidy can
|
|
actually analyze against the current compilation database:
|
|
|
|
- C/C++ source files (.c, .cpp, .cc, .cxx, .m, .mm) must be present
|
|
in compile_commands.json. Files absent from the database are test
|
|
files, excluded code, or platform-specific sources that were not
|
|
compiled. Feeding them to clang-tidy-diff produces spurious
|
|
"header not found" errors (gtest/gtest.h in particular).
|
|
|
|
- Header files (.h, .hpp, .hxx) always pass through. clang-tidy
|
|
analyzes header changes via the TUs that include them; there is
|
|
no separate TU for a header to match against the database.
|
|
|
|
- All other files (CMakeLists.txt, .yml, .md, etc.) are dropped.
|
|
|
|
Output is a unified diff suitable for piping into clang-tidy-diff.py.
|
|
If nothing remains, the output file is empty.
|
|
|
|
Used by .github/workflows/clang-tidy.yml as a pre-filter for the
|
|
`pr-review` artifact producer. Python stdlib only.
|
|
"""
|
|
|
|
import argparse
|
|
import json
|
|
import os
|
|
import subprocess
|
|
import sys
|
|
|
|
|
|
SOURCE_EXTS = {'.c', '.cpp', '.cc', '.cxx', '.m', '.mm'}
|
|
HEADER_EXTS = {'.h', '.hpp', '.hxx'}
|
|
|
|
|
|
def load_db_files(build_dir):
|
|
"""Return the set of source paths (repo-relative) in compile_commands.json."""
|
|
path = os.path.join(build_dir, 'compile_commands.json')
|
|
with open(path) as f:
|
|
db = json.load(f)
|
|
root = os.path.abspath('.')
|
|
prefix = root + os.sep
|
|
paths = set()
|
|
for entry in db:
|
|
p = entry.get('file', '')
|
|
if p.startswith(prefix):
|
|
paths.add(p[len(prefix):])
|
|
else:
|
|
# Relative or external path; record as-is
|
|
paths.add(p)
|
|
return paths
|
|
|
|
|
|
def changed_files(base_ref):
|
|
out = subprocess.check_output(
|
|
['git', 'diff', '--name-only', '{}...HEAD'.format(base_ref)],
|
|
text=True,
|
|
)
|
|
return [line.strip() for line in out.splitlines() if line.strip()]
|
|
|
|
|
|
def keep_file(path, db_files):
|
|
"""Decide whether to keep this path in the filtered diff."""
|
|
ext = os.path.splitext(path)[1].lower()
|
|
if ext in HEADER_EXTS:
|
|
return True
|
|
if ext in SOURCE_EXTS:
|
|
return path in db_files
|
|
return False
|
|
|
|
|
|
def filtered_diff(base_ref, keep_paths):
|
|
if not keep_paths:
|
|
return ''
|
|
cmd = ['git', 'diff', '-U0', '{}...HEAD'.format(base_ref), '--'] + sorted(keep_paths)
|
|
return subprocess.check_output(cmd, text=True)
|
|
|
|
|
|
def main():
|
|
parser = argparse.ArgumentParser(description=__doc__)
|
|
parser.add_argument('--build-dir', required=True,
|
|
help='CMake build dir containing compile_commands.json')
|
|
parser.add_argument('--base-ref', required=True,
|
|
help='Git ref to diff against (e.g. origin/main)')
|
|
parser.add_argument('--out', required=True,
|
|
help='Output path for the filtered unified diff')
|
|
args = parser.parse_args()
|
|
|
|
db_files = load_db_files(args.build_dir)
|
|
changed = changed_files(args.base_ref)
|
|
|
|
keep = [p for p in changed if keep_file(p, db_files)]
|
|
dropped = [p for p in changed if p not in keep]
|
|
|
|
print('clang-tidy-diff-filter: kept {} of {} changed files'.format(
|
|
len(keep), len(changed)))
|
|
if dropped:
|
|
print(' dropped (not in compile_commands.json or not source/header):')
|
|
for p in dropped:
|
|
print(' {}'.format(p))
|
|
|
|
diff = filtered_diff(args.base_ref, keep)
|
|
with open(args.out, 'w') as f:
|
|
f.write(diff)
|
|
return 0
|
|
|
|
|
|
if __name__ == '__main__':
|
|
sys.exit(main())
|