Skip to content

Commit f4337b6

Browse files
joaodinissfclaude
andcommitted
ci: scope PMD/CPD/Checkstyle to changed modules in the lint lane
compute-spotbugs-skip.sh becomes compute-analysis-skip.sh with a mode argument: `spotbugs` injects spotbugs.skip as before, `lint` injects pmd.skip, cpd.skip and checkstyle.skip, and each mode exports its -pl/-am reactor scope args. The lint lane gains the scope step and passes LINT_SCOPE_ARGS to both invocations; `compile` stays in the PMD/Checkstyle invocation because PMD's type-resolving rules need Tycho's aux-classpath (skip-injected -am dependencies compile but are not analysed). Changes under ddk-configuration (rulesets, filters) now also trigger the full-scan fail-safe in both lanes. Code Scanning note: repo-wide alert state reflects the default branch, which receives no lint/spotbugs analyses (verify runs on pull_request only), so a scoped upload that omits unchanged modules can only affect PR-context annotations — the same property the spotbugs category has had since the per-module skip landed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 9e7e1c9 commit f4337b6

2 files changed

Lines changed: 90 additions & 46 deletions

File tree

Lines changed: 52 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
#!/usr/bin/env bash
22
#
3-
# Scope SpotBugs to a pull request's changed modules.
3+
# Scope static analysis (SpotBugs, or PMD/CPD/Checkstyle) to a pull request's
4+
# changed modules.
45
#
5-
# Default is RUN (analyze). On a PR this injects <spotbugs.skip>true</spotbugs.skip>
6-
# into every UNCHANGED reactor module's pom, so spotbugs-maven-plugin skips the goal —
7-
# and therefore the per-module JVM fork (SpotBugsMojo gates on `skip` before forking) —
8-
# for those modules. The full-reactor compile is left intact (a changed module is still
9-
# analysed with its complete aux-classpath). Master/snapshot builds run a full scan;
10-
# this script is invoked on pull_request only.
6+
# Default is RUN (analyze). On a PR this injects <TOOL.skip>true</TOOL.skip>
7+
# properties into every UNCHANGED reactor module's pom, so the analysis mojos
8+
# skip those modules — for SpotBugs that also skips the per-module JVM fork
9+
# (SpotBugsMojo gates on `skip` before forking). A changed module is still
10+
# analysed with its complete aux-classpath: the -am-pulled unchanged
11+
# dependencies compile but are not analysed. Master/snapshot builds run a full
12+
# scan; this script is invoked on pull_request only.
1113
#
1214
# Accepted trade-off: only changed modules are analysed. A change whose effect shows
1315
# up as a finding in an unchanged dependent module surfaces on the master full scan.
@@ -17,16 +19,25 @@
1719
# only trimmed ~17% of the goal vs ~88% for this per-module skip (measured on this
1820
# reactor). A small upstream SpotBugs early-exit (skip the run when no application class
1921
# matches the screener) would make onlyAnalyze competitive; if that ever lands, switch
20-
# to onlyAnalyze and delete this script (tracked in #1455 / spotbugs/spotbugs#3796).
22+
# to onlyAnalyze and delete the spotbugs mode here (tracked in #1455 /
23+
# spotbugs/spotbugs#3796).
2124
#
22-
# On top of the skips, the changed reactor modules are exported as SPOTBUGS_SCOPE_ARGS
23-
# ("-pl <changed> -am") so the lane builds only those modules plus their upstream
24-
# dependencies instead of the full reactor. The -am-pulled unchanged dependencies still
25-
# carry the injected skip: they compile (complete aux-classpath) but are not analysed.
25+
# On top of the skips, the changed reactor modules are exported as
26+
# SPOTBUGS_SCOPE_ARGS / LINT_SCOPE_ARGS ("-pl <changed> -am") so the lane builds
27+
# only those modules plus their upstream dependencies instead of the full reactor.
28+
# The lane's gate cross-checks <MODE>_KEPT / <MODE>_EXPECT_REPORTS so a build
29+
# failure swallowed by --fail-never can never pass as "nothing to scan".
2630
#
27-
# Run from the repository root. Usage: compute-spotbugs-skip.sh <base-sha>
31+
# Run from the repository root. Usage: compute-analysis-skip.sh <base-sha> <spotbugs|lint>
2832
set -euo pipefail
2933
base="${1:?base sha required}"
34+
mode="${2:?mode required: spotbugs|lint}"
35+
36+
case "$mode" in
37+
spotbugs) props="spotbugs.skip"; prefix="SPOTBUGS" ;;
38+
lint) props="pmd.skip cpd.skip checkstyle.skip"; prefix="LINT" ;;
39+
*) echo "unknown mode: $mode" >&2; exit 2 ;;
40+
esac
3041

3142
# --no-renames reports a move as delete + add, so both the old and the new module
3243
# count as changed; D keeps deletion-only changes (e.g. a removed ruleset) visible.
@@ -50,8 +61,7 @@ ${module_dirs}
5061
EOF
5162

5263
# 1) A change to shared build/config can affect any module -> full scan (skip nothing).
53-
# ddk-configuration holds the analyzers' rulesets and filters (e.g. the SpotBugs
54-
# exclusion-filter), so a change there must re-scan everything, not skip silently.
64+
# ddk-configuration holds the analyzers' rulesets and filters, so it counts too.
5565
# ddk-target defines the target platform every module resolves against.
5666
# Fail safe: the worst case here is "analyse everything", never "analyse nothing".
5767
while IFS= read -r f; do
@@ -65,10 +75,10 @@ while IFS= read -r f; do
6575
# full scan analysed all of them (not just >=1) — a mojo death swallowed by
6676
# --fail-never can't pass as long as one sibling reported.
6777
if [ -n "${GITHUB_ENV:-}" ]; then
68-
echo "SPOTBUGS_KEPT=all" >> "$GITHUB_ENV"
69-
echo "SPOTBUGS_EXPECT_REPORTS=${all_source_modules}" >> "$GITHUB_ENV"
78+
echo "${prefix}_KEPT=all" >> "$GITHUB_ENV"
79+
echo "${prefix}_EXPECT_REPORTS=${all_source_modules}" >> "$GITHUB_ENV"
7080
fi
71-
echo "Build/config change ($f) -> full SpotBugs scan (no skips)."
81+
echo "Build/config change ($f) -> full ${mode} scan (no skips)."
7282
exit 0
7383
;;
7484
esac
@@ -81,18 +91,20 @@ EOF
8191
# grep's no-match exit would otherwise kill the script under pipefail.
8292
changed_mods=$(printf '%s\n' "${changed}" | { grep '/' || true; } | cut -d/ -f1 | sort -u)
8393

84-
# 3) Idempotently inject the skip property; handle poms with and without <properties>.
94+
# 3) Idempotently inject the skip properties; handle poms with and without <properties>.
8595
# sed -i.bak + rm is portable across GNU (CI) and BSD (local) sed.
8696
inject_skip() {
87-
local pom="$1/pom.xml"
97+
local pom="$1/pom.xml" prop
8898
[ -f "$pom" ] || return 0
89-
if grep -q '<spotbugs\.skip>' "$pom"; then return 0; fi
90-
if grep -q '<properties>' "$pom"; then
91-
sed -i.bak 's#<properties>#<properties>\n <spotbugs.skip>true</spotbugs.skip>#' "$pom"
92-
else
93-
sed -i.bak 's#</project># <properties>\n <spotbugs.skip>true</spotbugs.skip>\n </properties>\n</project>#' "$pom"
94-
fi
95-
rm -f "$pom.bak"
99+
for prop in $props; do
100+
if grep -q "<${prop//./\\.}>" "$pom"; then continue; fi
101+
if grep -q '<properties>' "$pom"; then
102+
sed -i.bak "s#<properties>#<properties>\n <${prop}>true</${prop}>#" "$pom"
103+
else
104+
sed -i.bak "s#</project># <properties>\n <${prop}>true</${prop}>\n </properties>\n</project>#" "$pom"
105+
fi
106+
rm -f "$pom.bak"
107+
done
96108
}
97109

98110
# 4) Skip every reactor module that was not touched by this PR. Kept modules with a
@@ -108,7 +120,7 @@ while IFS= read -r mod; do
108120
kept=$((kept + 1))
109121
kept_pl="${kept_pl:+${kept_pl},}../${mod}"
110122
# Only bundles with sources reliably emit a report (a source-less bundle,
111-
# e.g. pure branding, has nothing for the analyzer to write a SARIF about).
123+
# e.g. pure branding, has nothing for PMD to write a SARIF about).
112124
if [ -f "${mod}/META-INF/MANIFEST.MF" ] && [ -d "${mod}/src" ]; then
113125
expect_reports="${expect_reports:+${expect_reports} }${mod}"
114126
fi
@@ -120,6 +132,13 @@ done <<EOF
120132
${module_dirs}
121133
EOF
122134

135+
# 5) Scope the reactor to the changed modules + their upstream dependencies. ddk-target
136+
# is always kept in the -pl list: the target-definition artifact is referenced by
137+
# target-platform-configuration, not by any MANIFEST, so -am never pulls it — without
138+
# it in the reactor Tycho falls back to a local-repository copy, which fails on a
139+
# cold cache and can silently resolve a stale target definition on a warm one.
140+
# With no analysable changed module (docs-only, or source-less-only) no scope args
141+
# are exported; the workflow skips the lane's Maven step(s) entirely on <MODE>_KEPT=0.
123142
# The gate's presence check needs to distinguish "all modules skip-injected"
124143
# (zero reports is the expected state) from "the analysis silently died".
125144
# If the only changed modules are source-less (feature / target / repository — nothing
@@ -132,25 +151,18 @@ else
132151
effective_kept=$kept
133152
fi
134153
if [ -n "${GITHUB_ENV:-}" ]; then
135-
echo "SPOTBUGS_KEPT=${effective_kept}" >> "$GITHUB_ENV"
154+
echo "${prefix}_KEPT=${effective_kept}" >> "$GITHUB_ENV"
136155
fi
137156

138-
# 5) Scope the reactor to the changed modules + their upstream dependencies. ddk-target
139-
# is always kept in the -pl list: the target-definition artifact is referenced by
140-
# target-platform-configuration, not by any MANIFEST, so -am never pulls it — without
141-
# it in the reactor Tycho falls back to a local-repository copy, which fails on a
142-
# cold cache and can silently resolve a stale target definition on a warm one.
143-
# With no analysable changed module (docs-only, or source-less-only) no scope args
144-
# are exported; the workflow skips the Maven step entirely on SPOTBUGS_KEPT=0.
145157
if [ "$effective_kept" -gt 0 ] && [ -n "${GITHUB_ENV:-}" ]; then
146-
echo "SPOTBUGS_SCOPE_ARGS=-pl ../ddk-target,${kept_pl} -am" >> "$GITHUB_ENV"
147-
echo "SPOTBUGS_EXPECT_REPORTS=${expect_reports}" >> "$GITHUB_ENV"
158+
echo "${prefix}_SCOPE_ARGS=-pl ../ddk-target,${kept_pl} -am" >> "$GITHUB_ENV"
159+
echo "${prefix}_EXPECT_REPORTS=${expect_reports}" >> "$GITHUB_ENV"
148160
fi
149161

150-
echo "SpotBugs scope: scanning ${effective_kept} changed module(s), skipping ${skipped} unchanged."
162+
echo "${mode} scope: scanning ${effective_kept} changed module(s), skipping ${skipped} unchanged."
151163
echo "Changed modules: ${changed_mods:-<none>}"
152164
if [ "$kept" -gt 0 ] && [ "$effective_kept" -eq 0 ]; then
153-
echo "Only source-less modules changed (no analysable sources) -> no-op (KEPT=0)."
165+
echo "Only source-less modules changed (no analysable sources) -> no-op (${prefix}_KEPT=0)."
154166
fi
155167
if [ "$effective_kept" -gt 0 ]; then
156168
echo "Reactor scope args: -pl ../ddk-target,${kept_pl} -am"

‎.github/workflows/verify.yml‎

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ jobs:
2828
runs-on: ubuntu-24.04
2929
steps:
3030
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
31+
with:
32+
fetch-depth: 0 # need the PR base commit to diff the changed modules
3133
- uses: actions/setup-java@de7274f081f381c8f8158605e0321c36c376e2e6 # v6.0.1
3234
with:
3335
distribution: 'temurin'
@@ -43,17 +45,33 @@ jobs:
4345
key: ${{ runner.os }}-maven-publish-${{ hashFiles('**/pom.xml', '**/*.target') }}
4446
restore-keys: ${{ runner.os }}-maven-publish-
4547

48+
- name: Scope static analysis to the PR's changed modules
49+
# Injects pmd/cpd/checkstyle skip properties into unchanged module poms and
50+
# exports LINT_SCOPE_ARGS (-pl <changed> -am) so only the changed modules and
51+
# their upstream deps build (skip-injected deps compile for PMD's type
52+
# resolution but are not analysed). Build/config change -> full scan, full
53+
# reactor. pull_request only; master/snapshot run a full scan.
54+
run: bash .github/scripts/compute-analysis-skip.sh "${{ github.event.pull_request.base.sha }}" lint
55+
4656
- name: PMD + Checkstyle reports (SARIF)
4757
# PMD: SarifRenderer FQCN — emits pmd.sarif.json AND keeps pmd.xml.
4858
# Checkstyle: output.format=sarif — SARIF content in checkstyle-result.xml.
4959
# CPD is excluded here: the global -Dformat flag uses PMD's Renderer
5060
# hierarchy and would ClassCastException CPD's CPDReportRenderer.
61+
# `compile` stays: PMD's type-resolving rules need Tycho's aux-classpath.
62+
# jgit.dirtyWorkingTree=ignore: the scope step edits poms (see the spotbugs
63+
# lane for the rationale; this job releases nothing).
64+
# Skipped entirely when the scope step kept no modules (e.g. a docs-only
65+
# PR): every module would carry the skip properties, so the compile
66+
# output would be unused. The gate below relaxes on the same condition.
67+
if: env.LINT_KEPT != '0'
5168
run: |
52-
mvn -T 2C -f ./ddk-parent/pom.xml --batch-mode --fail-never \
69+
mvn -T 2C -f ./ddk-parent/pom.xml ${LINT_SCOPE_ARGS:-} --batch-mode --fail-never \
5370
compile \
5471
pmd:pmd checkstyle:checkstyle \
5572
-Dformat=net.sourceforge.pmd.renderers.SarifRenderer \
56-
-Dcheckstyle.output.format=sarif
73+
-Dcheckstyle.output.format=sarif \
74+
-Djgit.dirtyWorkingTree=ignore
5775
5876
- name: CPD report (separate invocation — no SARIF support)
5977
# CPD has no SARIF renderer; emits cpd.xml only. Run standalone so the
@@ -63,8 +81,11 @@ jobs:
6381
# compile pass.
6482
# NOTE: the CPD token threshold is governed by pmd.cpd.min in
6583
# ddk-parent/pom.xml.
84+
# No jgit flag needed: a direct goal invocation runs no lifecycle, so the
85+
# build-qualifier's dirty-tree check never executes here.
86+
if: env.LINT_KEPT != '0'
6687
run: |
67-
mvn -T 2C -f ./ddk-parent/pom.xml --batch-mode --fail-never \
88+
mvn -T 2C -f ./ddk-parent/pom.xml ${LINT_SCOPE_ARGS:-} --batch-mode --fail-never \
6889
pmd:cpd-check
6990
7091
- name: Merge per-module SARIFs (PMD + Checkstyle)
@@ -101,8 +122,17 @@ jobs:
101122
# merge() only writes its output when it found at least one valid input,
102123
# so a missing merged file means that analyzer silently died (e.g. a
103124
# plugin bump broke a renderer flag) — never a clean pass.
125+
# Exception: the scope step skip-injected every module (no reactor module
126+
# changed), where zero reports is the expected state. Each scanned
127+
# source-bearing module must additionally have produced its own PMD
128+
# SARIF, cpd.xml, and Checkstyle SARIF, so a partially-dead scoped
129+
# build cannot hide either.
104130
run: |
105131
set -eu
132+
if [ "${LINT_KEPT:-}" = "0" ]; then
133+
echo "All modules skip-injected (no reactor module changed) — nothing to lint."
134+
exit 0
135+
fi
106136
# Every reactor bundle with sources must have produced its own report: with
107137
# --fail-never a module whose analysis died would otherwise hide behind a
108138
# sibling's clean report.
@@ -134,7 +164,7 @@ jobs:
134164
sarif_total=$(jq '[.runs[].results[]?] | length' \
135165
.sarif-merged/pmd.sarif .sarif-merged/checkstyle.sarif 2>/dev/null \
136166
| awk '{s+=$1} END {print s+0}')
137-
cpd_total=$(find . -name 'cpd.xml' -path '*/target/*' -exec grep -c '<duplication ' {} + 2>/dev/null \
167+
cpd_total=$(find . -name 'cpd.xml' -path '*/target/*' -exec grep -cH '<duplication ' {} + 2>/dev/null \
138168
| awk -F: '{s+=$2} END {print s+0}')
139169
echo "PMD/Checkstyle SARIF violations: $sarif_total"
140170
echo "CPD duplications: $cpd_total"
@@ -144,7 +174,9 @@ jobs:
144174
fi
145175
146176
- name: Upload PMD/Checkstyle SARIF to Code Scanning
147-
if: always()
177+
# Skip when nothing was scanned (e.g. a docs-only PR -> all modules skipped):
178+
# an empty .sarif-merged would otherwise fail upload-sarif ("No SARIF files").
179+
if: ${{ always() && hashFiles('.sarif-merged/pmd.sarif', '.sarif-merged/checkstyle.sarif') != '' }}
148180
# Annotation-only, never the gate: a fork PR gets a read-only token and
149181
# upload-sarif 403s, which must not red an otherwise-clean lane.
150182
continue-on-error: true
@@ -184,7 +216,7 @@ jobs:
184216
# changed modules and their upstream deps build at all (skip-injected deps
185217
# compile for the aux-classpath but are not analysed). A build/config change ->
186218
# full scan, full reactor. pull_request only; master/snapshot run a full scan.
187-
run: bash .github/scripts/compute-spotbugs-skip.sh "${{ github.event.pull_request.base.sha }}"
219+
run: bash .github/scripts/compute-analysis-skip.sh "${{ github.event.pull_request.base.sha }}" spotbugs
188220

189221
- name: SpotBugs report (SARIF)
190222
# sarifOutput=true emits spotbugsSarif.json (also writes spotbugsXml.xml).

0 commit comments

Comments
 (0)