Skip to content

Commit c31425f

Browse files
committed
Add non-blocking cpp-linter PR workflow with downloadable reports
- Add cpp-linker-pr.yml with changed-files-only cpp-linter checks - Install latest clang-tidy from LLVM apt repo and use detected major - Upload cpp-linter report artifact with failures and fix guidance - Add strict-mode knob (disabled by default) for future enforcement - Add eld_cpp_linter_helpers.sh with check/fix helpers, --build-directory and --file support Signed-off-by: Shankar Easwaran <seaswara@qti.qualcomm.com>
1 parent 228eaa0 commit c31425f

6 files changed

Lines changed: 701 additions & 11 deletions

File tree

.clang-tidy

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
---
2+
# ELD clang-tidy policy following LLVM-style checks and tooling behavior.
3+
# Keep this practical: strong bug-finding checks on by default, avoid naming/style
4+
# checks that generate high-noise during initial adoption.
5+
6+
Checks: >
7+
-*,
8+
clang-analyzer-*,
9+
bugprone-*,
10+
performance-*,
11+
modernize-use-nullptr,
12+
modernize-use-override,
13+
llvm-header-guard,
14+
llvm-include-order,
15+
llvm-namespace-comment,
16+
-readability-identifier-naming,
17+
-readability-magic-numbers
18+
19+
WarningsAsErrors: ''
20+
HeaderFilterRegex: '.*'
21+
FormatStyle: file
22+
UseColor: true
23+
...

.github/workflows/ci.yml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@ on:
1717
- '.github/workflows/update-build-dashboard-data.yml'
1818
- '.github/workflows/scripts/record_builds.py'
1919
- '.github/workflows/BuildStatusDataRecorder/action.yaml'
20-
- '.github/workflows/zephyr-nightly.yml'
20+
- '.github/workflows/clang-format-pr.yml'
21+
- '.github/workflows/cpp-linter-pr.yml'
2122

2223
permissions:
2324
contents: write
Lines changed: 222 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,222 @@
1+
name: Cpp Linter PR Check
2+
3+
on:
4+
# Temporarily disable PR-triggered runs.
5+
# pull_request:
6+
# Run on push for now.
7+
push:
8+
workflow_dispatch:
9+
10+
permissions:
11+
# Required for checkout and metadata reads.
12+
contents: read
13+
# Allows cpp-linter action to publish check-run results.
14+
checks: write
15+
# Needed for sticky PR comments with report links.
16+
pull-requests: write
17+
18+
env:
19+
# Knob for future strict mode. Keep false for now to avoid failing PRs.
20+
CPP_LINTER_ENFORCE: "false"
21+
# Build directory for external LLVM ELD build (compile_commands.json lives here).
22+
CPP_LINTER_BUILD_DIR: "llvm-build-ext-clang-pr"
23+
CPP_LINTER_CCACHE_DIR: ".ccache-external-llvm"
24+
25+
jobs:
26+
cpp-linter:
27+
# Keep the check non-blocking while surfacing actionable diagnostics.
28+
runs-on: ubuntu-latest
29+
steps:
30+
- name: Checkout PR
31+
uses: actions/checkout@v6
32+
with:
33+
fetch-depth: 0
34+
35+
- name: Collect changed C/C++ files
36+
id: changed-files
37+
shell: bash
38+
run: |
39+
set -euo pipefail
40+
# Build a deterministic file list used by all later steps.
41+
BASE_REF="${{ github.base_ref }}"
42+
git fetch origin "${BASE_REF}" --depth=1
43+
git diff --name-only --diff-filter=ACMRT "origin/${BASE_REF}...HEAD" \
44+
| grep -E '\.(c|cc|cpp|cxx|h|hh|hpp|hxx|inc|def)$' > .cpp-linter-files.txt || true
45+
COUNT="$(wc -l < .cpp-linter-files.txt | tr -d ' ')"
46+
echo "count=${COUNT}" >> "$GITHUB_OUTPUT"
47+
if [[ "${COUNT}" -eq 0 ]]; then
48+
echo "has_files=false" >> "$GITHUB_OUTPUT"
49+
else
50+
echo "has_files=true" >> "$GITHUB_OUTPUT"
51+
fi
52+
echo "Changed files count: ${COUNT}"
53+
54+
- name: Install latest clang-tidy from LLVM apt repo
55+
id: toolchain
56+
if: steps.changed-files.outputs.has_files == 'true'
57+
shell: bash
58+
run: |
59+
set -euo pipefail
60+
# Install toolchain from LLVM apt so clang-tidy/clang-format stay current.
61+
sudo apt-get update
62+
sudo apt-get install -y wget gpg lsb-release ccache cmake ninja-build
63+
wget -qO- https://apt.llvm.org/llvm-snapshot.gpg.key \
64+
| gpg --dearmor \
65+
| sudo tee /usr/share/keyrings/llvm-archive-keyring.gpg >/dev/null
66+
UBUNTU_CODENAME="$(lsb_release -cs)"
67+
echo "deb [signed-by=/usr/share/keyrings/llvm-archive-keyring.gpg] http://apt.llvm.org/${UBUNTU_CODENAME}/ llvm-toolchain-${UBUNTU_CODENAME} main" \
68+
| sudo tee /etc/apt/sources.list.d/llvm.list >/dev/null
69+
sudo apt-get update
70+
sudo apt-get install -y clang clang-tidy llvm llvm-dev
71+
clang-tidy --version
72+
clang-format --version
73+
# cpp-linter action expects a clang major version string.
74+
CLANG_MAJOR="$(clang-tidy --version | sed -n 's/.*version \([0-9][0-9]*\).*/\1/p' | head -n1)"
75+
echo "clang_major=${CLANG_MAJOR}" >> "$GITHUB_OUTPUT"
76+
77+
- name: Prepare external LLVM release layout
78+
id: llvm-layout
79+
if: steps.changed-files.outputs.has_files == 'true'
80+
shell: bash
81+
run: |
82+
set -euo pipefail
83+
# Create a lightweight external LLVM prefix expected by configure_external_llvm.sh.
84+
ROOT="${GITHUB_WORKSPACE}"
85+
EXT_LLVM_DIR="${ROOT}/external-llvm-release"
86+
mkdir -p "${EXT_LLVM_DIR}/bin" "${EXT_LLVM_DIR}/lib/cmake"
87+
LLVM_PREFIX="$(llvm-config --prefix)"
88+
LLVM_CMAKE_DIR="$(llvm-config --cmakedir)"
89+
ln -sf "$(command -v clang)" "${EXT_LLVM_DIR}/bin/clang"
90+
ln -sf "$(command -v clang++)" "${EXT_LLVM_DIR}/bin/clang++"
91+
ln -sf "${LLVM_PREFIX}/bin/llvm-tblgen" "${EXT_LLVM_DIR}/bin/llvm-tblgen"
92+
ln -sfn "${LLVM_CMAKE_DIR}" "${EXT_LLVM_DIR}/lib/cmake/llvm"
93+
echo "ext_llvm_dir=${EXT_LLVM_DIR}" >> "$GITHUB_OUTPUT"
94+
95+
- name: Configure and build ELD with external LLVM
96+
if: steps.changed-files.outputs.has_files == 'true'
97+
shell: bash
98+
run: |
99+
set -euo pipefail
100+
# Produce compile_commands.json for clang-tidy-driven checks.
101+
mkdir -p "${CPP_LINTER_BUILD_DIR}" "${CPP_LINTER_CCACHE_DIR}"
102+
./configure_external_llvm.sh \
103+
"${{ steps.llvm-layout.outputs.ext_llvm_dir }}" \
104+
"${CPP_LINTER_BUILD_DIR}" \
105+
--target ld.eld \
106+
--ccache-dir "${CPP_LINTER_CCACHE_DIR}"
107+
108+
- name: Run C/C++ linter (non-blocking)
109+
id: cpp-lint
110+
uses: cpp-linter/cpp-linter-action@v2
111+
if: steps.changed-files.outputs.has_files == 'true'
112+
continue-on-error: true
113+
env:
114+
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
115+
with:
116+
version: "${{ steps.toolchain.outputs.clang_major }}"
117+
# Respect repository .clang-format.
118+
style: "file"
119+
# Use project defaults for clang-tidy checks.
120+
tidy-checks: ""
121+
# Limit scope/noise to touched files only.
122+
files-changed-only: true
123+
# Keep PR clean; report via artifact/link instead.
124+
thread-comments: false
125+
step-summary: false
126+
file-annotations: false
127+
128+
- name: Run helper linter (non-blocking detailed diagnostics)
129+
id: helper-lint
130+
if: steps.changed-files.outputs.has_files == 'true'
131+
continue-on-error: true
132+
shell: bash
133+
run: |
134+
set -euo pipefail
135+
# Helper script collects richer clang-tidy diagnostics into a log artifact.
136+
source ./etc/bash/eld_cpp_linter_helpers.sh
137+
eld_cpp_linter_check \
138+
--base-branch "${{ github.base_ref }}" \
139+
--build-directory "${CPP_LINTER_BUILD_DIR}" \
140+
> cpp-linter-helper.log 2>&1 || true
141+
142+
- name: Write cpp-linter report
143+
if: always()
144+
shell: bash
145+
run: |
146+
# Single downloadable report for reviewers and authors.
147+
{
148+
echo "Cpp Linter PR Report"
149+
echo "===================="
150+
echo "Date (UTC): $(date -u '+%Y-%m-%d %H:%M:%S')"
151+
echo "Base branch: ${{ github.base_ref }}"
152+
echo "clang-tidy version:"
153+
clang-tidy --version || true
154+
echo
155+
echo "Build directory: ${CPP_LINTER_BUILD_DIR}"
156+
echo "ccache directory: ${CPP_LINTER_CCACHE_DIR}"
157+
echo
158+
echo "Changed C/C++ files:"
159+
if [[ -s .cpp-linter-files.txt ]]; then
160+
sed 's/^/ - /' .cpp-linter-files.txt
161+
else
162+
echo " (none)"
163+
fi
164+
echo
165+
echo "cpp-linter action outputs:"
166+
echo " checks-failed: ${{ steps.cpp-lint.outputs.checks-failed }}"
167+
echo " tidy-checks-failed: ${{ steps.cpp-lint.outputs.tidy-checks-failed }}"
168+
echo " format-checks-failed: ${{ steps.cpp-lint.outputs.format-checks-failed }}"
169+
echo
170+
echo "What to fix:"
171+
echo " 1. Download the 'cpp-linter-report' artifact from this workflow run."
172+
echo " 2. For local reproduction/fixes, use:"
173+
echo " source etc/bash/eld_cpp_linter_helpers.sh"
174+
echo " eld_cpp_linter_check --base-branch '${{ github.base_ref }}' --build-directory '<build-dir>'"
175+
echo " eld_cpp_linter_fix --base-branch '${{ github.base_ref }}' --build-directory '<build-dir>'"
176+
echo
177+
echo "Helper linter detailed log:"
178+
if [[ -f cpp-linter-helper.log ]]; then
179+
sed 's/^/ /' cpp-linter-helper.log
180+
else
181+
echo " (helper log not generated)"
182+
fi
183+
echo
184+
echo "PR enforcement knob:"
185+
echo " CPP_LINTER_ENFORCE=${CPP_LINTER_ENFORCE}"
186+
} > cpp-linter-report.txt
187+
188+
- name: Upload cpp-linter report artifact
189+
if: always()
190+
uses: actions/upload-artifact@v4
191+
with:
192+
# Stable artifact name referenced by PR comment and docs.
193+
name: cpp-linter-report
194+
path: |
195+
cpp-linter-report.txt
196+
cpp-linter-helper.log
197+
.cpp-linter-files.txt
198+
if-no-files-found: error
199+
200+
- name: Report non-blocking lint warnings
201+
if: ${{ steps.cpp-lint.outputs.checks-failed != '' && steps.cpp-lint.outputs.checks-failed != '0' }}
202+
run: |
203+
# Keep job green for now, but visibly flag that follow-up is required.
204+
echo "::warning::cpp-linter reported ${{ steps.cpp-lint.outputs.checks-failed }} warning(s). See workflow artifacts: cpp-linter-report."
205+
206+
- name: Post PR comment with report link
207+
if: ${{ steps.cpp-lint.outputs.checks-failed != '' && steps.cpp-lint.outputs.checks-failed != '0' }}
208+
uses: marocchino/sticky-pull-request-comment@v2
209+
with:
210+
# Sticky comment updates in-place across pushes.
211+
header: cpp-linter-report
212+
message: |
213+
cpp-linter found **${{ steps.cpp-lint.outputs.checks-failed }}** warning(s).
214+
Download the **cpp-linter-report** artifact from this workflow run:
215+
https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}
216+
217+
- name: Optional strict mode (currently disabled)
218+
if: ${{ env.CPP_LINTER_ENFORCE == 'true' && steps.cpp-lint.outputs.checks-failed != '' && steps.cpp-lint.outputs.checks-failed != '0' }}
219+
run: |
220+
# Switch this on later to enforce lint cleanliness.
221+
echo "cpp-linter strict mode enabled and checks failed."
222+
exit 1

README.md

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,81 @@ eld_clang_format_fix <base-branch>
253253
Pull requests run `.github/workflows/clang-format-pr.yml`, which fails if any
254254
changed C/C++ source file is not formatted with `clang-format --style=file`.
255255

256+
## Clang-tidy and cpp-linter
257+
258+
ELD PRs also run clang-tidy-based lint checks via:
259+
`.github/workflows/cpp-linter-pr.yml`
260+
261+
Current behavior:
262+
- Runs on changed C/C++ files in the PR.
263+
- Uses external-LLVM build setup to generate `compile_commands.json`.
264+
- Runs non-blocking for now (does not fail the PR by default).
265+
- Uploads downloadable logs/artifacts (including detailed diagnostics).
266+
267+
### Local helper functions
268+
269+
Use the helper script at:
270+
`etc/bash/eld_cpp_linter_helpers.sh`
271+
272+
To enable the functions in your shell:
273+
274+
```bash
275+
source </path/to/eld>/etc/bash/eld_cpp_linter_helpers.sh
276+
```
277+
278+
For usage/help:
279+
280+
```bash
281+
eld_cpp_linter_check --help
282+
```
283+
284+
Common usage:
285+
286+
```bash
287+
# Check changed files against base branch
288+
eld_cpp_linter_check --base-branch main --build-directory <build-dir>
289+
290+
# Apply clang-tidy fixes on changed files
291+
eld_cpp_linter_fix --base-branch main --build-directory <build-dir>
292+
293+
# Check or fix a single file
294+
eld_cpp_linter_check --build-directory <build-dir> --file path/to/file.cpp
295+
eld_cpp_linter_fix --build-directory <build-dir> --file path/to/file.cpp
296+
```
297+
298+
### How to skip files from clang-tidy checks
299+
300+
1. Skip in PR workflow input list (temporary CI-level skip):
301+
302+
```bash
303+
git diff --name-only --diff-filter=ACMRT "origin/${BASE_REF}...HEAD" \
304+
| grep -E '\.(c|cc|cpp|cxx|h|hh|hpp|hxx|inc|def)$' \
305+
| grep -Ev '^(path/to/file1\.cpp|path/to/legacy/.*)$' \
306+
> .cpp-linter-files.txt || true
307+
```
308+
309+
2. Run helpers on a specific file only (local scope control):
310+
311+
```bash
312+
eld_cpp_linter_check --build-directory <build-dir> --file path/to/file.cpp
313+
```
314+
315+
3. Add source-level suppressions where justified:
316+
- `// NOLINT(<check-name>)`
317+
- `// NOLINTBEGIN(<check-name>)` / `// NOLINTEND(<check-name>)`
318+
319+
4. Use repo-level `.clang-tidy` policy filters, if maintained, for broad path/check tuning.
320+
321+
### Where to find clang-tidy check names
322+
323+
- Official clang-tidy checks list:
324+
`https://clang.llvm.org/extra/clang-tidy/checks/list.html`
325+
- Checks supported by your installed clang-tidy version:
326+
327+
```bash
328+
clang-tidy -list-checks -checks='*'
329+
```
330+
256331
## ELD Build Status
257332

258333
Live status of workflows building with ELD

0 commit comments

Comments
 (0)