Skip to content

ci: in-process SpotBugs + changed-modules-only reactor in the spotbugs lane #1456

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
joaodinissf wants to merge 4 commits into
base: master
Choose a base branch
from
+525 −9
Draft

ci: in-process SpotBugs + changed-modules-only reactor in the spotbugs lane #1456

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
91239aa
ci: fast static analysis with early-fail lint + inline SARIF
joaodinissf May 30, 2026
8ce709a
ci: scope SpotBugs to the PR's changed modules (per-module skip)
joaodinissf May 31, 2026
e187c63
ci: run SpotBugs in-process in the verify spotbugs lane
joaodinissf Jul 10, 2026
6fa2d0e
ci: build only changed modules + upstream deps in the spotbugs lane
joaodinissf Jul 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
124 changes: 124 additions & 0 deletions .github/scripts/compute-spotbugs-skip.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
#!/usr/bin/env bash
#
# Scope SpotBugs to a pull request's changed modules.
#
# Default is RUN (analyze). On a PR this injects <spotbugs.skip>true</spotbugs.skip>
# into every UNCHANGED reactor module's pom, so spotbugs-maven-plugin skips the goal —
# and therefore the per-module JVM fork (SpotBugsMojo gates on `skip` before forking) —
# for those modules. The full-reactor compile is left intact (a changed module is still
# analysed with its complete aux-classpath). Master/snapshot builds run a full scan;
# this script is invoked on pull_request only.
#
# Why this and not -Dspotbugs.onlyAnalyze: onlyAnalyze is one clean flag, but SpotBugs
# applies its class screener too late (after the per-module fork + class scan), so it
# only trimmed ~17% of the goal vs ~88% for this per-module skip (measured on this
# reactor). A small upstream SpotBugs early-exit (skip the run when no application class
# matches the screener) would make onlyAnalyze competitive; if that ever lands, switch
# to onlyAnalyze and delete this script (tracked in #1455 / spotbugs/spotbugs#3796).
#
# On top of the skips, the changed reactor modules are exported as SPOTBUGS_SCOPE_ARGS
# ("-pl <changed> -am") so the lane builds only those modules plus their upstream
# dependencies instead of the full reactor. The -am-pulled unchanged dependencies still
# carry the injected skip: they compile (complete aux-classpath) but are not analysed.
#
# Run from the repository root. Usage: compute-spotbugs-skip.sh <base-sha>
set -euo pipefail
base="${1:?base sha required}"

changed=$(git diff --name-only --diff-filter=ACMR "${base}...HEAD")

# 1) A change to shared build/config can affect any module -> full scan (skip nothing).
# Fail safe: the worst case here is "analyse everything", never "analyse nothing".
while IFS= read -r f; do
[ -n "$f" ] || continue
case "$f" in
pom.xml | ddk-parent/* | .mvn/* | *.target | .github/* | *[Ss]pot[Bb]ugs*[Ee]xclude*)
# KEPT must be set on every path: in a workflow `if:` an unset env var is
# null, which coerces to 0 and compares EQUAL to '0' — wrongly skipping
# the build. "all" marks the full-scan case (only "0" relaxes the gate).
if [ -n "${GITHUB_ENV:-}" ]; then
echo "SPOTBUGS_KEPT=all" >> "$GITHUB_ENV"
fi
echo "Build/config change ($f) -> full SpotBugs scan (no skips)."
exit 0
;;
esac
done <<EOF
${changed}
EOF

# 2) Changed top-level module directories (the reactor module == top-level dir here).
# `|| true`: a PR touching only root files (e.g. README.md) has no '/' paths;
# grep's no-match exit would otherwise kill the script under pipefail.
changed_mods=$(printf '%s\n' "${changed}" | { grep '/' || true; } | cut -d/ -f1 | sort -u)

# 3) Reactor module dirs from ddk-parent's <modules> (strip the leading ../).
# ddk-parent is NOT in its own <modules>, so it can never be skip-injected — which
# is what prevents an accidental inherited (global) skip.
module_dirs=$(grep -oE '<module>\.\./[^<]+</module>' ddk-parent/pom.xml \
| sed -E 's#.*\.\./([^<]+)</module>#\1#')

# 4) Idempotently inject the skip property; handle poms with and without <properties>.
# sed -i.bak + rm is portable across GNU (CI) and BSD (local) sed.
inject_skip() {
local pom="$1/pom.xml"
[ -f "$pom" ] || return 0
if grep -q '<spotbugs\.skip>' "$pom"; then return 0; fi
if grep -q '<properties>' "$pom"; then
sed -i.bak 's#<properties>#<properties>\n <spotbugs.skip>true</spotbugs.skip>#' "$pom"
else
sed -i.bak 's#</project># <properties>\n <spotbugs.skip>true</spotbugs.skip>\n </properties>\n</project>#' "$pom"
fi
rm -f "$pom.bak"
}

# 5) Skip every reactor module that was not touched by this PR. Kept modules with a
# bundle MANIFEST are expected to produce an analysis report — the gate checks this
# so a swallowed resolution/compile failure can never pass as "nothing to scan".
kept=0
skipped=0
kept_pl=""
expect_reports=""
while IFS= read -r mod; do
[ -n "$mod" ] || continue
if printf '%s\n' "${changed_mods}" | grep -qx "$mod"; then
kept=$((kept + 1))
kept_pl="${kept_pl:+${kept_pl},}../${mod}"
# Only bundles with sources reliably emit a report (a source-less bundle,
# e.g. pure branding, has nothing for the analyzer to write a SARIF about).
if [ -f "${mod}/META-INF/MANIFEST.MF" ] && [ -d "${mod}/src" ]; then
expect_reports="${expect_reports:+${expect_reports} }${mod}"
fi
else
inject_skip "$mod"
skipped=$((skipped + 1))
fi
done <<EOF
${module_dirs}
EOF

# The gate's presence check needs to distinguish "all modules skip-injected"
# (zero reports is the expected state) from "the analysis silently died".
if [ -n "${GITHUB_ENV:-}" ]; then
echo "SPOTBUGS_KEPT=${kept}" >> "$GITHUB_ENV"
fi

# 6) Scope the reactor to the changed modules + their upstream dependencies. ddk-target
# is always kept in the -pl list: the target-definition artifact is referenced by
# target-platform-configuration, not by any MANIFEST, so -am never pulls it — without
# it in the reactor Tycho falls back to a local-repository copy, which fails on a
# cold cache and can silently resolve a stale target definition on a warm one.
# With no changed reactor module (e.g. a docs-only PR) no scope args are exported;
# the workflow skips the Maven step entirely on SPOTBUGS_KEPT=0.
if [ "$kept" -gt 0 ] && [ -n "${GITHUB_ENV:-}" ]; then
echo "SPOTBUGS_SCOPE_ARGS=-pl ../ddk-target,${kept_pl} -am" >> "$GITHUB_ENV"
echo "SPOTBUGS_EXPECT_REPORTS=${expect_reports}" >> "$GITHUB_ENV"
fi

echo "SpotBugs scope: scanning ${kept} changed module(s), skipping ${skipped} unchanged."
echo "Changed modules: ${changed_mods:-<none>}"
if [ -n "${kept_pl}" ]; then
echo "Reactor scope args: -pl ../ddk-target,${kept_pl} -am"
else
echo "Reactor scope args: <full reactor>"
fi
237 changes: 228 additions & 9 deletions .github/workflows/verify.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,30 @@
name: verify
on:
pull_request:

# Code Scanning needs write access to upload SARIF results for inline annotations.
permissions:
contents: read
security-events: write

jobs:
pmd:
# Fast, early-fail lint lane: PMD + Checkstyle (+ CPD). Turns red in a few
# minutes on any violation, independent of the long build below, so a stray
# PMD/Checkstyle issue is reported immediately rather than after `verify`.
#
# `compile` is in the same invocation as the analysis goals: PMD's
# type-resolving rules (e.g. InvalidLogMessageFormat on the SLF4J
# trailing-Throwable idiom) need Tycho's aux-classpath, which a fresh `mvn`
# does not inherit from a prior step's target/classes.
#
# `--fail-never` lets every module produce its report (no Maven cascade-skip),
# so the uploaded SARIF — and therefore the inline annotations — are complete.
# The trade-off: --fail-never suppresses even compile and target-resolution
# failures (mvn exits 0 on a broken build), so the gate pairs the jq count
# with a presence check — zero valid analyzer inputs fails the lane rather
# than reading as a clean pass. Compilation itself is independently gated by
# maven-verify.
lint:
runs-on: ubuntu-24.04
steps:
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
Expand All @@ -12,26 +34,225 @@ jobs:
java-version: '21'
- name: Set up Workspace Environment Variable
run: echo "WORKSPACE=${{ github.workspace }}" >> $GITHUB_ENV
- name: PMD Check
run: mvn pmd:pmd pmd:cpd pmd:check pmd:cpd-check -f ./ddk-parent/pom.xml --batch-mode --fail-at-end
checkstyle:
- name: Restore Maven dependency cache
# Restore-only, mirroring snapshot.yml's producer cache exactly (path and
# key are hashed into the cache version — see the maven-verify step).
uses: actions/cache/restore@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0
with:
path: ~/.m2/repository
key: ${{ runner.os }}-maven-publish-${{ hashFiles('**/pom.xml', '**/*.target') }}
restore-keys: ${{ runner.os }}-maven-publish-

- name: PMD + Checkstyle reports (SARIF)
# PMD: SarifRenderer FQCN — emits pmd.sarif.json AND keeps pmd.xml.
# Checkstyle: output.format=sarif — SARIF content in checkstyle-result.xml.
# CPD is excluded here: the global -Dformat flag uses PMD's Renderer
# hierarchy and would ClassCastException CPD's CPDReportRenderer.
run: |
mvn -T 2C -f ./ddk-parent/pom.xml --batch-mode --fail-never \
compile \
pmd:pmd checkstyle:checkstyle \
-Dformat=net.sourceforge.pmd.renderers.SarifRenderer \
-Dcheckstyle.output.format=sarif

- name: CPD report (separate invocation — no SARIF support)
# CPD has no SARIF renderer; emits cpd.xml only. Run standalone so the
# PMD -Dformat flag isn't in scope.
# NOTE: project CPD token threshold is currently very high (issue #1339),
# which effectively disables detection; re-tune once #1339 lands.
run: |
mvn -T 2C -f ./ddk-parent/pom.xml --batch-mode --fail-never \
compile \
pmd:cpd-check

- name: Merge per-module SARIFs (PMD + Checkstyle)
if: always()
# Code Scanning accepts one run per category per upload; each analyzer
# writes one SARIF per module, so concatenate each analyzer's results
# into a single run under .sarif-merged/.
run: |
mkdir -p .sarif-merged
# Merge per-module SARIFs into one run. Filter to JSON-parseable files:
# the ddk-parent aggregator writes a plain-XML checkstyle-result.xml that
# would break jq, and modules with no findings may emit non-SARIF stubs.
merge() { # $1 = find-glob, $2 = output
local f valid=()
while IFS= read -r f; do
jq -e . "$f" >/dev/null 2>&1 && valid+=("$f")
done < <(find . -path "$1")
if [ ${#valid[@]} -gt 0 ]; then
# del(.ruleIndex): each result's ruleIndex points into its OWN run's
# rules array, but the merge keeps only the first run's tool — Code
# Scanning must resolve rules by ruleId string instead.
jq -s '{
"$schema": .[0]."$schema", version: .[0].version,
runs: [{ tool: .[0].runs[0].tool,
results: [.[].runs[].results[]? | del(.ruleIndex)],
invocations: [.[].runs[].invocations[]?] }]
}' "${valid[@]}" > "$2"
fi
}
merge '*/target/pmd.sarif.json' .sarif-merged/pmd.sarif
merge '*/target/checkstyle-result.xml' .sarif-merged/checkstyle.sarif

- name: Gate on PMD / CPD / Checkstyle violations
# merge() only writes its output when it found at least one valid input,
# so a missing merged file means that analyzer silently died (e.g. a
# plugin bump broke a renderer flag) — never a clean pass.
run: |
set -eu
for f in .sarif-merged/pmd.sarif .sarif-merged/checkstyle.sarif; do
if [ ! -s "$f" ]; then
echo "::error::No valid SARIF input produced for ${f##*/} — the analysis silently failed."
exit 1
fi
done
if [ "$(find . -name 'cpd.xml' -path '*/target/*' | wc -l)" -eq 0 ]; then
echo "::error::No cpd.xml produced — CPD silently failed."
exit 1
fi
sarif_total=$(jq '[.runs[].results[]?] | length' \
.sarif-merged/pmd.sarif .sarif-merged/checkstyle.sarif 2>/dev/null \
| awk '{s+=$1} END {print s+0}')
cpd_total=$(find . -name 'cpd.xml' -path '*/target/*' -exec grep -c '<duplication ' {} + 2>/dev/null \
| awk -F: '{s+=$2} END {print s+0}')
echo "PMD/Checkstyle SARIF violations: $sarif_total"
echo "CPD duplications: $cpd_total"
if [ "$sarif_total" != "0" ] || [ "$cpd_total" != "0" ]; then
echo "::error::Static analysis found violations (PMD/CPD/Checkstyle)."
exit 1
fi

- name: Upload PMD/Checkstyle SARIF to Code Scanning
if: always()
uses: github/codeql-action/upload-sarif@7fd177fa680c9881b53cdab4d346d32574c9f7f4 # v3.35.4
with:
sarif_file: .sarif-merged
category: lint

# SpotBugs is the slow critical-path analysis (the experiments' durable
# finding), so it runs in its own parallel lane and never delays `lint`.
spotbugs:
runs-on: ubuntu-24.04
env:
MAVEN_OPTS: -Xmx4g
steps:
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
with:
fetch-depth: 0 # need the PR base commit to diff the changed modules
- uses: actions/setup-java@0f481fcb613427c0f801b606911222b5b6f3083a # v5
with:
distribution: 'temurin'
java-version: '21'
- name: Set up Workspace Environment Variable
run: echo "WORKSPACE=${{ github.workspace }}" >> $GITHUB_ENV
- name: Checkstyle Check
run: mvn checkstyle:checkstyle checkstyle:check -f ./ddk-parent/pom.xml --batch-mode --fail-at-end
- name: Restore Maven dependency cache
# Restore-only, mirroring snapshot.yml's producer cache exactly (path and
# key are hashed into the cache version — see the maven-verify step).
uses: actions/cache/restore@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0
with:
path: ~/.m2/repository
key: ${{ runner.os }}-maven-publish-${{ hashFiles('**/pom.xml', '**/*.target') }}
restore-keys: ${{ runner.os }}-maven-publish-

- name: Scope SpotBugs to the PR's changed modules
# Injects <spotbugs.skip>true> into unchanged module poms so their analysis is
# skipped, and exports SPOTBUGS_SCOPE_ARGS (-pl <changed> -am) so only the
# changed modules and their upstream deps build at all (skip-injected deps
# compile for the aux-classpath but are not analysed). A build/config change ->
# full scan, full reactor. pull_request only; master/snapshot run a full scan.
run: bash .github/scripts/compute-spotbugs-skip.sh "${{ github.event.pull_request.base.sha }}"

- name: SpotBugs report (SARIF)
# sarifOutput=true emits spotbugsSarif.json (also writes spotbugsXml.xml).
# jgit.dirtyWorkingTree=ignore: the scope step intentionally edits poms, so the
# working tree is dirty here; this job releases nothing, so we tell Tycho's jgit
# build-qualifier to use the last commit's timestamp instead of failing (the
# repo keeps jgit.dirtyWorkingTree=error for maven-verify / releases).
# spotbugs.fork=false analyses in the Maven JVM instead of forking a fresh 2 GB
# JVM per module (59 forks); the shared heap is governed by MAVEN_OPTS above
# (the plugin's maxHeap applies to forks only).
# Skipped entirely when the scope step kept no modules (e.g. a docs-only
# PR): every module would carry spotbugs.skip, so the compile output
# would be unused. The gate below relaxes on the same condition.
if: env.SPOTBUGS_KEPT != '0'
run: |
mvn -T 2C -f ./ddk-parent/pom.xml ${SPOTBUGS_SCOPE_ARGS:-} --batch-mode --fail-never \
compile \
spotbugs:spotbugs \
-Dspotbugs.sarifOutput=true \
-Dspotbugs.fork=false \
-Djgit.dirtyWorkingTree=ignore

- name: Merge per-module SpotBugs SARIFs
if: always()
run: |
mkdir -p .sarif-merged
valid=()
while IFS= read -r f; do
jq -e . "$f" >/dev/null 2>&1 && valid+=("$f")
done < <(find . -path '*/target/spotbugsSarif.json')
if [ ${#valid[@]} -gt 0 ]; then
# del(.ruleIndex): see the lint merge — indexes are per-run, the
# merged tool keeps only the first run's rules.
jq -s '{
"$schema": .[0]."$schema", version: .[0].version,
runs: [{ tool: .[0].runs[0].tool,
results: [.[].runs[].results[]? | del(.ruleIndex)],
invocations: [.[].runs[].invocations[]?] }]
}' "${valid[@]}" > .sarif-merged/spotbugs.sarif
fi

- name: Gate on SpotBugs violations
# A missing merged SARIF means the analysis silently died (--fail-never
# suppresses even compile/resolution failures) — never a clean pass.
# Exception: the scope step skip-injected every module (no reactor module
# changed, e.g. a docs-only PR), where zero reports is the expected state.
# Each scanned source-bearing module must additionally have produced its
# own SARIF, so a partially-dead scoped build cannot hide either.
run: |
set -eu
if [ "${SPOTBUGS_KEPT:-}" = "0" ]; then
echo "All modules skip-injected (no reactor module changed) — nothing to scan."
exit 0
fi
for mod in ${SPOTBUGS_EXPECT_REPORTS:-}; do
if [ ! -s "${mod}/target/spotbugsSarif.json" ]; then
echo "::error::${mod} was scanned but produced no SpotBugs SARIF — a build failure was swallowed by --fail-never."
exit 1
fi
done
if [ ! -s .sarif-merged/spotbugs.sarif ]; then
echo "::error::No SpotBugs SARIF produced — the analysis silently failed."
exit 1
fi
sb_total=$(jq '[.runs[].results[]?] | length' .sarif-merged/spotbugs.sarif 2>/dev/null || echo 0)
echo "SpotBugs SARIF violations: $sb_total"
if [ "$sb_total" != "0" ]; then
echo "::error::SpotBugs found violations."
exit 1
fi

- name: Upload SpotBugs SARIF to Code Scanning
# Skip when nothing was scanned (e.g. a docs-only PR -> all modules skipped):
# an empty .sarif-merged would otherwise fail upload-sarif ("No SARIF files").
if: ${{ always() && hashFiles('.sarif-merged/spotbugs.sarif') != '' }}
uses: github/codeql-action/upload-sarif@7fd177fa680c9881b53cdab4d346d32574c9f7f4 # v3.35.4
with:
sarif_file: .sarif-merged
category: spotbugs

line-endings:
runs-on: ubuntu-24.04
steps:
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
- name: Check LF line endings
run: bash .github/scripts/check-line-endings.sh

# Build + tests only. Static analysis now lives in the `lint` and `spotbugs`
# jobs, so the redundant checkstyle/pmd/spotbugs goals are dropped from here —
# this is the wall-clock long pole and no longer re-runs analysis.
# No `-T 2C`: tests are not known to pass reliably under reactor parallelism.
maven-verify:
runs-on: ubuntu-24.04
steps:
Expand All @@ -58,9 +279,7 @@ jobs:
key: ${{ runner.os }}-maven-publish-${{ hashFiles('**/pom.xml', '**/*.target') }}
restore-keys: ${{ runner.os }}-maven-publish-
- name: Build with Maven within a virtual X Server Environment
# Run pmd:pmd and pmd:cpd first to generate reports for all modules, then run pmd:check and pmd:cpd-check
# This ensures all violations are collected and reported before the build fails
run: xvfb-run mvn clean verify checkstyle:check pmd:pmd pmd:cpd pmd:check pmd:cpd-check spotbugs:check -f ./ddk-parent/pom.xml --batch-mode --fail-at-end
run: xvfb-run mvn clean verify -f ./ddk-parent/pom.xml --batch-mode --fail-at-end
- name: Fail on missing surefire reports
if: always()
run: bash .github/scripts/check-surefire-reports.sh
Expand Down
Loading
Loading