From 8d721b6d4c310949a302be9ae3846a305aff9629 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Dinis=20Ferreira?= Date: Wed, 23 Sep 2026 00:37:54 +0200 Subject: [PATCH 1/4] ci: scope SpotBugs to the PR's changed modules (per-module skip) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SpotBugs' per-module analysis is the spotbugs job's long pole. A PR only needs its changed modules scanned, so a pre-step injects 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. Validate and merge only expected SpotBugs reports. When the scope contains no analysable modules, both merging and gating succeed without reports. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/compute-spotbugs-skip.sh | 105 +++++++++++++++++++++++ .github/workflows/verify.yml | 28 +++++- 2 files changed, 131 insertions(+), 2 deletions(-) create mode 100755 .github/scripts/compute-spotbugs-skip.sh diff --git a/.github/scripts/compute-spotbugs-skip.sh b/.github/scripts/compute-spotbugs-skip.sh new file mode 100755 index 000000000..fa99ebde7 --- /dev/null +++ b/.github/scripts/compute-spotbugs-skip.sh @@ -0,0 +1,105 @@ +#!/usr/bin/env bash +# +# Scope SpotBugs to a pull request's changed modules. +# +# Default is RUN (analyze). On a PR this injects true +# 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. +# +# 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 this script. +# +# Run from the repository root. Usage: compute-spotbugs-skip.sh +set -euo pipefail +base="${1:?base sha required}" + +# --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") + +# 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. +# 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*) + echo "Build/config change ($f) -> full SpotBugs scan (no skips)." + exit 0 + ;; + esac +done < (strip the leading ../). +# ddk-parent is NOT in its own , so it can never be skip-injected — which +# is what prevents an accidental inherited (global) skip. +module_dirs=$(grep -oE '\.\./[^<]+' ddk-parent/pom.xml \ + | sed -E 's#.*\.\./([^<]+)#\1#') + +# 4) Idempotently inject the skip property; handle poms with and without . +# 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 '' "$pom"; then return 0; fi + if grep -q '' "$pom"; then + sed -i.bak 's##\n true#' "$pom" + else + sed -i.bak 's## \n true\n \n#' "$pom" + fi + rm -f "$pom.bak" +} + +# 5) Skip every reactor module that was not touched by this PR. Kept bundles with +# sources are expected to produce a SARIF; the gate checks each one. +kept=0 +skipped=0 +expect_reports="" +while IFS= read -r mod; do + [ -n "$mod" ] || continue + if printf '%s\n' "${changed_mods}" | grep -qx "$mod"; then + kept=$((kept + 1)) + 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 <> "$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:-}" diff --git a/.github/workflows/verify.yml b/.github/workflows/verify.yml index 4e16cd0eb..c601279f4 100644 --- a/.github/workflows/verify.yml +++ b/.github/workflows/verify.yml @@ -136,6 +136,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 +153,34 @@ 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 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-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 +189,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 +207,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 From 2c2dce32e9b2fc750efbd2710352daa64a2fd027 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Dinis=20Ferreira?= Date: Wed, 23 Sep 2026 00:38:59 +0200 Subject: [PATCH 2/4] ci: build only changed modules + upstream deps in the spotbugs lane Export SPOTBUGS_SCOPE_ARGS with the changed modules and -am so the lane compiles only those modules and their upstream dependencies. Unchanged dependencies retain spotbugs.skip and provide the analysis classpath without being analysed themselves. Include ddk-target explicitly because the target definition is not a MANIFEST dependency that -am can discover. Shared build/config changes retain a full reactor and scan. Export the full expected-report list and an explicit scope state on that path. With no analysable changed modules, skip Maven and accept no reports in the merge and gate. Otherwise preserve shared validation for every expected report, including failed analysis and invalid input, before counting findings. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/compute-spotbugs-skip.sh | 86 +++++++++++++++++++----- .github/workflows/verify.yml | 15 +++-- docs/ci-static-analysis-design.md | 12 +++- 3 files changed, 89 insertions(+), 24 deletions(-) diff --git a/.github/scripts/compute-spotbugs-skip.sh b/.github/scripts/compute-spotbugs-skip.sh index fa99ebde7..02e6e6437 100755 --- a/.github/scripts/compute-spotbugs-skip.sh +++ b/.github/scripts/compute-spotbugs-skip.sh @@ -17,7 +17,12 @@ # 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. +# 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 -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 set -euo pipefail @@ -27,6 +32,23 @@ base="${1:?base sha required}" # 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 (strip the leading ../), and the +# subset that bears sources. ddk-parent is NOT in its own , 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 '\.\./[^<]+' ddk-parent/pom.xml \ + | sed -E 's#.*\.\./([^<]+)#\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 < 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. @@ -36,6 +58,16 @@ 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 "SPOTBUGS_KEPT=all" >> "$GITHUB_ENV" + echo "SPOTBUGS_EXPECT_REPORTS=${all_source_modules}" >> "$GITHUB_ENV" + fi echo "Build/config change ($f) -> full SpotBugs scan (no skips)." exit 0 ;; @@ -49,13 +81,7 @@ EOF # 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 (strip the leading ../). -# ddk-parent is NOT in its own , so it can never be skip-injected — which -# is what prevents an accidental inherited (global) skip. -module_dirs=$(grep -oE '\.\./[^<]+' ddk-parent/pom.xml \ - | sed -E 's#.*\.\./([^<]+)#\1#') - -# 4) Idempotently inject the skip property; handle poms with and without . +# 3) Idempotently inject the skip property; handle poms with and without . # sed -i.bak + rm is portable across GNU (CI) and BSD (local) sed. inject_skip() { local pom="$1/pom.xml" @@ -69,15 +95,20 @@ inject_skip() { rm -f "$pom.bak" } -# 5) Skip every reactor module that was not touched by this PR. Kept bundles with -# sources are expected to produce a SARIF; the gate checks each one. +# 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 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 @@ -91,15 +122,38 @@ EOF # The gate's presence check needs to distinguish "all modules skip-injected" # (zero reports is the expected state) from "the analysis silently died". -# Only source-less modules changed (feature / target / repository) counts as -# nothing to scan: SpotBugs cannot write a report for a bundle without classes. -if [ -z "$expect_reports" ]; then - kept=0 +# 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 "SPOTBUGS_KEPT=${kept}" >> "$GITHUB_ENV" + echo "SPOTBUGS_KEPT=${effective_kept}" >> "$GITHUB_ENV" +fi + +# 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 Maven step entirely on SPOTBUGS_KEPT=0. +if [ "$effective_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 "SpotBugs scope: scanning ${effective_kept} changed module(s), skipping ${skipped} unchanged." echo "Changed modules: ${changed_mods:-}" +if [ "$kept" -gt 0 ] && [ "$effective_kept" -eq 0 ]; then + echo "Only source-less modules changed (no analysable sources) -> no-op (KEPT=0)." +fi +if [ "$effective_kept" -gt 0 ]; then + echo "Reactor scope args: -pl ../ddk-target,${kept_pl} -am" +else + echo "Reactor scope args: " +fi diff --git a/.github/workflows/verify.yml b/.github/workflows/verify.yml index c601279f4..11d20aa33 100644 --- a/.github/workflows/verify.yml +++ b/.github/workflows/verify.yml @@ -154,10 +154,11 @@ jobs: restore-keys: ${{ runner.os }}-maven-publish- - name: Scope SpotBugs to the PR's changed modules - # Injects 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. + # Injects true> into unchanged module poms so their analysis is + # skipped, and exports SPOTBUGS_SCOPE_ARGS (-pl -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) @@ -166,8 +167,12 @@ jobs: # 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 \ diff --git a/docs/ci-static-analysis-design.md b/docs/ci-static-analysis-design.md index 8e7decc74..56ffba37b 100644 --- a/docs/ci-static-analysis-design.md +++ b/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 ` + subset breaks this, but `-pl -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. From 2fe362afe1ce4efe174ed11d852a8db44e3ed1df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Dinis=20Ferreira?= Date: Wed, 23 Sep 2026 00:39:09 +0200 Subject: [PATCH 3/4] ci: run CPD without a compile pass in the lint lane CPD tokenizes sources under src/ and needs neither bytecode nor a resolved target platform, so the second lint invocation drops its compile goals. cpd.xml outputs are identical with and without the compile pass, verified at the current token threshold and at the PMD default of 100 (timestamp attributes aside). Measured locally (warm tree, JDK 21): 6.8s vs 44.3s at the current threshold; 4.5s vs 28.5s at threshold 100. In CI the invocation was 53s, ~40s of it redundant recompilation and JVM startup. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/verify.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/verify.yml b/.github/workflows/verify.yml index 11d20aa33..b7385333a 100644 --- a/.github/workflows/verify.yml +++ b/.github/workflows/verify.yml @@ -69,11 +69,13 @@ jobs: - 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. run: | mvn -T 2C -f ./ddk-parent/pom.xml --batch-mode --fail-at-end \ - compile \ pmd:cpd-check - name: Merge per-module SARIFs (PMD + Checkstyle) From 6fe886964305f0395bda66be00ee6149d4fb0c26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Dinis=20Ferreira?= Date: Wed, 23 Sep 2026 00:39:28 +0200 Subject: [PATCH 4/4] ci: scope PMD/CPD/Checkstyle to changed modules in the lint lane Generalize compute-spotbugs-skip.sh to compute-analysis-skip.sh with a mode argument. The lint mode injects pmd.skip, cpd.skip and checkstyle.skip into unchanged modules and exports the expected reports and -pl/-am scope. SpotBugs keeps its existing scoping behavior through the same script. Compile the changed modules and their dependencies before PMD/Checkstyle so type resolution has the complete classpath. Run CPD separately without compilation. Shared build/config changes trigger a full scan in both lanes. Skip the report goals, merging and gating when the scope contains no analysable modules. Otherwise retain the common validation of every expected SARIF report, URI bases and rule descriptors. Count CPD duplications from the expected reports, including a single-module scope. Co-Authored-By: Claude Opus 5.5 --- ...tbugs-skip.sh => compute-analysis-skip.sh} | 92 +++++++++++-------- .github/workflows/verify.yml | 41 ++++++++- 2 files changed, 88 insertions(+), 45 deletions(-) rename .github/scripts/{compute-spotbugs-skip.sh => compute-analysis-skip.sh} (68%) diff --git a/.github/scripts/compute-spotbugs-skip.sh b/.github/scripts/compute-analysis-skip.sh similarity index 68% rename from .github/scripts/compute-spotbugs-skip.sh rename to .github/scripts/compute-analysis-skip.sh index 02e6e6437..b30699b19 100755 --- a/.github/scripts/compute-spotbugs-skip.sh +++ b/.github/scripts/compute-analysis-skip.sh @@ -1,13 +1,15 @@ #!/usr/bin/env bash # -# Scope SpotBugs to a pull request's changed modules. +# Scope static analysis (SpotBugs, or PMD/CPD/Checkstyle) to a pull request's +# changed modules. # -# Default is RUN (analyze). On a PR this injects true -# 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. +# Default is RUN (analyze). On a PR this injects true +# 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. @@ -17,16 +19,25 @@ # 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). +# 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 -# ("-pl -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. +# On top of the skips, the changed reactor modules are exported as +# SPOTBUGS_SCOPE_ARGS / LINT_SCOPE_ARGS ("-pl -am") so the lane builds +# only those modules plus their upstream dependencies instead of the full reactor. +# The lane's gate cross-checks _KEPT / _EXPECT_REPORTS so a build +# failure swallowed by --fail-never can never pass as "nothing to scan". # -# Run from the repository root. Usage: compute-spotbugs-skip.sh +# Run from the repository root. Usage: compute-analysis-skip.sh 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. @@ -50,8 +61,7 @@ ${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 (e.g. the SpotBugs -# exclusion-filter), so a change there must re-scan everything, not skip silently. +# 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 @@ -65,10 +75,10 @@ while IFS= read -r f; do # 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 "SPOTBUGS_KEPT=all" >> "$GITHUB_ENV" - echo "SPOTBUGS_EXPECT_REPORTS=${all_source_modules}" >> "$GITHUB_ENV" + echo "${prefix}_KEPT=all" >> "$GITHUB_ENV" + echo "${prefix}_EXPECT_REPORTS=${all_source_modules}" >> "$GITHUB_ENV" fi - echo "Build/config change ($f) -> full SpotBugs scan (no skips)." + echo "Build/config change ($f) -> full ${mode} scan (no skips)." exit 0 ;; esac @@ -81,18 +91,20 @@ EOF # 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 property; handle poms with and without . +# 3) Idempotently inject the skip properties; handle poms with and without . # sed -i.bak + rm is portable across GNU (CI) and BSD (local) sed. inject_skip() { - local pom="$1/pom.xml" + local pom="$1/pom.xml" prop [ -f "$pom" ] || return 0 - if grep -q '' "$pom"; then return 0; fi - if grep -q '' "$pom"; then - sed -i.bak 's##\n true#' "$pom" - else - sed -i.bak 's## \n true\n \n#' "$pom" - fi - rm -f "$pom.bak" + for prop in $props; do + if grep -q "<${prop//./\\.}>" "$pom"; then continue; fi + if grep -q '' "$pom"; then + sed -i.bak "s##\n <${prop}>true#" "$pom" + else + sed -i.bak "s## \n <${prop}>true\n \n#" "$pom" + fi + rm -f "$pom.bak" + done } # 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 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). + # 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 @@ -120,6 +132,13 @@ done <_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 @@ -132,25 +151,18 @@ else effective_kept=$kept fi if [ -n "${GITHUB_ENV:-}" ]; then - echo "SPOTBUGS_KEPT=${effective_kept}" >> "$GITHUB_ENV" + echo "${prefix}_KEPT=${effective_kept}" >> "$GITHUB_ENV" fi -# 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 Maven step entirely on SPOTBUGS_KEPT=0. if [ "$effective_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" + echo "${prefix}_SCOPE_ARGS=-pl ../ddk-target,${kept_pl} -am" >> "$GITHUB_ENV" + echo "${prefix}_EXPECT_REPORTS=${expect_reports}" >> "$GITHUB_ENV" fi -echo "SpotBugs scope: scanning ${effective_kept} changed module(s), skipping ${skipped} unchanged." +echo "${mode} scope: scanning ${effective_kept} changed module(s), skipping ${skipped} unchanged." echo "Changed modules: ${changed_mods:-}" if [ "$kept" -gt 0 ] && [ "$effective_kept" -eq 0 ]; then - echo "Only source-less modules changed (no analysable sources) -> no-op (KEPT=0)." + 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" diff --git a/.github/workflows/verify.yml b/.github/workflows/verify.yml index b7385333a..daab13733 100644 --- a/.github/workflows/verify.yml +++ b/.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,17 +56,33 @@ 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 -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 @@ -74,8 +92,11 @@ jobs: # 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 \ + 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) @@ -83,6 +104,10 @@ jobs: # 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 @@ -92,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=() @@ -121,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 @@ -161,7 +192,7 @@ jobs: # 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 }}" + 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).