Skip to content

Commit 37e4b7d

Browse files
joaodinissfclaude
andcommitted
ci: scope SpotBugs to the PR's changed modules (per-module skip)
SpotBugs' per-module analysis is the spotbugs job's long pole. A PR only needs its changed modules scanned, so a pre-step injects <spotbugs.skip>true> into every unchanged reactor module's pom — the plugin then skips the goal, and the per-module JVM fork, for them. The full-reactor compile is kept (a changed module keeps its complete aux-classpath); a build/config change falls back to a full scan. pull_request only — master/snapshot run a full scan. -Dspotbugs.onlyAnalyze was the cleaner-looking alternative but screens too late (after the per-module fork), ~17% vs ~88% measured; the script header documents the migration if an upstream SpotBugs early-exit ever lands. - .github/scripts/compute-spotbugs-skip.sh: diff -> changed modules -> inject skip into the unchanged ones (idempotent; build/config change -> full scan). - verify.yml spotbugs job: fetch-depth 0 + a scope step before compile; -Djgit.dirtyWorkingTree=ignore because the scope step dirties poms on purpose and this job releases nothing (releases/maven-verify keep =error); SARIF upload guarded so an empty scan set (no module scanned) doesn't fail the upload. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 3e46952 commit 37e4b7d

2 files changed

Lines changed: 129 additions & 2 deletions

File tree

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
#!/usr/bin/env bash
2+
#
3+
# Scope SpotBugs to a pull request's changed modules.
4+
#
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.
11+
#
12+
# Accepted trade-off: only changed modules are analysed. A change whose effect shows
13+
# up as a finding in an unchanged dependent module surfaces on the master full scan.
14+
#
15+
# Why this and not -Dspotbugs.onlyAnalyze: onlyAnalyze is one clean flag, but SpotBugs
16+
# applies its class screener too late (after the per-module fork + class scan), so it
17+
# only trimmed ~17% of the goal vs ~88% for this per-module skip (measured on this
18+
# reactor). A small upstream SpotBugs early-exit (skip the run when no application class
19+
# matches the screener) would make onlyAnalyze competitive; if that ever lands, switch
20+
# to onlyAnalyze and delete this script.
21+
#
22+
# Run from the repository root. Usage: compute-spotbugs-skip.sh <base-sha>
23+
set -euo pipefail
24+
base="${1:?base sha required}"
25+
26+
# --no-renames reports a move as delete + add, so both the old and the new module
27+
# count as changed; D keeps deletion-only changes (e.g. a removed ruleset) visible.
28+
changed=$(git diff --name-only --no-renames --diff-filter=ACMD "${base}...HEAD")
29+
30+
# 1) A change to shared build/config can affect any module -> full scan (skip nothing).
31+
# ddk-configuration holds the analyzers' rulesets and filters (e.g. the SpotBugs
32+
# exclusion-filter), so a change there must re-scan everything, not skip silently.
33+
# ddk-target defines the target platform every module resolves against.
34+
# Fail safe: the worst case here is "analyse everything", never "analyse nothing".
35+
while IFS= read -r f; do
36+
[ -n "$f" ] || continue
37+
case "$f" in
38+
pom.xml | ddk-parent/* | .mvn/* | *.target | ddk-target/* | .github/* | ddk-configuration/* | *[Ss]pot[Bb]ugs*[Ee]xclude*)
39+
echo "Build/config change ($f) -> full SpotBugs scan (no skips)."
40+
exit 0
41+
;;
42+
esac
43+
done <<EOF
44+
${changed}
45+
EOF
46+
47+
# 2) Changed top-level module directories (the reactor module == top-level dir here).
48+
# `|| true`: a PR touching only root files (e.g. README.md) has no '/' paths;
49+
# grep's no-match exit would otherwise kill the script under pipefail.
50+
changed_mods=$(printf '%s\n' "${changed}" | { grep '/' || true; } | cut -d/ -f1 | sort -u)
51+
52+
# 3) Reactor module dirs from ddk-parent's <modules> (strip the leading ../).
53+
# ddk-parent is NOT in its own <modules>, so it can never be skip-injected — which
54+
# is what prevents an accidental inherited (global) skip.
55+
module_dirs=$(grep -oE '<module>\.\./[^<]+</module>' ddk-parent/pom.xml \
56+
| sed -E 's#.*\.\./([^<]+)</module>#\1#')
57+
58+
# 4) Idempotently inject the skip property; handle poms with and without <properties>.
59+
# sed -i.bak + rm is portable across GNU (CI) and BSD (local) sed.
60+
inject_skip() {
61+
local pom="$1/pom.xml"
62+
[ -f "$pom" ] || return 0
63+
if grep -q '<spotbugs\.skip>' "$pom"; then return 0; fi
64+
if grep -q '<properties>' "$pom"; then
65+
sed -i.bak 's#<properties>#<properties>\n <spotbugs.skip>true</spotbugs.skip>#' "$pom"
66+
else
67+
sed -i.bak 's#</project># <properties>\n <spotbugs.skip>true</spotbugs.skip>\n </properties>\n</project>#' "$pom"
68+
fi
69+
rm -f "$pom.bak"
70+
}
71+
72+
# 5) Skip every reactor module that was not touched by this PR. Kept bundles with
73+
# sources are expected to produce a SARIF; the gate checks each one.
74+
kept=0
75+
skipped=0
76+
expect_reports=""
77+
while IFS= read -r mod; do
78+
[ -n "$mod" ] || continue
79+
if printf '%s\n' "${changed_mods}" | grep -qx "$mod"; then
80+
kept=$((kept + 1))
81+
if [ -f "${mod}/META-INF/MANIFEST.MF" ] && [ -d "${mod}/src" ]; then
82+
expect_reports="${expect_reports:+${expect_reports} }${mod}"
83+
fi
84+
else
85+
inject_skip "$mod"
86+
skipped=$((skipped + 1))
87+
fi
88+
done <<EOF
89+
${module_dirs}
90+
EOF
91+
92+
# The gate's presence check needs to distinguish "all modules skip-injected"
93+
# (zero reports is the expected state) from "the analysis silently died".
94+
# Only source-less modules changed (feature / target / repository) counts as
95+
# nothing to scan: SpotBugs cannot write a report for a bundle without classes.
96+
if [ -z "$expect_reports" ]; then
97+
kept=0
98+
fi
99+
if [ -n "${GITHUB_ENV:-}" ]; then
100+
echo "SPOTBUGS_KEPT=${kept}" >> "$GITHUB_ENV"
101+
echo "SPOTBUGS_EXPECT_REPORTS=${expect_reports}" >> "$GITHUB_ENV"
102+
fi
103+
104+
echo "SpotBugs scope: scanning ${kept} changed module(s), skipping ${skipped} unchanged."
105+
echo "Changed modules: ${changed_mods:-<none>}"

‎.github/workflows/verify.yml‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,8 @@ jobs:
159159
MAVEN_OPTS: -Xmx4g
160160
steps:
161161
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
162+
with:
163+
fetch-depth: 0 # need the PR base commit to diff the changed modules
162164
- uses: actions/setup-java@de7274f081f381c8f8158605e0321c36c376e2e6 # v6.0.1
163165
with:
164166
distribution: 'temurin'
@@ -174,13 +176,25 @@ jobs:
174176
key: ${{ runner.os }}-maven-publish-${{ hashFiles('**/pom.xml', '**/*.target') }}
175177
restore-keys: ${{ runner.os }}-maven-publish-
176178

179+
- name: Scope SpotBugs to the PR's changed modules
180+
# Injects <spotbugs.skip>true> into unchanged module poms so the per-module
181+
# SpotBugs fork is skipped for them (the lever that actually scopes the cost).
182+
# Full compile is preserved (correct aux-classpath); a build/config change ->
183+
# full scan. pull_request only; master/snapshot run a full scan.
184+
run: bash .github/scripts/compute-spotbugs-skip.sh "${{ github.event.pull_request.base.sha }}"
185+
177186
- name: SpotBugs report (SARIF)
178187
# sarifOutput=true emits spotbugsSarif.json (also writes spotbugsXml.xml).
188+
# jgit.dirtyWorkingTree=ignore: the scope step intentionally edits poms, so the
189+
# working tree is dirty here; this job releases nothing, so we tell Tycho's jgit
190+
# build-qualifier to use the last commit's timestamp instead of failing (the
191+
# repo keeps jgit.dirtyWorkingTree=error for maven-verify / releases).
179192
run: |
180193
mvn -T 2C -f ./ddk-parent/pom.xml --batch-mode --fail-never \
181194
compile \
182195
spotbugs:spotbugs \
183-
-Dspotbugs.sarifOutput=true
196+
-Dspotbugs.sarifOutput=true \
197+
-Djgit.dirtyWorkingTree=ignore
184198
185199
- name: Merge per-module SpotBugs SARIFs
186200
if: always()
@@ -204,8 +218,14 @@ jobs:
204218
- name: Gate on SpotBugs violations
205219
# A missing merged SARIF means the analysis silently died (--fail-never
206220
# suppresses even compile/resolution failures) — never a clean pass.
221+
# Exception: the scope step skip-injected every module (no reactor module
222+
# changed, e.g. a docs-only PR), where zero reports is the expected state.
207223
run: |
208224
set -eu
225+
if [ "${SPOTBUGS_KEPT:-}" = "0" ]; then
226+
echo "All modules skip-injected (no reactor module changed) — nothing to scan."
227+
exit 0
228+
fi
209229
# Every reactor bundle with sources must have produced its own report: with
210230
# --fail-never a module whose analysis died would otherwise hide behind a
211231
# sibling's clean report.
@@ -230,7 +250,9 @@ jobs:
230250
fi
231251
232252
- name: Upload SpotBugs SARIF to Code Scanning
233-
if: always()
253+
# Skip when nothing was scanned (e.g. a docs-only PR -> all modules skipped):
254+
# an empty .sarif-merged would otherwise fail upload-sarif ("No SARIF files").
255+
if: ${{ always() && hashFiles('.sarif-merged/spotbugs.sarif') != '' }}
234256
# Annotation-only, never the gate: a fork PR gets a read-only token and
235257
# upload-sarif 403s, which must not red an otherwise-clean lane.
236258
continue-on-error: true

0 commit comments

Comments
 (0)