PX4 CI: Incremental Clang-Tidy on Changed Files — Implementation Plan

groovy-hopping-pie.md

Plan: Incremental Clang-Tidy + Inline PR Annotations

Context

Clang-Tidy is the T2 critical path bottleneck at 11:16 (9:09 is the analysis step, ~1,025 source files, 16 threads). It gates all T3 jobs. A typical PX4 PR touches 1–7 .cpp/.hpp files.

platisd/clang-tidy-pr-comments was evaluated. It consumes a clang-tidy YAML fixes file and posts inline, line-level review comments on the PR diff — including one-click "suggested change" blocks when clang-tidy has a fix. It does not run clang-tidy itself.

This opens a two-layer architecture:

1. Gate layer (existing): file-level incremental tidy → CI pass/fail. Catches all issues in changed files.
2. Annotation layer (new): line-level diff-only tidy → YAML → platisd posts inline PR comments showing exactly where and what to fix.

These are complementary: the gate ensures correctness, the annotations improve developer UX dramatically.

---

Approach

Layer 1 — Incremental Gate (file-level)

Replace make -j16 clang-tidy on PRs with an inline workflow step:

1. Compute changed C++ files: git diff --name-only origin/$BASE_REF...HEAD -- '*.cpp' '*.hpp' '*.h' '*.c'
2. For each changed .hpp/.h: expand to all .cpp TUs that include it (grep source tree, cross-ref compile_commands.json)
3. If no C++ files changed → skip entirely (exit 0) — handles CI-only PRs like this branch
4. Pass the TU list as positional args to run-clang-tidy.py — its files regex filter already supports this
5. On push to main/stable/beta/release/**: keep full make -j16 clang-tidy as safety net

No new Makefile target or helper script needed — the workflow has all the context (github.base_ref, workspace path, exclude list) directly available.

Layer 2 — Inline PR Annotations (clang-tidy-diff + platisd)

After the gate step (on PRs, same-repo only — same if: guard as post-flash-comment at line 727):

1. Install clang-tools package (adds clang-tidy-diff-18.py — not in ubuntu.sh, confirmed via Tools/setup/ubuntu.sh:137-139)
2. Run clang-tidy-diff: git diff -U0 origin/$BASE_REF...HEAD | clang-tidy-diff-18.py -p1 -path build/px4_sitl_default-clang -export-fixes fixes.yml
3. Use platisd/clang-tidy-pr-comments@v1 to post inline comments from fixes.yml
4. Set request_changes: false — the gate (Layer 1) already blocks merges; annotations are informational

Key detail: clang-tidy-diff.py runs tidy on changed files, reports only issues on changed lines. The gate (Layer 1) is stricter (reports all issues in changed files). Both share the same ccache-warm build, so Layer 2 analysis is fast.

Fork PRs: Layer 2 is skipped (same if: guard as existing post-flash-comment job). Gate still runs on forks.

---

Critical Files

File	Change
.github/workflows/ci-orchestrator.yml	1) Replace analysis step with calls to new scripts. 2) Add post-clang-tidy-comments job
Tools/ci/run-clang-tidy-pr.py	New: computes changed TUs + runs run-clang-tidy.py; fully self-contained
Tools/setup/ubuntu.sh	No change — clang-tools installed in workflow step to keep container lean

Makefile — no changes. Tools/run-clang-tidy.py — no changes.

Convention: no $GITHUB_OUTPUT needed — the script runs tidy directly and exits with tidy's return code. The workflow step fails if the script exits non-zero.

---

Workflow Changes

Clang-Tidy Job (~line 330–344)

Replace the single analysis step with one step:

- name: Analysis - Run Clang-Tidy
  run: |
    if [ "${{ github.event_name }}" != "pull_request" ]; then
      make -j$(nproc) clang-tidy
    else
      python3 Tools/ci/run-clang-tidy-pr.py origin/${{ github.base_ref }}
    fi

Script: Tools/ci/run-clang-tidy-pr.py

Single Python script replacing both bash scripts. Called directly from the workflow.

#!/usr/bin/env python3
"""
Run clang-tidy incrementally on files changed in a PR.

Usage: run-clang-tidy-pr.py <base-ref>
  base-ref: e.g. origin/main

Computes the set of translation units (TUs) affected by the PR diff,
then invokes Tools/run-clang-tidy.py on that subset only.
Exits 0 silently when no C++ files were changed.
"""

import argparse
import json
import os
import subprocess
import sys

EXTENSIONS_CPP = {'.cpp', '.c'}
EXTENSIONS_HDR = {'.hpp', '.h'}
# Manual exclusions from Makefile:508
EXCLUDE_EXTRA = '|'.join([
    'src/systemcmds/tests',
    'src/examples',
    'src/modules/gyro_fft/CMSIS_5',
    'src/lib/drivers/smbus',
    'src/drivers/gpio',
    r'src/modules/commander/failsafe/emscripten',
    r'failsafe_test\.dir',
])


def repo_root():
    try:
        return subprocess.check_output(
            ['git', 'rev-parse', '--show-toplevel'], text=True).strip()
    except subprocess.CalledProcessError:
        print('error: not inside a git repository', file=sys.stderr)
        sys.exit(1)


def changed_files(base_ref, root):
    try:
        out = subprocess.check_output(
            ['git', 'diff', '--name-only', f'{base_ref}...HEAD',
             '--', '*.cpp', '*.hpp', '*.h', '*.c'],
            text=True, cwd=root).strip()
        return out.splitlines() if out else []
    except subprocess.CalledProcessError:
        print(f'error: could not diff against "{base_ref}" — '
              'is the ref valid and fetched?', file=sys.stderr)
        sys.exit(1)


def submodule_paths(root):
    # Returns [] if .gitmodules is absent or has no paths — both valid
    try:
        out = subprocess.check_output(
            ['git', 'config', '--file', '.gitmodules',
             '--get-regexp', 'path'],
            text=True, cwd=root).strip()
        return [line.split()[1] for line in out.splitlines()]
    except subprocess.CalledProcessError:
        return []


def build_exclude(root):
    submodules = '|'.join(submodule_paths(root))
    return f'{submodules}|{EXCLUDE_EXTRA}' if submodules else EXCLUDE_EXTRA


def load_db(build_dir):
    db_path = os.path.join(build_dir, 'compile_commands.json')
    if not os.path.isfile(db_path):
        print(f'error: {db_path} not found', file=sys.stderr)
        print('Run "make px4_sitl_default-clang" first to generate '
              'the compilation database', file=sys.stderr)
        sys.exit(1)
    try:
        with open(db_path) as f:
            return json.load(f)
    except json.JSONDecodeError as e:
        print(f'error: compile_commands.json is malformed: {e}', file=sys.stderr)
        sys.exit(1)


def find_tus(changed, db, root):
    db_files = {e['file'] for e in db}
    result = set()
    for f in changed:
        abs_path = os.path.join(root, f)
        ext = os.path.splitext(f)[1]
        if ext in EXTENSIONS_CPP:
            if abs_path in db_files:
                result.add(abs_path)
        elif ext in EXTENSIONS_HDR:
            hdr = os.path.basename(f)
            for e in db:
                try:
                    if hdr in open(e['file']).read():
                        result.add(e['file'])
                except OSError:
                    pass  # file deleted in PR — skip
    return sorted(result)


def main():
    parser = argparse.ArgumentParser(description=__doc__,
                                     formatter_class=argparse.RawDescriptionHelpFormatter)
    parser.add_argument('base_ref',
                        help='Git ref to diff against, e.g. origin/main')
    args = parser.parse_args()

    root = repo_root()
    build_dir = os.path.join(root, 'build', 'px4_sitl_default-clang')

    run_tidy = os.path.join(root, 'Tools', 'run-clang-tidy.py')
    if not os.path.isfile(run_tidy):
        print(f'error: {run_tidy} not found', file=sys.stderr)
        sys.exit(1)

    changed = changed_files(args.base_ref, root)
    if not changed:
        print('No C++ files changed — skipping clang-tidy')
        sys.exit(0)

    db = load_db(build_dir)
    tus = find_tus(changed, db, root)

    if not tus:
        print('No matching TUs in compile_commands.json — skipping clang-tidy')
        sys.exit(0)

    print(f'Running clang-tidy on {len(tus)} translation unit(s)')

    result = subprocess.run(
        [sys.executable, run_tidy,
         '-header-filter=.*\\.hpp',
         '-j0',
         f'-exclude={build_exclude(root)}',
         '-p', build_dir] + tus
    )
    sys.exit(result.returncode)


if __name__ == '__main__':
    main()

Failure modes handled:

Failure	Behaviour
Not in a git repo	error: not inside a git repository → exit 1
Invalid/unfetched base_ref	error: could not diff against "..." → exit 1
.gitmodules absent or empty	Returns [] submodules — silent, valid
compile_commands.json missing	Clear message + hint to run build → exit 1
compile_commands.json malformed	error: compile_commands.json is malformed → exit 1
Source file in DB deleted by PR	except OSError: pass — skipped silently
run-clang-tidy.py missing	error: .../run-clang-tidy.py not found → exit 1
No C++ files changed	"No C++ files changed — skipping" → exit 0
No TUs match DB	"No matching TUs — skipping" → exit 0

New Job: post-clang-tidy-comments

Add after the clang-tidy job (similar structure to post-flash-comment at line 716):

post-clang-tidy-comments:
  name: "T2: Clang-Tidy PR Annotations"
  needs: [clang-tidy]
  runs-on: [runs-on,runner=2cpu-linux-x64,image=ubuntu24-full-x64,"run-id=${{ github.run_id }}"]
  permissions:
    pull-requests: write
    contents: read
  if: >-
    github.event.pull_request
    && github.event.pull_request.head.repo.full_name == github.repository
    && needs.clang-tidy.result == 'success'
  container:
    image: ghcr.io/px4/px4-dev:v1.17.0-beta1
  steps:
    - uses: actions/checkout@v4
      with:
        fetch-depth: 0
    - name: Setup - Configure Git Safe Directory
      run: git config --system --add safe.directory '*'
    - name: Setup - Install clang-tools (for clang-tidy-diff)
      run: apt-get install -y clang-tools
    - name: Cache - Restore ccache (reuse clang-tidy build)
      uses: actions/cache/restore@v4
      with:
        path: ~/.ccache
        key: ccache-clang-tidy-${{ github.ref_name }}-${{ github.sha }}
        restore-keys: |
          ccache-clang-tidy-${{ github.ref_name }}-
          ccache-clang-tidy-${{ github.base_ref || 'main' }}-
          ccache-clang-tidy-
    - name: Build - px4_sitl_default (Clang) [ccache warm, ~0:16]
      run: make -j16 px4_sitl_default-clang
    - name: Analysis - Run clang-tidy-diff and export fixes
      run: |
        mkdir -p clang-tidy-result
        git diff -U0 origin/${{ github.base_ref }}...HEAD \
          | clang-tidy-diff-18.py -p1 \
              -path build/px4_sitl_default-clang \
              -export-fixes clang-tidy-result/fixes.yml \
              -j0
    - name: Post - Annotate PR with clang-tidy findings
      uses: platisd/clang-tidy-pr-comments@v1
      with:
        github_token: ${{ secrets.GITHUB_TOKEN }}
        clang_tidy_fixes: clang-tidy-result/fixes.yml
        request_changes: false
        suggestions_per_comment: 10

Note: The build step reuses the ccache from the gate job (same key). With a warm hit it takes ~0:16 (confirmed from step timings). The diff analysis is fast since it only processes changed files.

---

Verification

1. CI-only PR (no C++ changes): gate prints "No C++ files changed — skipping clang-tidy", exits in <5s. No annotation job runs.
2. PR touching 1 .cpp: gate runs tidy on that file only (~30s–2min). Annotation job posts inline comments if issues found.
3. PR touching a .hpp: gate expands to all TUs that include it, runs tidy on those. Annotation job posts line-level comments.
4. PR introducing a tidy violation: gate fails CI (job red). Annotation job posts an inline suggestion comment on the offending line.
5. Push to main: full make -j16 clang-tidy runs (no filtering). Annotation job skipped (not a PR).
6. Fork PR: gate runs normally. Annotation job skipped (head.repo.full_name != github.repository).

Check the "Analysis - Run Clang-Tidy" step: should show file count and take <2 min for typical PRs vs 9:09 baseline. Check the PR for inline review comments after the annotation job completes.