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>

Author: joaodinissf

.github/scripts/compute-spotbugs-skip.sh

@@ -0,0 +1,88 @@
+#!/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.
+#
+# 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).
+#    ddk-configuration holds the analyzers' rulesets and filters (e.g. the SpotBugs
+#    exclusion-filter), so a change there must re-scan everything, not skip silently.
+#    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/* | ddk-configuration/* | *[Ss]pot[Bb]ugs*[Ee]xclude*)
+      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=0
+skipped=0
+while IFS= read -r mod; do
+  [ -n "$mod" ] || continue
+  if printf '%s\n' "${changed_mods}" | grep -qx "$mod"; then
+    kept=$((kept + 1))
+  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
+
+echo "SpotBugs scope: scanning ${kept} changed module(s), skipping ${skipped} unchanged."
+echo "Changed modules: ${changed_mods:-<none>}"

.github/workflows/verify.yml

@@ -141,6 +141,8 @@ jobs:
       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'
@@ -156,13 +158,25 @@ jobs:
           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 the per-module
+        # SpotBugs fork is skipped for them (the lever that actually scopes the cost).
+        # Full compile is preserved (correct aux-classpath); a build/config change ->
+        # full scan. 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).
         run: |
           mvn -T 2C -f ./ddk-parent/pom.xml --batch-mode --fail-never \
             compile \
             spotbugs:spotbugs \
-            -Dspotbugs.sarifOutput=true
+            -Dspotbugs.sarifOutput=true \
+            -Djgit.dirtyWorkingTree=ignore
 
       - name: Merge per-module SpotBugs SARIFs
         if: always()
@@ -186,8 +200,14 @@ jobs:
       - 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.
         run: |
           set -eu
+          if [ "${SPOTBUGS_KEPT:-}" = "0" ]; then
+            echo "All modules skip-injected (no reactor module changed) — nothing to scan."
+            exit 0
+          fi
           if [ ! -s .sarif-merged/spotbugs.sarif ]; then
             echo "::error::No SpotBugs SARIF produced — the analysis silently failed."
             exit 1
@@ -200,7 +220,9 @@ jobs:
           fi
 
       - name: Upload SpotBugs SARIF to Code Scanning
-        if: always()
+        # 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') != '' }}
         # Annotation-only, never the gate: a fork PR gets a read-only token and
         # upload-sarif 403s, which must not red an otherwise-clean lane.
         continue-on-error: true