From 51a478d70709f6191f481e66dd7716c8c59221e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Dinis=20Ferreira?= Date: Fri, 10 Jul 2026 23:42:55 +0200 Subject: [PATCH 1/2] 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 Fable 5 --- .github/workflows/verify.yml | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/.github/workflows/verify.yml b/.github/workflows/verify.yml index 3298fa093..82e0a890e 100644 --- a/.github/workflows/verify.yml +++ b/.github/workflows/verify.yml @@ -58,11 +58,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. - # NOTE: project CPD token threshold is currently very high (issue #1339), - # which effectively disables detection; re-tune once #1339 lands. + # No `compile`: CPD is token-based over src/ and needs neither bytecode + # nor the target platform — outputs are identical with and without a + # compile pass (verified at the current token threshold and at 100). + # NOTE: the CPD token threshold is governed by pmd.cpd.min in + # ddk-parent/pom.xml (#1339; tuned to 100 by #1397). run: | mvn -T 2C -f ./ddk-parent/pom.xml --batch-mode --fail-never \ - compile \ pmd:cpd-check - name: Merge per-module SARIFs (PMD + Checkstyle) From 08c868fcc33053880a812ae4776ee85a9b4e5571 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Dinis=20Ferreira?= Date: Fri, 10 Jul 2026 23:48:30 +0200 Subject: [PATCH 2/2] ci: scope PMD/CPD/Checkstyle to changed modules in the lint lane MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit compute-spotbugs-skip.sh becomes compute-analysis-skip.sh with a mode argument: `spotbugs` injects spotbugs.skip as before, `lint` injects pmd.skip, cpd.skip and checkstyle.skip, and each mode exports its -pl/-am reactor scope args. The lint lane gains the scope step and passes LINT_SCOPE_ARGS to both invocations; `compile` stays in the PMD/Checkstyle invocation because PMD's type-resolving rules need Tycho's aux-classpath (skip-injected -am dependencies compile but are not analysed). Changes under ddk-configuration (rulesets, filters) now also trigger the full-scan fail-safe in both lanes. Code Scanning note: repo-wide alert state reflects the default branch, which receives no lint/spotbugs analyses (verify runs on pull_request only), so a scoped upload that omits unchanged modules can only affect PR-context annotations — the same property the spotbugs category has had since the per-module skip landed. Co-Authored-By: Claude Fable 5 --- ...tbugs-skip.sh => compute-analysis-skip.sh} | 92 +++++++++++-------- .github/workflows/verify.yml | 56 +++++++++-- 2 files changed, 102 insertions(+), 46 deletions(-) rename .github/scripts/{compute-spotbugs-skip.sh => compute-analysis-skip.sh} (67%) diff --git a/.github/scripts/compute-spotbugs-skip.sh b/.github/scripts/compute-analysis-skip.sh similarity index 67% rename from .github/scripts/compute-spotbugs-skip.sh rename to .github/scripts/compute-analysis-skip.sh index 96f7f7906..d645ee9a8 100755 --- a/.github/scripts/compute-spotbugs-skip.sh +++ b/.github/scripts/compute-analysis-skip.sh @@ -1,29 +1,40 @@ #!/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. # # 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). +# 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 changed=$(git diff --name-only --diff-filter=ACMR "${base}...HEAD") @@ -45,8 +56,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 @@ -60,10 +70,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 @@ -76,18 +86,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 @@ -103,7 +115,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 @@ -115,6 +127,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 @@ -127,25 +146,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 82e0a890e..2418ca384 100644 --- a/.github/workflows/verify.yml +++ b/.github/workflows/verify.yml @@ -28,6 +28,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@b6effb05e454b25005698d916606bdc6ffcbf961 # v5 with: distribution: 'temurin' @@ -43,17 +45,33 @@ jobs: key: ${{ runner.os }}-maven-publish-${{ hashFiles('**/pom.xml', '**/*.target') }} restore-keys: ${{ runner.os }}-maven-publish- + - 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-never \ + mvn -T 2C -f ./ddk-parent/pom.xml ${LINT_SCOPE_ARGS:-} --batch-mode --fail-never \ 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 @@ -63,8 +81,11 @@ jobs: # compile pass (verified at the current token threshold and at 100). # NOTE: the CPD token threshold is governed by pmd.cpd.min in # ddk-parent/pom.xml (#1339; tuned to 100 by #1397). + # 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-never \ + mvn -T 2C -f ./ddk-parent/pom.xml ${LINT_SCOPE_ARGS:-} --batch-mode --fail-never \ pmd:cpd-check - name: Merge per-module SARIFs (PMD + Checkstyle) @@ -101,8 +122,29 @@ jobs: # 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. + # Exception: the scope step skip-injected every module (no reactor module + # changed), where zero reports is the expected state. Each scanned + # source-bearing module must additionally have produced its own PMD + # SARIF, cpd.xml, and Checkstyle SARIF, so a partially-dead scoped + # build cannot hide either. run: | set -eu + if [ "${LINT_KEPT:-}" = "0" ]; then + echo "All modules skip-injected (no reactor module changed) — nothing to lint." + exit 0 + fi + for mod in ${LINT_EXPECT_REPORTS:-}; do + for rep in pmd.sarif.json cpd.xml; do + if [ ! -s "${mod}/target/${rep}" ]; then + echo "::error::${mod} was scanned but produced no ${rep} — a build failure was swallowed by --fail-never." + exit 1 + fi + done + if ! jq -e . "${mod}/target/checkstyle-result.xml" >/dev/null 2>&1; then + echo "::error::${mod} was scanned but produced no valid Checkstyle SARIF — a build failure was swallowed by --fail-never." + exit 1 + fi + done 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." @@ -116,7 +158,7 @@ jobs: 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 '/dev/null \ + cpd_total=$(find . -name 'cpd.xml' -path '*/target/*' -exec grep -cH '/dev/null \ | awk -F: '{s+=$2} END {print s+0}') echo "PMD/Checkstyle SARIF violations: $sarif_total" echo "CPD duplications: $cpd_total" @@ -126,7 +168,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 @@ -166,7 +210,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).