Skip to content

Commit f1dd203

Browse files
greenc-FNALgreenc-FNAL
authored
Resolve clang-tidy-fix issues, separate header guard check/fix (#363)
* Resolve `clang-tidy-fix` issues, separate header guard check/fix Add Python script to check and fix header guards according to the project's naming convention (`X_Y_HEADER_EXT` format). Add GitHub Actions workflows for automated header guard checking and fixing via bot commands. The check workflow runs on PRs with header file changes and reports violations. The fix workflow can be triggered via bot comment to automatically correct guards. Improve clang-tidy workflow to reuse existing fixes from previous runs when possible, avoiding redundant analysis. Add artifact downloading from both check and fix workflows to apply cached fixes. Separate fix generation from application for better control. Update `handle-fix-commit` action to guard PR-specific steps with pull request number checks, preventing failures when running outside PR context. Remove `CreateClangTidyTargets.cmake` module as clang-tidy integration now uses `CMAKE_CXX_CLANG_TIDY` directly. Update `CMakeLists.txt` to report clang-tidy availability without creating custom targets. Disable `llvm-header-guard` check in `.clang-tidy` since the new script enforces project-specific conventions. Upgrade `actions/upload-artifact` from v5 to v6 in `handle-fix-commit` action. * Resolve relative path to absolute for comparison * Address faulty identification of closing `#endif` for guard annotation
1 parent abdbc0c commit f1dd203

File tree

9 files changed

+475
-43
lines changed

9 files changed

+475
-43
lines changed

‎.clang-tidy‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ Checks: >
1717
-cppcoreguidelines-pro-bounds-pointer-arithmetic,
1818
-cppcoreguidelines-macro-usage,
1919
-cppcoreguidelines-non-private-member-variables-in-classes,
20+
-llvm-header-guard, # Use scripts/fix_header_guards.py instead
2021
misc-*,
2122
-misc-non-private-member-variables-in-classes,
2223
-misc-no-recursion,

‎.github/actions/handle-fix-commit/action.yaml‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -55,14 +55,14 @@ runs:
5555
fi
5656
5757
- name: No changes to apply
58-
if: steps.check_changes.outputs.changes == 'false'
58+
if: steps.check_changes.outputs.changes == 'false' && github.event.pull_request.number
5959
uses: thollander/actions-comment-pull-request@24bffb9b452ba05a4f3f77933840a6a841d1b32b # v3.0.1
6060
with:
6161
message: 'No automatic ${{ inputs.tool }} fixes were necessary.'
6262

6363
- name: Get PR maintainer_can_modify property
6464
id: pr-properties
65-
if: steps.check_changes.outputs.changes == 'true'
65+
if: steps.check_changes.outputs.changes == 'true' && github.event.pull_request.number
6666
uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8.0.0
6767
with:
6868
script: |
@@ -123,7 +123,7 @@ runs:
123123
exit 1
124124
125125
- name: Notify of commit
126-
if: steps.commit_and_push.conclusion == 'success' && steps.commit_and_push.outputs.pushed == 'true'
126+
if: steps.commit_and_push.conclusion == 'success' && steps.commit_and_push.outputs.pushed == 'true' && github.event.pull_request.number
127127
uses: thollander/actions-comment-pull-request@24bffb9b452ba05a4f3f77933840a6a841d1b32b # v3.0.1
128128
with:
129129
message: |
@@ -141,13 +141,13 @@ runs:
141141
142142
- name: Upload patch
143143
if: steps.create_patch.outputs.patch_name
144-
uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0
144+
uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0
145145
with:
146146
name: fix-patch
147147
path: ${{ inputs.working-directory }}/${{ steps.create_patch.outputs.patch_name }}
148148

149149
- name: Comment with patch instructions
150-
if: steps.create_patch.outputs.patch_name
150+
if: steps.create_patch.outputs.patch_name && github.event.pull_request.number
151151
uses: thollander/actions-comment-pull-request@24bffb9b452ba05a4f3f77933840a6a841d1b32b # v3.0.1
152152
with:
153153
message: |

‎.github/workflows/clang-tidy-check.yaml‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ name: Clang-Tidy Check
33

44
permissions:
55
contents: read
6+
pull-requests: read
67

78

89
on:
@@ -121,7 +122,7 @@ jobs:
121122
build-type: Debug
122123
extra-options: "-DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DCMAKE_CXX_SCAN_FOR_MODULES=OFF -DCMAKE_CXX_CLANG_TIDY='clang-tidy;--export-fixes=clang-tidy-fixes.yaml'"
123124

124-
- name: Run clang-tidy using CMake target
125+
- name: Run clang-tidy using CMake
125126
id: tidy
126127
shell: bash
127128
run: |
@@ -130,7 +131,7 @@ jobs:
130131
131132
echo "➡️ Running clang-tidy checks..."
132133
cmake_status=0
133-
cmake --build . -j $(nproc) > clang-tidy.log 2>&1 || cmake_status=$?
134+
cmake --build . -j "$(nproc)" > clang-tidy.log 2>&1 || cmake_status=$?
134135
135136
# Distinguish tooling failures from issue detection by checking log content
136137
if [ "$cmake_status" -ne 0 ]; then

‎.github/workflows/clang-tidy-fix.yaml‎

Lines changed: 58 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -76,36 +76,82 @@ jobs:
7676
repository: ${{ needs.pre-check.outputs.repo }}
7777
token: ${{ secrets.WORKFLOW_PAT }}
7878

79+
- name: Try to download existing fixes from check workflow
80+
id: download_fixes_check
81+
if: needs.pre-check.outputs.tidy_checks == ''
82+
uses: dawidd6/action-download-artifact@fe9d59ce33ce92db8a6ac90b2c8be6b6d90417c8 # v15
83+
with:
84+
workflow: clang-tidy-check.yaml
85+
name: clang-tidy-report
86+
path: fixes
87+
branch: ${{ needs.pre-check.outputs.ref }}
88+
continue-on-error: true
89+
90+
- name: Try to download existing fixes from previous fix workflow
91+
id: download_fixes_fix
92+
if: steps.download_fixes_check.outcome != 'success' && needs.pre-check.outputs.tidy_checks == ''
93+
uses: dawidd6/action-download-artifact@fe9d59ce33ce92db8a6ac90b2c8be6b6d90417c8 # v15
94+
with:
95+
workflow: clang-tidy-fix.yaml
96+
name: clang-tidy-report
97+
path: fixes
98+
branch: ${{ needs.pre-check.outputs.ref }}
99+
continue-on-error: true
100+
101+
- name: Apply fixes from artifact if available
102+
id: apply_from_artifact
103+
if: needs.pre-check.outputs.tidy_checks == '' && (steps.download_fixes_check.outcome == 'success' || steps.download_fixes_fix.outcome == 'success')
104+
run: |
105+
if [ -f fixes/clang-tidy-fixes.yaml ]; then
106+
echo "Applying fixes from existing artifact..."
107+
cd phlex-src
108+
clang-apply-replacements ../fixes || true
109+
echo "applied=true" >> "$GITHUB_OUTPUT"
110+
else
111+
echo "applied=false" >> "$GITHUB_OUTPUT"
112+
fi
113+
79114
- name: Setup build environment
115+
if: steps.apply_from_artifact.outputs.applied != 'true'
80116
uses: Framework-R-D/phlex/.github/actions/setup-build-env@main
81117

82-
- name: Configure CMake for clang-tidy
118+
- name: Prepare CMake configuration options
119+
if: steps.apply_from_artifact.outputs.applied != 'true'
120+
id: prep_tidy_opts
83121
env:
84122
TIDY_CHECKS: ${{ needs.pre-check.outputs.tidy_checks }}
85123
run: |
86124
. /entrypoint.sh
87125
cd "$GITHUB_WORKSPACE"
88126
89-
CLANG_TIDY_OPTS="clang-tidy;--fix;--export-fixes=clang-tidy-fixes.yaml"
127+
CLANG_TIDY_OPTS="clang-tidy;--export-fixes=clang-tidy-fixes.yaml"
90128
if [ -n "$TIDY_CHECKS" ]; then
91129
CHECKS_NORMALIZED=$(echo "$TIDY_CHECKS" | tr ' ' ',')
92130
CLANG_TIDY_OPTS="${CLANG_TIDY_OPTS};--checks=-*,${CHECKS_NORMALIZED}"
93131
fi
132+
echo "clang_tidy_opts=${CLANG_TIDY_OPTS}" >> "$GITHUB_OUTPUT"
94133
95-
cmake -B phlex-build -S phlex-src \
96-
-DCMAKE_BUILD_TYPE=Debug \
97-
-DCMAKE_EXPORT_COMPILE_COMMANDS=ON \
98-
-DCMAKE_CXX_SCAN_FOR_MODULES=OFF \
99-
-DCMAKE_CXX_CLANG_TIDY="${CLANG_TIDY_OPTS}"
134+
- name: Configure CMake (Debug)
135+
if: steps.apply_from_artifact.outputs.applied != 'true'
136+
uses: Framework-R-D/phlex/.github/actions/configure-cmake@main
137+
with:
138+
build-type: Debug
139+
extra-options: "-DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DCMAKE_CXX_SCAN_FOR_MODULES=OFF -DCMAKE_CXX_CLANG_TIDY='${{ steps.prep_tidy_opts.outputs.clang_tidy_opts }}'"
100140

101-
- name: Apply clang-tidy fixes using CMake build
141+
- name: Generate clang-tidy fixes using CMake build
142+
if: steps.apply_from_artifact.outputs.applied != 'true'
102143
run: |
103144
. /entrypoint.sh
104145
cd "$GITHUB_WORKSPACE/phlex-build"
146+
cmake --build . -j "$(nproc)" || true
105147
106-
echo "Applying clang-tidy fixes..."
107-
cmake --build . -j $(nproc) || true
108-
echo "Clang-tidy fixes applied"
148+
- name: Apply clang-tidy fixes
149+
if: steps.apply_from_artifact.outputs.applied != 'true'
150+
run: |
151+
cd "$GITHUB_WORKSPACE/phlex-build"
152+
if [ -f clang-tidy-fixes.yaml ]; then
153+
clang-apply-replacements . || true
154+
fi
109155
110156
- name: Upload clang-tidy report
111157
if: always()
@@ -114,6 +160,7 @@ jobs:
114160
name: clang-tidy-report
115161
path: phlex-build/clang-tidy-fixes.yaml
116162
retention-days: 7
163+
if-no-files-found: ignore
117164

118165
- name: Handle fix commit
119166
uses: Framework-R-D/phlex/.github/actions/handle-fix-commit@main

‎.github/workflows/header-guards-check.yaml‎

Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
name: Header Guards Check
2+
run-name: "${{ github.actor }} checking header guards"
3+
4+
permissions:
5+
contents: read
6+
pull-requests: read
7+
8+
on:
9+
pull_request:
10+
workflow_dispatch:
11+
inputs:
12+
ref:
13+
description: "The branch, ref, or SHA to checkout. Defaults to the repository's default branch."
14+
required: false
15+
type: string
16+
workflow_call:
17+
inputs:
18+
checkout-path:
19+
description: "Path to check out code to"
20+
required: false
21+
type: string
22+
skip-relevance-check:
23+
description: "Bypass relevance check"
24+
required: false
25+
type: boolean
26+
default: false
27+
pr-base-sha:
28+
description: "Base SHA of the PR for relevance check"
29+
required: false
30+
type: string
31+
pr-head-sha:
32+
description: "Head SHA of the PR for relevance check"
33+
required: false
34+
type: string
35+
ref:
36+
description: "The branch, ref, or SHA to checkout"
37+
required: false
38+
type: string
39+
repo:
40+
description: "The repository to checkout from"
41+
required: false
42+
type: string
43+
44+
env:
45+
local_checkout_path: ${{ (github.event_name == 'workflow_call' && inputs.checkout-path) || format('{0}-src', github.event.repository.name) }}
46+
47+
jobs:
48+
pre-check:
49+
runs-on: ubuntu-latest
50+
outputs:
51+
is_act: ${{ steps.detect_act.outputs.is_act }}
52+
ref: ${{ (github.event_name == 'workflow_call' && inputs.ref) || (github.event_name == 'workflow_dispatch' && (github.event.inputs.ref || github.ref)) || github.sha }}
53+
repo: ${{ (github.event_name == 'workflow_call' && inputs.repo) || github.repository }}
54+
base_sha: ${{ (github.event_name == 'workflow_call' && inputs.pr-base-sha) || github.event.pull_request.base.sha || github.event.before }}
55+
steps:
56+
- name: Detect act environment
57+
id: detect_act
58+
uses: Framework-R-D/phlex/.github/actions/detect-act-env@main
59+
60+
detect-changes:
61+
needs: pre-check
62+
if: >
63+
needs.pre-check.result == 'success' &&
64+
github.event_name != 'workflow_dispatch' &&
65+
(
66+
github.event_name != 'workflow_call' ||
67+
(inputs.skip-relevance-check != 'true' && github.event.inputs == null && github.event.comment == null)
68+
) &&
69+
needs.pre-check.outputs.is_act != 'true'
70+
runs-on: ubuntu-latest
71+
permissions:
72+
contents: read
73+
packages: read
74+
outputs:
75+
has_changes: ${{ steps.filter.outputs.matched }}
76+
steps:
77+
- name: Checkout code
78+
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
79+
with:
80+
fetch-depth: 0
81+
path: ${{ env.local_checkout_path }}
82+
ref: ${{ needs.pre-check.outputs.ref }}
83+
repository: ${{ needs.pre-check.outputs.repo }}
84+
85+
- name: Detect relevant changes
86+
id: filter
87+
uses: Framework-R-D/phlex/.github/actions/detect-relevant-changes@main
88+
with:
89+
repo-path: ${{ env.local_checkout_path }}
90+
base-ref: ${{ needs.pre-check.outputs.base_sha }}
91+
head-ref: ${{ (github.event_name == 'workflow_call' && inputs.pr-head-sha) || needs.pre-check.outputs.ref }}
92+
file-type: cpp
93+
94+
- name: Report detection outcome
95+
run: |
96+
if [ "${{ steps.filter.outputs.matched }}" != "true" ]; then
97+
echo "::notice::No header file changes detected; check will be skipped."
98+
else
99+
echo "::group::Header files"
100+
printf '%s\n' "${{ steps.filter.outputs.matched_files }}"
101+
echo "::endgroup::"
102+
fi
103+
104+
header-guards-check:
105+
needs: [pre-check, detect-changes]
106+
if: >
107+
always() &&
108+
(
109+
needs.detect-changes.result == 'skipped' ||
110+
(
111+
needs.detect-changes.result == 'success' &&
112+
needs.detect-changes.outputs.has_changes == 'true'
113+
)
114+
)
115+
runs-on: ubuntu-latest
116+
permissions:
117+
contents: read
118+
119+
steps:
120+
- name: Checkout code
121+
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
122+
with:
123+
ref: ${{ needs.pre-check.outputs.ref }}
124+
path: ${{ env.local_checkout_path }}
125+
repository: ${{ needs.pre-check.outputs.repo }}
126+
127+
- name: Set up Python
128+
uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0
129+
with:
130+
python-version: '3.12'
131+
132+
- name: Check header guards
133+
id: check
134+
working-directory: ${{ env.local_checkout_path }}
135+
run: |
136+
python3 scripts/fix_header_guards.py --check --root . phlex plugins form > check_output.txt 2>&1 || echo "has_issues=true" >> "$GITHUB_OUTPUT"
137+
cat check_output.txt
138+
139+
- name: Report results
140+
if: always() && steps.check.outputs.has_issues == 'true'
141+
run: |
142+
echo "::error::Header guard check failed."
143+
echo "::error::Comment '@${{ github.event.repository.name }}bot header-guards-fix' on the PR to auto-fix."
144+
exit 1
145+
146+
header-guards-check-skipped:
147+
needs: [pre-check, detect-changes]
148+
if: >
149+
needs.pre-check.result == 'success' &&
150+
github.event_name != 'workflow_dispatch' &&
151+
(
152+
github.event_name != 'workflow_call' ||
153+
(inputs.skip-relevance-check != 'true' && github.event.inputs == null && github.event.comment == null)
154+
) &&
155+
needs.pre-check.outputs.is_act != 'true' &&
156+
(needs.detect-changes.result == 'success' && needs.detect-changes.outputs.has_changes != 'true')
157+
runs-on: ubuntu-latest
158+
permissions:
159+
contents: read
160+
161+
steps:
162+
- name: No relevant header changes detected
163+
run: echo "::notice::No header file changes detected; header-guards-check skipped."

0 commit comments

Comments
 (0)