Skip to content

Commit 49f7655

Browse files
joaodinissfclaude
andcommitted
ci: fast static analysis with early-fail lint + inline SARIF
Replace the pmd/checkstyle jobs with a parallel shape that gives early, inline feedback and stops re-running analysis inside the build: - lint: compile + pmd:pmd + checkstyle:checkstyle (SARIF) + pmd:cpd-check, -T 2C, --fail-never; gates by counting the merged SARIF (+ cpd.xml grep). Fails in ~3-5 min on its own check, independent of the build. - spotbugs: compile + spotbugs:spotbugs (SARIF), -Xmx4g, own parallel lane (the slow analysis). - maven-verify: build + tests only; the redundant checkstyle/pmd/spotbugs goals are dropped (now owned by lint/spotbugs). - line-endings: unchanged. - both new lanes restore the master-produced Linux-maven-publish-* cache, restore-only, mirroring snapshot.yml's path spec and key recipe exactly (the path spec is hashed into the cache version, so the mirroring must be literal). All three emit SARIF 2.1.0, merged per tool and uploaded to Code Scanning (security-events: write) for inline annotations on the PR diff + Security tab. No custom Python annotator. Count-gate rather than the *:check goals: the check goals @Execute-fork a second analysis and cannot emit SARIF, and without the full compile classpath they false-positive on type-resolving rules. Each report goal runs once (full-reactor compile -> correct + SARIF) and the gate counts the result. Rationale + tables in docs/ci-static-analysis-design.md; measurement protocol in docs/ci-measurement-protocol.md. CPD gating is wired but inert until #1339 lowers the token threshold. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent df46e04 commit 49f7655

3 files changed

Lines changed: 368 additions & 9 deletions

File tree

.github/workflows/verify.yml

Lines changed: 195 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,30 @@
11
name: verify
22
on:
33
pull_request:
4+
5+
# Code Scanning needs write access to upload SARIF results for inline annotations.
6+
permissions:
7+
contents: read
8+
security-events: write
9+
410
jobs:
5-
pmd:
11+
# Fast, early-fail lint lane: PMD + Checkstyle (+ CPD). Turns red in a few
12+
# minutes on any violation, independent of the long build below, so a stray
13+
# PMD/Checkstyle issue is reported immediately rather than after `verify`.
14+
#
15+
# `compile` is in the same invocation as the analysis goals: PMD's
16+
# type-resolving rules (e.g. InvalidLogMessageFormat on the SLF4J
17+
# trailing-Throwable idiom) need Tycho's aux-classpath, which a fresh `mvn`
18+
# does not inherit from a prior step's target/classes.
19+
#
20+
# `--fail-never` lets every module produce its report (no Maven cascade-skip),
21+
# so the uploaded SARIF — and therefore the inline annotations — are complete.
22+
# The trade-off: --fail-never suppresses even compile and target-resolution
23+
# failures (mvn exits 0 on a broken build), so the gate pairs the jq count
24+
# with a presence check — zero valid analyzer inputs fails the lane rather
25+
# than reading as a clean pass. Compilation itself is independently gated by
26+
# maven-verify.
27+
lint:
628
runs-on: ubuntu-24.04
729
steps:
830
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
@@ -12,10 +34,111 @@ jobs:
1234
java-version: '21'
1335
- name: Set up Workspace Environment Variable
1436
run: echo "WORKSPACE=${{ github.workspace }}" >> $GITHUB_ENV
15-
- name: PMD Check
16-
run: mvn pmd:pmd pmd:cpd pmd:check pmd:cpd-check -f ./ddk-parent/pom.xml --batch-mode --fail-at-end
17-
checkstyle:
37+
- name: Restore Maven dependency cache
38+
# Restore-only, mirroring snapshot.yml's producer cache exactly (path and
39+
# key are hashed into the cache version — see the maven-verify step).
40+
uses: actions/cache/restore@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0
41+
with:
42+
path: ~/.m2/repository
43+
key: ${{ runner.os }}-maven-publish-${{ hashFiles('**/pom.xml', '**/*.target') }}
44+
restore-keys: ${{ runner.os }}-maven-publish-
45+
46+
- name: PMD + Checkstyle reports (SARIF)
47+
# PMD: SarifRenderer FQCN — emits pmd.sarif.json AND keeps pmd.xml.
48+
# Checkstyle: output.format=sarif — SARIF content in checkstyle-result.xml.
49+
# CPD is excluded here: the global -Dformat flag uses PMD's Renderer
50+
# hierarchy and would ClassCastException CPD's CPDReportRenderer.
51+
run: |
52+
mvn -T 2C -f ./ddk-parent/pom.xml --batch-mode --fail-never \
53+
compile \
54+
pmd:pmd checkstyle:checkstyle \
55+
-Dformat=net.sourceforge.pmd.renderers.SarifRenderer \
56+
-Dcheckstyle.output.format=sarif
57+
58+
- name: CPD report (separate invocation — no SARIF support)
59+
# CPD has no SARIF renderer; emits cpd.xml only. Run standalone so the
60+
# PMD -Dformat flag isn't in scope.
61+
# NOTE: project CPD token threshold is currently very high (issue #1339),
62+
# which effectively disables detection; re-tune once #1339 lands.
63+
run: |
64+
mvn -T 2C -f ./ddk-parent/pom.xml --batch-mode --fail-never \
65+
compile \
66+
pmd:cpd-check
67+
68+
- name: Merge per-module SARIFs (PMD + Checkstyle)
69+
if: always()
70+
# Code Scanning accepts one run per category per upload; each analyzer
71+
# writes one SARIF per module, so concatenate each analyzer's results
72+
# into a single run under .sarif-merged/.
73+
run: |
74+
mkdir -p .sarif-merged
75+
# Merge per-module SARIFs into one run. Filter to JSON-parseable files:
76+
# the ddk-parent aggregator writes a plain-XML checkstyle-result.xml that
77+
# would break jq, and modules with no findings may emit non-SARIF stubs.
78+
merge() { # $1 = find-glob, $2 = output
79+
local f valid=()
80+
while IFS= read -r f; do
81+
jq -e . "$f" >/dev/null 2>&1 && valid+=("$f")
82+
done < <(find . -path "$1")
83+
if [ ${#valid[@]} -gt 0 ]; then
84+
# del(.ruleIndex): each result's ruleIndex points into its OWN run's
85+
# rules array, but the merge keeps only the first run's tool — Code
86+
# Scanning must resolve rules by ruleId string instead.
87+
jq -s '{
88+
"$schema": .[0]."$schema", version: .[0].version,
89+
runs: [{ tool: .[0].runs[0].tool,
90+
results: [.[].runs[].results[]? | del(.ruleIndex)],
91+
invocations: [.[].runs[].invocations[]?] }]
92+
}' "${valid[@]}" > "$2"
93+
fi
94+
}
95+
merge '*/target/pmd.sarif.json' .sarif-merged/pmd.sarif
96+
merge '*/target/checkstyle-result.xml' .sarif-merged/checkstyle.sarif
97+
98+
- name: Gate on PMD / CPD / Checkstyle violations
99+
# merge() only writes its output when it found at least one valid input,
100+
# so a missing merged file means that analyzer silently died (e.g. a
101+
# plugin bump broke a renderer flag) — never a clean pass.
102+
run: |
103+
set -eu
104+
for f in .sarif-merged/pmd.sarif .sarif-merged/checkstyle.sarif; do
105+
if [ ! -s "$f" ]; then
106+
echo "::error::No valid SARIF input produced for ${f##*/} — the analysis silently failed."
107+
exit 1
108+
fi
109+
done
110+
if [ "$(find . -name 'cpd.xml' -path '*/target/*' | wc -l)" -eq 0 ]; then
111+
echo "::error::No cpd.xml produced — CPD silently failed."
112+
exit 1
113+
fi
114+
sarif_total=$(jq '[.runs[].results[]?] | length' \
115+
.sarif-merged/pmd.sarif .sarif-merged/checkstyle.sarif 2>/dev/null \
116+
| awk '{s+=$1} END {print s+0}')
117+
cpd_total=$(find . -name 'cpd.xml' -path '*/target/*' -exec grep -c '<duplication ' {} + 2>/dev/null \
118+
| awk -F: '{s+=$2} END {print s+0}')
119+
echo "PMD/Checkstyle SARIF violations: $sarif_total"
120+
echo "CPD duplications: $cpd_total"
121+
if [ "$sarif_total" != "0" ] || [ "$cpd_total" != "0" ]; then
122+
echo "::error::Static analysis found violations (PMD/CPD/Checkstyle)."
123+
exit 1
124+
fi
125+
126+
- name: Upload PMD/Checkstyle SARIF to Code Scanning
127+
if: always()
128+
# Annotation-only, never the gate: a fork PR gets a read-only token and
129+
# upload-sarif 403s, which must not red an otherwise-clean lane.
130+
continue-on-error: true
131+
uses: github/codeql-action/upload-sarif@7fd177fa680c9881b53cdab4d346d32574c9f7f4 # v3.35.4
132+
with:
133+
sarif_file: .sarif-merged
134+
category: lint
135+
136+
# SpotBugs is the slow critical-path analysis (the experiments' durable
137+
# finding), so it runs in its own parallel lane and never delays `lint`.
138+
spotbugs:
18139
runs-on: ubuntu-24.04
140+
env:
141+
MAVEN_OPTS: -Xmx4g
19142
steps:
20143
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
21144
- uses: actions/setup-java@0f481fcb613427c0f801b606911222b5b6f3083a # v5
@@ -24,14 +147,79 @@ jobs:
24147
java-version: '21'
25148
- name: Set up Workspace Environment Variable
26149
run: echo "WORKSPACE=${{ github.workspace }}" >> $GITHUB_ENV
27-
- name: Checkstyle Check
28-
run: mvn checkstyle:checkstyle checkstyle:check -f ./ddk-parent/pom.xml --batch-mode --fail-at-end
150+
- name: Restore Maven dependency cache
151+
# Restore-only, mirroring snapshot.yml's producer cache exactly (path and
152+
# key are hashed into the cache version — see the maven-verify step).
153+
uses: actions/cache/restore@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0
154+
with:
155+
path: ~/.m2/repository
156+
key: ${{ runner.os }}-maven-publish-${{ hashFiles('**/pom.xml', '**/*.target') }}
157+
restore-keys: ${{ runner.os }}-maven-publish-
158+
159+
- name: SpotBugs report (SARIF)
160+
# sarifOutput=true emits spotbugsSarif.json (also writes spotbugsXml.xml).
161+
run: |
162+
mvn -T 2C -f ./ddk-parent/pom.xml --batch-mode --fail-never \
163+
compile \
164+
spotbugs:spotbugs \
165+
-Dspotbugs.sarifOutput=true
166+
167+
- name: Merge per-module SpotBugs SARIFs
168+
if: always()
169+
run: |
170+
mkdir -p .sarif-merged
171+
valid=()
172+
while IFS= read -r f; do
173+
jq -e . "$f" >/dev/null 2>&1 && valid+=("$f")
174+
done < <(find . -path '*/target/spotbugsSarif.json')
175+
if [ ${#valid[@]} -gt 0 ]; then
176+
# del(.ruleIndex): see the lint merge — indexes are per-run, the
177+
# merged tool keeps only the first run's rules.
178+
jq -s '{
179+
"$schema": .[0]."$schema", version: .[0].version,
180+
runs: [{ tool: .[0].runs[0].tool,
181+
results: [.[].runs[].results[]? | del(.ruleIndex)],
182+
invocations: [.[].runs[].invocations[]?] }]
183+
}' "${valid[@]}" > .sarif-merged/spotbugs.sarif
184+
fi
185+
186+
- name: Gate on SpotBugs violations
187+
# A missing merged SARIF means the analysis silently died (--fail-never
188+
# suppresses even compile/resolution failures) — never a clean pass.
189+
run: |
190+
set -eu
191+
if [ ! -s .sarif-merged/spotbugs.sarif ]; then
192+
echo "::error::No SpotBugs SARIF produced — the analysis silently failed."
193+
exit 1
194+
fi
195+
sb_total=$(jq '[.runs[].results[]?] | length' .sarif-merged/spotbugs.sarif 2>/dev/null || echo 0)
196+
echo "SpotBugs SARIF violations: $sb_total"
197+
if [ "$sb_total" != "0" ]; then
198+
echo "::error::SpotBugs found violations."
199+
exit 1
200+
fi
201+
202+
- name: Upload SpotBugs SARIF to Code Scanning
203+
if: always()
204+
# Annotation-only, never the gate: a fork PR gets a read-only token and
205+
# upload-sarif 403s, which must not red an otherwise-clean lane.
206+
continue-on-error: true
207+
uses: github/codeql-action/upload-sarif@7fd177fa680c9881b53cdab4d346d32574c9f7f4 # v3.35.4
208+
with:
209+
sarif_file: .sarif-merged
210+
category: spotbugs
211+
29212
line-endings:
30213
runs-on: ubuntu-24.04
31214
steps:
32215
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
33216
- name: Check LF line endings
34217
run: bash .github/scripts/check-line-endings.sh
218+
219+
# Build + tests only. Static analysis now lives in the `lint` and `spotbugs`
220+
# jobs, so the redundant checkstyle/pmd/spotbugs goals are dropped from here —
221+
# this is the wall-clock long pole and no longer re-runs analysis.
222+
# No `-T 2C`: tests are not known to pass reliably under reactor parallelism.
35223
maven-verify:
36224
runs-on: ubuntu-24.04
37225
steps:
@@ -58,9 +246,7 @@ jobs:
58246
key: ${{ runner.os }}-maven-publish-${{ hashFiles('**/pom.xml', '**/*.target') }}
59247
restore-keys: ${{ runner.os }}-maven-publish-
60248
- name: Build with Maven within a virtual X Server Environment
61-
# Run pmd:pmd and pmd:cpd first to generate reports for all modules, then run pmd:check and pmd:cpd-check
62-
# This ensures all violations are collected and reported before the build fails
63-
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
249+
run: xvfb-run mvn clean verify -f ./ddk-parent/pom.xml --batch-mode --fail-at-end
64250
- name: Fail on missing surefire reports
65251
if: always()
66252
run: bash .github/scripts/check-surefire-reports.sh

docs/ci-measurement-protocol.md

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
# CI timing-measurement protocol
2+
3+
How to get **trustworthy** numbers for a CI-shape change. Written because the
4+
earlier experiment program (2026-05) produced numbers that can't be trusted:
5+
single samples, taken during an Eclipse p2 mirror-flaky window, against a CI
6+
shape that has since changed. Don't repeat that.
7+
8+
## What we're measuring
9+
10+
- **Wall-clock** per run = the slowest job (what a developer waits for).
11+
- **Per-job duration** = the analysis bottleneck (settles e.g. spotbugs-vs-maven-verify).
12+
- **Failure latency** = time-to-red on a *planted* lint violation — the metric the
13+
early-fail goal actually cares about. Measure it separately.
14+
15+
Wall-clock is the headline, but per-job + failure-latency are what tell you *why*.
16+
17+
## Noise floor (observed on this repo's `verify` runs)
18+
19+
Per-job spread across recent runs — any change must beat this to be real signal:
20+
21+
| Job | median | spread (±) |
22+
|---|---|---|
23+
| line-endings | ~5s | ±3s |
24+
| checkstyle (old shape) | ~115s | ±~104s (≈90%!) |
25+
| pmd (old shape) | ~191s | ±63s |
26+
| maven-verify | ~869s (~14.5m) | ±~93s (≈11%) |
27+
28+
**Decision rule:** accept candidate B as faster than A only if
29+
`median(B) < median(A) − 2 × IQR(A)`. For maven-verify a real win must exceed ~100s;
30+
for checkstyle, ~200s. Anything smaller is noise.
31+
32+
## Protocol
33+
34+
1. **Pre-flight — confirm mirrors are healthy.** Run one throwaway build; if
35+
target-platform resolution stalls or errors, **stop** — the window is bad
36+
(this is what voided the 2026-05-09 numbers). Measure only on a clean window.
37+
2. **Warm the caches.** Run 3–5 discard builds first so `~/.m2` + `.cache/tycho`
38+
are populated; cold-cache runs have a different (network-bound) profile.
39+
3. **Sample.** N = 15–20 `workflow_dispatch` runs per candidate, back-to-back in a
40+
≤30-minute window. Run A and B **interleaved within ≤10 minutes** of each other
41+
so they see the same mirror weather and runner-pool load.
42+
4. **Report medians + IQR**, per job *and* total wall-clock. Never a single sample,
43+
never the mean.
44+
5. **Pin `ubuntu-24.04`** (not a matrix) for consistent runner hardware.
45+
6. **Record per-sample metadata:** cache hit/miss, run start time, headSha.
46+
7. **Failure latency:** plant a synthetic PMD/Checkstyle violation, measure how long
47+
until the `lint` check goes red (independent of the build job).
48+
49+
## Why not just trust the old experiment numbers
50+
51+
- Single sample each (no repetition).
52+
- 2026-05-09 mirror-flaky window → resolution time is contaminated and varies per job
53+
even within one dispatch.
54+
- Numbers disagree across rounds (sequential measured 21m one round, 33m another).
55+
- `#1369` reshaped master's CI after the experiments ran, so their baseline is stale.
56+
57+
## What is *not* a timing lever (validated)
58+
59+
- **Local p2 mirror**: with a warm `~/.m2`/`.cache/tycho`, offline resolution is
60+
~equal to online (measured 5s vs 6s) — a mirror's steady-state speedup is ~0.
61+
Its only value is cold-cache + flaky-mirror insurance; deferred per the rare-flake
62+
rule.
63+
- **`-Dtycho.mode=maven` on a gate pass**: marginal on a warm cache (resolution is
64+
already seconds); and it *breaks* PMD type-resolving rules (strips the classpath →
65+
false positives). Not used.

0 commit comments

Comments
 (0)