ci: scope pull-request analysis and avoid redundant compilation

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

@@ -0,0 +1,171 @@
+#!/usr/bin/env bash
+#
+# Scope static analysis (SpotBugs, or PMD/CPD/Checkstyle) to a pull request's
+# changed modules.
+#
+# Default is RUN (analyze). On a PR this injects <TOOL.skip>true</TOOL.skip>
+# properties into every UNCHANGED reactor module's pom, so the analysis mojos
+# skip those modules — for SpotBugs that also skips the per-module JVM fork
+# (SpotBugsMojo gates on `skip` before forking). A changed module is still
+# analysed with its complete aux-classpath: the -am-pulled unchanged
+# dependencies compile but are not analysed. Master/snapshot builds run a full
+# scan; this script is invoked on pull_request only.
+#
+# Accepted trade-off: only changed modules are analysed. A change whose effect shows
+# up as a finding in an unchanged dependent module surfaces on the master full scan.
+#
+# 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 the spotbugs mode here (tracked in #1455 /
+# spotbugs/spotbugs#3796).
+#
+# On top of the skips, the changed reactor modules are exported as
+# SPOTBUGS_SCOPE_ARGS / LINT_SCOPE_ARGS ("-pl <changed> -am") so the lane builds
+# only those modules plus their upstream dependencies instead of the full reactor.
+# The lane's gate cross-checks <MODE>_KEPT / <MODE>_EXPECT_REPORTS so a build
+# failure swallowed by --fail-never can never pass as "nothing to scan".
+#
+# Run from the repository root.  Usage: compute-analysis-skip.sh <base-sha> <spotbugs|lint>
+set -euo pipefail
+base="${1:?base sha required}"
+mode="${2:?mode required: spotbugs|lint}"
+
+case "$mode" in
+  spotbugs) props="spotbugs.skip"; prefix="SPOTBUGS" ;;
+  lint)     props="pmd.skip cpd.skip checkstyle.skip"; prefix="LINT" ;;
+  *) echo "unknown mode: $mode" >&2; exit 2 ;;
+esac
+
+# --no-renames reports a move as delete + add, so both the old and the new module
+# count as changed; D keeps deletion-only changes (e.g. a removed ruleset) visible.
+changed=$(git diff --name-only --no-renames --diff-filter=ACMD "${base}...HEAD")
+
+# Reactor module dirs from ddk-parent's <modules> (strip the leading ../), and the
+# subset that bears sources. ddk-parent is NOT in its own <modules>, so it can never be
+# skip-injected — which prevents an accidental inherited (global) skip. Both lists are
+# computed up front because the full-scan early-exit below needs the source-bearing set
+# for its report-presence gate.
+module_dirs=$(grep -oE '<module>\.\./[^<]+</module>' ddk-parent/pom.xml \
+  | sed -E 's#.*\.\./([^<]+)</module>#\1#')
+all_source_modules=""
+while IFS= read -r mod; do
+  [ -n "$mod" ] || continue
+  if [ -f "${mod}/META-INF/MANIFEST.MF" ] && [ -d "${mod}/src" ]; then
+    all_source_modules="${all_source_modules:+${all_source_modules} }${mod}"
+  fi
+done <<EOF
+${module_dirs}
+EOF
+
+# 1) A change to shared build/config can affect any module -> full scan (skip nothing).
+#    ddk-configuration holds the analyzers' rulesets and filters, so it counts too.
+#    ddk-target defines the target platform every module resolves against.
+#    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 | ddk-target/* | .github/* | ddk-configuration/* | *[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).
+      # EXPECT_REPORTS lists every source-bearing module so the gate verifies a
+      # full scan analysed all of them (not just >=1) — a mojo death swallowed by
+      # --fail-never can't pass as long as one sibling reported.
+      if [ -n "${GITHUB_ENV:-}" ]; then
+        echo "${prefix}_KEPT=all" >> "$GITHUB_ENV"
+        echo "${prefix}_EXPECT_REPORTS=${all_source_modules}" >> "$GITHUB_ENV"
+      fi
+      echo "Build/config change ($f) -> full ${mode} 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) Idempotently inject the skip properties; 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" prop
+  [ -f "$pom" ] || return 0
+  for prop in $props; do
+    if grep -q "<${prop//./\\.}>" "$pom"; then continue; fi
+    if grep -q '<properties>' "$pom"; then
+      sed -i.bak "s#<properties>#<properties>\n    <${prop}>true</${prop}>#" "$pom"
+    else
+      sed -i.bak "s#</project>#  <properties>\n    <${prop}>true</${prop}>\n  </properties>\n</project>#" "$pom"
+    fi
+    rm -f "$pom.bak"
+  done
+}
+
+# 4) 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 PMD 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
+
+# 5) 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 analysable changed module (docs-only, or source-less-only) no scope args
+#    are exported; the workflow skips the lane's Maven step(s) entirely on <MODE>_KEPT=0.
+# The gate's presence check needs to distinguish "all modules skip-injected"
+# (zero reports is the expected state) from "the analysis silently died".
+# If the only changed modules are source-less (feature / target / repository — nothing
+# any analyzer can report), fold into the docs-only no-op: export KEPT=0 so the lane
+# skips the Maven step, gate, and upload instead of failing the merged-report presence
+# check on output that could never exist.
+if [ "$kept" -eq 0 ] || [ -z "$expect_reports" ]; then
+  effective_kept=0
+else
+  effective_kept=$kept
+fi
+if [ -n "${GITHUB_ENV:-}" ]; then
+  echo "${prefix}_KEPT=${effective_kept}" >> "$GITHUB_ENV"
+fi
+
+if [ "$effective_kept" -gt 0 ] && [ -n "${GITHUB_ENV:-}" ]; then
+  echo "${prefix}_SCOPE_ARGS=-pl ../ddk-target,${kept_pl} -am" >> "$GITHUB_ENV"
+  echo "${prefix}_EXPECT_REPORTS=${expect_reports}" >> "$GITHUB_ENV"
+fi
+
+echo "${mode} scope: scanning ${effective_kept} changed module(s), skipping ${skipped} unchanged."
+echo "Changed modules: ${changed_mods:-<none>}"
+if [ "$kept" -gt 0 ] && [ "$effective_kept" -eq 0 ]; then
+  echo "Only source-less modules changed (no analysable sources) -> no-op (${prefix}_KEPT=0)."
+fi
+if [ "$effective_kept" -gt 0 ]; then
+  echo "Reactor scope args: -pl ../ddk-target,${kept_pl} -am"
+else
+  echo "Reactor scope args: <full reactor>"
+fi

.github/workflows/verify.yml

@@ -36,6 +36,8 @@ jobs:
     runs-on: ubuntu-24.04
     steps:
       - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1  # v7.0.1
+        with:
+          fetch-depth: 0  # need the PR base commit to diff the changed modules
       - uses: actions/setup-java@de7274f081f381c8f8158605e0321c36c376e2e6  # v6.0.1
         with:
           distribution: 'temurin'
@@ -54,33 +56,58 @@ jobs:
       - name: Install XML report validator
         run: sudo apt-get update && sudo apt-get install --yes libxml2-utils
 
+      - name: Scope static analysis to the PR's changed modules
+        # Injects pmd/cpd/checkstyle skip properties into unchanged module poms and
+        # exports LINT_SCOPE_ARGS (-pl <changed> -am) so only the changed modules and
+        # their upstream deps build (skip-injected deps compile for PMD's type
+        # resolution but are not analysed). Build/config change -> full scan, full
+        # reactor. pull_request only; master/snapshot run a full scan.
+        run: bash .github/scripts/compute-analysis-skip.sh "${{ github.event.pull_request.base.sha }}" lint
+
       - 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.
+        # `compile` stays: PMD's type-resolving rules need Tycho's aux-classpath.
+        # jgit.dirtyWorkingTree=ignore: the scope step edits poms (see the spotbugs
+        # lane for the rationale; this job releases nothing).
+        # Skipped entirely when the scope step kept no modules (e.g. a docs-only
+        # PR): every module would carry the skip properties, so the compile
+        # output would be unused. The gate below relaxes on the same condition.
+        if: env.LINT_KEPT != '0'
         run: |
-          mvn -T 2C -f ./ddk-parent/pom.xml --batch-mode --fail-at-end \
+          mvn -T 2C -f ./ddk-parent/pom.xml ${LINT_SCOPE_ARGS:-} --batch-mode --fail-at-end \
             compile \
             pmd:pmd checkstyle:checkstyle \
             -Dformat=net.sourceforge.pmd.renderers.SarifRenderer \
-            -Dcheckstyle.output.format=sarif
+            -Dcheckstyle.output.format=sarif \
+            -Djgit.dirtyWorkingTree=ignore
 
       - 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.
+        # No `compile`: CPD is token-based over src/ and needs neither bytecode
+        # nor the target platform — cpd.xml is identical with and without a
+        # compile pass.
         # NOTE: the CPD token threshold is governed by pmd.cpd.min in
         # ddk-parent/pom.xml.
+        # No jgit flag needed: a direct goal invocation runs no lifecycle, so the
+        # build-qualifier's dirty-tree check never executes here.
+        if: env.LINT_KEPT != '0'
         run: |
-          mvn -T 2C -f ./ddk-parent/pom.xml --batch-mode --fail-at-end \
-            compile \
+          mvn -T 2C -f ./ddk-parent/pom.xml ${LINT_SCOPE_ARGS:-} --batch-mode --fail-at-end \
             pmd:cpd-check
 
       - name: Merge per-module SARIFs (PMD + Checkstyle)
         if: always()
         # Merge only expected module reports into one run per analyzer.
         run: |
           set -euo pipefail
+          if [ "${LINT_KEPT:-}" = "0" ]; then
+            echo "Scope contains no analysable modules — nothing to lint."
+            exit 0
+          fi
           source .github/scripts/sarif.sh
           source_modules=$(sarif_source_modules)
           merge_sarif pmd.sarif.json .sarif-merged/pmd.sarif "${LINT_EXPECT_REPORTS:-$source_modules}" PMD
@@ -90,6 +117,10 @@ jobs:
         # Require successful reports from every expected module before counting findings.
         run: |
           set -euo pipefail
+          if [ "${LINT_KEPT:-}" = "0" ]; then
+            echo "Scope contains no analysable modules — nothing to lint."
+            exit 0
+          fi
           source .github/scripts/sarif.sh
           source_modules=$(sarif_source_modules)
           cpd_reports=()
@@ -119,7 +150,9 @@ jobs:
           fi
 
       - name: Upload PMD/Checkstyle 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/pmd.sarif', '.sarif-merged/checkstyle.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
@@ -136,6 +169,8 @@ jobs:
       MAVEN_OPTS: -Xmx4g
     steps:
       - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1  # v7.0.1
+        with:
+          fetch-depth: 0  # need the PR base commit to diff the changed modules
       - uses: actions/setup-java@de7274f081f381c8f8158605e0321c36c376e2e6  # v6.0.1
         with:
           distribution: 'temurin'
@@ -151,18 +186,39 @@ 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 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-analysis-skip.sh "${{ github.event.pull_request.base.sha }}" spotbugs
+
       - 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).
+        # 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 --batch-mode --fail-at-end \
+          mvn -T 2C -f ./ddk-parent/pom.xml ${SPOTBUGS_SCOPE_ARGS:-} --batch-mode --fail-at-end \
             compile \
             spotbugs:spotbugs \
-            -Dspotbugs.sarifOutput=true
+            -Dspotbugs.sarifOutput=true \
+            -Djgit.dirtyWorkingTree=ignore
 
       - name: Merge per-module SpotBugs SARIFs
         if: always()
         run: |
           set -euo pipefail
+          if [ "${SPOTBUGS_KEPT:-}" = "0" ]; then
+            echo "Scope contains no analysable modules — nothing to scan."
+            exit 0
+          fi
           source .github/scripts/sarif.sh
           source_modules=$(sarif_source_modules)
           merge_sarif spotbugsSarif.json .sarif-merged/spotbugs.sarif "${SPOTBUGS_EXPECT_REPORTS:-$source_modules}" SpotBugs
@@ -171,6 +227,10 @@ jobs:
         # Require successful reports from every expected module before counting findings.
         run: |
           set -euo pipefail
+          if [ "${SPOTBUGS_KEPT:-}" = "0" ]; then
+            echo "Scope contains no analysable modules — nothing to scan."
+            exit 0
+          fi
           source .github/scripts/sarif.sh
           source_modules=$(sarif_source_modules)
           for mod in ${SPOTBUGS_EXPECT_REPORTS:-$source_modules}; do
@@ -185,7 +245,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

docs/ci-static-analysis-design.md

@@ -94,9 +94,15 @@ the count-gate *stricter* than `:check` (over-fail, never under-fail), each guar
 
 ## Two operational rules
 
-- **`compile` must be full-reactor** (`-f ddk-parent/pom.xml`, no `-pl`). PMD's
-  type-resolving rules need the complete aux-classpath; a `-pl` subset produces
-  false positives (the trailing-`Throwable` case).
+- **A changed module must compile against its complete aux-classpath**, or PMD's
+  type-resolving rules false-positive (the trailing-`Throwable` case). A bare `-pl <module>`
+  subset breaks this, but `-pl <changed> -am` does not: `--also-make` restores the module's
+  dependency closure, which compiles for the classpath even though those deps carry the
+  analysis skip. Tycho adds each bundle's MANIFEST requirements (`Require-Bundle`,
+  `Import-Package`) to the Maven project model, so `-am` pulls in reactor siblings required
+  that way too. `ddk-target` is the one edge with no MANIFEST reference at all, so it is
+  pinned explicitly into every scoped reactor (a cold cache without it fails loudly rather
+  than resolving a stale target).
 - **Merge SARIFs from SARIF files only.** Code Scanning accepts one run per category,
   so per-module SARIFs are merged (jq) before upload. The `ddk-parent` aggregator emits
   plain-XML `checkstyle-result.xml`; it is excluded by the expected source-module list.