Skip to content

Commit 3c48bdb

Browse files
authored
ci: add Scalpel-based POM dependency detection alongside grep
Add Maveniverse Scalpel 0.1.0 permanently to .mvn/extensions.xml as a parallel POM dependency detection mechanism in incremental-build.sh. Scalpel compares effective POM models between base and PR branches, catching managed dependencies, plugin version changes, BOM imports, and transitive dependency impacts that the existing grep approach misses. Both detection methods run in parallel; results are merged (union) before testing. If Scalpel fails, the script falls back to grep-only. On developer machines, Scalpel is a no-op (no base branch env vars).
1 parent 347de21 commit 3c48bdb

3 files changed

Lines changed: 183 additions & 29 deletions

File tree

.github/CI-ARCHITECTURE.md

Lines changed: 33 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -90,10 +90,14 @@ PR comment: /component-test kafka http
9090
The core test runner. Determines which modules to test using:
9191

9292
1. **File-path analysis**: Maps changed files to Maven modules
93-
2. **POM dependency analysis**: For `parent/pom.xml` changes, detects property changes and finds modules that reference the affected properties in their `pom.xml` files (uses simple grep, not Maveniverse Toolbox — see Known Limitations below)
93+
2. **POM dependency analysis** (dual detection):
94+
- **Grep-based**: For `parent/pom.xml` changes, detects property changes and finds modules that explicitly reference the affected properties via `${property}` in their `pom.xml` files
95+
- **Scalpel-based**: Uses [Maveniverse Scalpel](https://github.com/maveniverse/scalpel) (Maven extension) for effective POM model comparison — catches managed dependencies, plugin version changes, BOM imports, and transitive dependency impacts that the grep approach misses
9496
3. **Extra modules**: Additional modules passed via `/component-test`
9597

96-
Results are merged, deduplicated, and tested. The script also:
98+
Both detection methods run in parallel. Their results are merged (union), deduplicated, and tested. If Scalpel fails (build error, runtime error), the script falls back to grep-only with no regression.
99+
100+
The script also:
97101

98102
- Detects tests disabled in CI (`@DisabledIfSystemProperty(named = "ci.env.name")`)
99103
- Applies an exclusion list for generated/meta modules
@@ -119,21 +123,40 @@ Installs system packages required for the build.
119123

120124
The CI sets `-Dci.env.name=github.com` via `MVND_OPTS` (in `install-mvnd`). Tests can use `@DisabledIfSystemProperty(named = "ci.env.name")` to skip flaky tests in CI. The test comment warns about these skipped tests.
121125

122-
## Known Limitations of POM Dependency Detection
126+
## POM Dependency Detection: Dual Approach
127+
128+
### Grep-based detection (legacy)
129+
130+
The grep approach searches for `${property-name}` references in module `pom.xml` files. It has known limitations:
131+
132+
1. **Managed dependencies without explicit `<version>`** — Modules inheriting versions via `<dependencyManagement>` without declaring `<version>${property}</version>` are missed.
133+
2. **Maven plugin version changes** — Plugin version properties consumed in `parent/pom.xml` via `<pluginManagement>` are invisible to child modules.
134+
3. **BOM imports** — Modules using artifacts from a BOM are not linked to the BOM version property.
135+
4. **Transitive dependency changes** — Only direct property references are detected.
136+
5. **Non-property version changes** — Structural `<dependencyManagement>` edits without property substitution are not caught.
137+
138+
### Scalpel-based detection (new)
139+
140+
[Maveniverse Scalpel](https://github.com/maveniverse/scalpel) is a Maven core extension that compares effective POM models between the base branch and the PR. It resolves all 5 grep limitations by:
123141

124-
The property-grep approach has structural limitations that can cause missed modules:
142+
- Reading old POM files from the merge-base commit (via JGit)
143+
- Comparing properties, managed dependencies, and managed plugins between old and new POMs
144+
- Resolving the full transitive dependency graph to find all affected modules
145+
- Detecting plugin version changes via `project.getBuildPlugins()` comparison
125146

126-
1. **Managed dependencies without explicit** `<version>` — Most Camel modules inherit dependency versions via `<dependencyManagement>` in the parent POM and do not declare `<version>${property}</version>` themselves. When a managed dependency version property changes, only modules that explicitly reference the property are detected — modules relying on inheritance are missed.
147+
Scalpel runs in **report mode** (`-Dscalpel.mode=report`), writing a JSON report to `target/scalpel-report.json` without modifying the Maven reactor. The report includes affected modules with reasons (`SOURCE_CHANGE`, `POM_CHANGE`, `TRANSITIVE_DEPENDENCY`, `MANAGED_PLUGIN`).
127148

128-
2. **Maven plugin version changes are completely invisible** — Plugin version properties (e.g. `<maven-surefire-plugin-version>`) are both defined and consumed in `parent/pom.xml` via `<pluginManagement>`. Since the module search excludes `parent/pom.xml`, no modules are found and **no tests run at all** for plugin updates. Modules inherit plugins from the parent without any `${property}` reference in their own `pom.xml`.
149+
### Dual-detection strategy
129150

130-
3. **BOM imports** — When a BOM version property changes (e.g. `<spring-boot-bom-version>`), modules using artifacts from that BOM are not detected because they reference the BOM's artifacts, not the BOM property.
151+
Both methods run in parallel. Results are merged (union) before testing. This lets us:
131152

132-
4. **Transitive dependency changes** — Modules affected only via transitive dependencies are not detected.
153+
1. **Validate Scalpel** — Compare what each method detects across many PRs
154+
2. **No regression** — If Scalpel fails, grep results are still used
155+
3. **Gradual migration** — Once Scalpel is validated, grep can be removed
133156

134-
5. **Non-property version changes** — Direct edits to `<version>` values (not using `${property}` substitution) or structural changes to `<dependencyManagement>` sections are not caught.
157+
Scalpel is configured permanently in `.mvn/extensions.xml` (version `0.1.0`). On developer machines it is a no-op — without CI environment variables (`GITHUB_BASE_REF`), no base branch is detected and Scalpel returns immediately. The `mvn validate` with report mode adds ~60-90 seconds in CI.
135158

136-
These limitations mean the incremental build may under-test when parent POM properties change. A future improvement could use [Maveniverse Toolbox](https://github.com/maveniverse/toolbox) `tree-find` or [Scalpel](https://github.com/maveniverse/scalpel) to resolve the full dependency graph and detect all affected modules.
159+
Note: the script overrides `fullBuildTriggers` to empty (`-Dscalpel.fullBuildTriggers=`) because Scalpel's default (`.mvn/**`) would trigger a full build whenever `.mvn/extensions.xml` itself changes (e.g., Dependabot bumping Scalpel).
137160

138161
## Manual Integration Test Advisories
139162

.github/actions/incremental-build/incremental-build.sh

Lines changed: 145 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,14 @@
1919
#
2020
# Determines which modules to test by:
2121
# 1. File-path analysis: maps changed files to their Maven modules
22-
# 2. POM dependency analysis: for changed pom.xml files, detects property
23-
# changes and finds modules that reference the affected properties
22+
# 2. POM dependency analysis (dual detection):
23+
# a. Grep-based: detects property changes in parent/pom.xml and finds
24+
# modules that explicitly reference the affected properties
25+
# b. Scalpel-based: uses Maveniverse Scalpel extension for effective POM
26+
# model comparison — catches managed deps, plugin changes, BOM imports,
27+
# and transitive dependency impacts that grep misses
2428
#
25-
# Both sets of affected modules are merged and deduplicated before testing.
29+
# All sets of affected modules are merged and deduplicated before testing.
2630

2731
set -euo pipefail
2832

@@ -185,6 +189,84 @@ analyzePomDependencies() {
185189
done <<< "$changed_props"
186190
}
187191

192+
# ── POM dependency analysis via Scalpel (parallel) ─────────────────────
193+
#
194+
# Uses Maveniverse Scalpel (Maven extension) for effective POM model
195+
# comparison. Detects changed properties, managed dependencies, managed
196+
# plugins, and transitive dependency impacts that the grep approach misses.
197+
# Runs alongside grep — results are merged (union) for testing.
198+
# See https://github.com/maveniverse/scalpel
199+
200+
# Run Scalpel in report mode to detect modules affected by POM changes.
201+
# Sets caller-visible variables: scalpel_module_ids, scalpel_module_paths,
202+
# scalpel_props, scalpel_managed_deps, scalpel_managed_plugins
203+
runScalpelDetection() {
204+
echo " Running Scalpel change detection..."
205+
206+
# Ensure sufficient git history for JGit merge-base detection
207+
# (CI uses shallow clones; Scalpel needs to find the merge base)
208+
git fetch origin main:refs/remotes/origin/main --depth=200 2>/dev/null || true
209+
git fetch --deepen=200 2>/dev/null || true
210+
211+
# Scalpel is permanently configured in .mvn/extensions.xml.
212+
# On developer machines it's a no-op (no GITHUB_BASE_REF → no base branch detected).
213+
# Run Maven validate with Scalpel in report mode:
214+
# - mode=report: write JSON report without trimming the reactor
215+
# - fullBuildTriggers="": override .mvn/** default (Scalpel lives in .mvn/extensions.xml)
216+
# - alsoMake/alsoMakeDependents=false: we only want directly affected modules
217+
# (our script handles -amd expansion separately)
218+
local scalpel_args="-Dscalpel.mode=report -Dscalpel.fullBuildTriggers= -Dscalpel.alsoMake=false -Dscalpel.alsoMakeDependents=false"
219+
# For workflow_dispatch, GITHUB_BASE_REF may not be set
220+
if [ -z "${GITHUB_BASE_REF:-}" ]; then
221+
scalpel_args="$scalpel_args -Dscalpel.baseBranch=origin/main"
222+
fi
223+
224+
echo " Scalpel: running mvn validate (report mode)..."
225+
./mvnw -B -q validate $scalpel_args -l /tmp/scalpel-validate.log 2>/dev/null || {
226+
echo " WARNING: Scalpel detection failed (exit $?), skipping"
227+
grep -i "scalpel" /tmp/scalpel-validate.log 2>/dev/null | head -5 || true
228+
return
229+
}
230+
231+
# Parse the Scalpel report
232+
local report="target/scalpel-report.json"
233+
if [ ! -f "$report" ]; then
234+
echo " WARNING: Scalpel report not found at $report"
235+
grep -i "scalpel" /tmp/scalpel-validate.log 2>/dev/null | head -5 || true
236+
return
237+
fi
238+
239+
# Check if full build was triggered
240+
local full_build
241+
full_build=$(jq -r '.fullBuildTriggered' "$report")
242+
if [ "$full_build" = "true" ]; then
243+
local trigger_file
244+
trigger_file=$(jq -r '.triggerFile // "unknown"' "$report")
245+
echo " Scalpel: Full build triggered by change to $trigger_file"
246+
return
247+
fi
248+
249+
# Extract affected module artifactIds (colon-prefixed for Maven -pl compatibility)
250+
scalpel_module_ids=$(jq -r '.affectedModules[].artifactId' "$report" 2>/dev/null | sort -u | sed 's/^/:/' | tr '\n' ',' | sed 's/,$//' || true)
251+
scalpel_module_paths=$(jq -r '.affectedModules[].path' "$report" 2>/dev/null | sort -u | tr '\n' ',' | sed 's/,$//' || true)
252+
scalpel_props=$(jq -r '(.changedProperties // []) | if length > 0 then join(", ") else "" end' "$report" 2>/dev/null || true)
253+
scalpel_managed_deps=$(jq -r '(.changedManagedDependencies // []) | if length > 0 then join(", ") else "" end' "$report" 2>/dev/null || true)
254+
scalpel_managed_plugins=$(jq -r '(.changedManagedPlugins // []) | if length > 0 then join(", ") else "" end' "$report" 2>/dev/null || true)
255+
256+
local mod_count
257+
mod_count=$(jq '.affectedModules | length' "$report" 2>/dev/null || echo "0")
258+
echo " Scalpel detected $mod_count affected modules"
259+
if [ -n "$scalpel_props" ]; then
260+
echo " Changed properties: $scalpel_props"
261+
fi
262+
if [ -n "$scalpel_managed_deps" ]; then
263+
echo " Changed managed deps: $scalpel_managed_deps"
264+
fi
265+
if [ -n "$scalpel_managed_plugins" ]; then
266+
echo " Changed managed plugins: $scalpel_managed_plugins"
267+
fi
268+
}
269+
188270
# ── Disabled-test detection ─────────────────────────────────────────────
189271

190272
# Scan tested modules for @DisabledIfSystemProperty(named = "ci.env.name")
@@ -269,6 +351,8 @@ writeComment() {
269351
local changed_props_summary="$4"
270352
local testedDependents="$5"
271353
local extra_modules="$6"
354+
local managed_deps_summary="${7:-}"
355+
local managed_plugins_summary="${8:-}"
272356

273357
echo "<!-- ci-tested-modules -->" > "$comment_file"
274358

@@ -288,21 +372,33 @@ writeComment() {
288372

289373
# Section 2: pom dependency-detected modules
290374
if [ -n "$dep_ids" ]; then
375+
echo "" >> "$comment_file"
376+
echo ":white_check_mark: **POM dependency changes: targeted tests included**" >> "$comment_file"
291377
echo "" >> "$comment_file"
292378
if [ -n "$changed_props_summary" ]; then
293-
echo ":white_check_mark: **POM dependency changes: targeted tests included**" >> "$comment_file"
294-
echo "" >> "$comment_file"
295379
echo "Changed properties: ${changed_props_summary}" >> "$comment_file"
296380
echo "" >> "$comment_file"
297-
local dep_count
298-
dep_count=$(echo "$dep_ids" | tr ',' '\n' | wc -l | tr -d ' ')
299-
echo "<details><summary>Modules affected by dependency changes (${dep_count})</summary>" >> "$comment_file"
381+
fi
382+
if [ -n "$managed_deps_summary" ]; then
383+
echo "Changed managed dependencies: ${managed_deps_summary}" >> "$comment_file"
300384
echo "" >> "$comment_file"
301-
echo "$dep_ids" | tr ',' '\n' | while read -r m; do
302-
echo "- \`$m\`" >> "$comment_file"
303-
done
385+
fi
386+
if [ -n "$managed_plugins_summary" ]; then
387+
echo "Changed managed plugins: ${managed_plugins_summary}" >> "$comment_file"
304388
echo "" >> "$comment_file"
305-
echo "</details>" >> "$comment_file"
389+
fi
390+
local dep_count
391+
dep_count=$(echo "$dep_ids" | tr ',' '\n' | wc -l | tr -d ' ')
392+
echo "<details><summary>Modules affected by dependency changes (${dep_count})</summary>" >> "$comment_file"
393+
echo "" >> "$comment_file"
394+
echo "$dep_ids" | tr ',' '\n' | while read -r m; do
395+
echo "- \`$m\`" >> "$comment_file"
396+
done
397+
echo "" >> "$comment_file"
398+
echo "</details>" >> "$comment_file"
399+
if [ -n "$managed_deps_summary" ] || [ -n "$managed_plugins_summary" ]; then
400+
echo "" >> "$comment_file"
401+
echo "> :microscope: Detected via [Maveniverse Scalpel](https://github.com/maveniverse/scalpel) effective POM comparison" >> "$comment_file"
306402
fi
307403
fi
308404

@@ -389,20 +485,27 @@ main() {
389485
done
390486
pl="${pl:1}" # strip leading comma
391487

392-
# Only analyze parent/pom.xml for dependency detection
393-
# (matches original detect-test.sh behavior; detection improvements deferred to follow-up PR)
488+
# Only analyze parent/pom.xml for grep-based dependency detection
489+
# (matches original detect-test.sh behavior)
394490
if echo "$diff_body" | grep -q '^diff --git a/parent/pom.xml'; then
395491
pom_files="parent/pom.xml"
396492
fi
397493

398-
# ── Step 2: POM dependency analysis ──
494+
# ── Step 2: POM dependency analysis (dual: grep + Scalpel) ──
399495
# Variables shared with analyzePomDependencies/findAffectedModules
400496
local dep_module_ids=""
401497
local all_changed_props=""
402-
498+
# Scalpel results (not local — set by runScalpelDetection)
499+
scalpel_module_ids=""
500+
scalpel_module_paths=""
501+
scalpel_props=""
502+
scalpel_managed_deps=""
503+
scalpel_managed_plugins=""
504+
505+
# Step 2a: Grep-based detection (existing approach)
403506
if [ -n "$pom_files" ]; then
404507
echo ""
405-
echo "Analyzing parent POM dependency changes..."
508+
echo "Analyzing parent POM dependency changes (grep)..."
406509
while read -r pom_file; do
407510
[ -z "$pom_file" ] && continue
408511

@@ -421,6 +524,29 @@ main() {
421524
done <<< "$pom_files"
422525
fi
423526

527+
# Step 2b: Scalpel detection (parallel, for any pom.xml change)
528+
# Scalpel uses effective POM model comparison — catches managed deps,
529+
# plugin changes, and transitive impacts that grep misses.
530+
if echo "$diff_body" | grep -q '^diff --git a/.*pom\.xml'; then
531+
echo ""
532+
echo "Running Scalpel POM analysis..."
533+
runScalpelDetection
534+
fi
535+
536+
# Step 2c: Merge grep and Scalpel results (union, deduplicated)
537+
if [ -n "$scalpel_module_ids" ]; then
538+
dep_module_ids="${dep_module_ids:+${dep_module_ids},}${scalpel_module_ids}"
539+
dep_module_ids=$(echo "$dep_module_ids" | tr ',' '\n' | sort -u | tr '\n' ',' | sed 's/,$//')
540+
fi
541+
if [ -n "$scalpel_props" ]; then
542+
if [ -z "$all_changed_props" ]; then
543+
all_changed_props="$scalpel_props"
544+
else
545+
# Merge and deduplicate property names
546+
all_changed_props=$(printf '%s, %s' "$all_changed_props" "$scalpel_props" | tr ',' '\n' | sed 's/^ *//' | sort -u | tr '\n' ',' | sed 's/,$//' | sed 's/,/, /g')
547+
fi
548+
fi
549+
424550
# ── Step 3: Merge and deduplicate ──
425551
# Separate file-path modules into testable (has src/test) and pom-only.
426552
# Pom-only modules (e.g. "parent") are kept in the build list but must NOT
@@ -458,7 +584,7 @@ main() {
458584
if [ -z "$final_pl" ]; then
459585
echo ""
460586
echo "No modules to test"
461-
writeComment "incremental-test-comment.md" "" "" "" "" ""
587+
writeComment "incremental-test-comment.md" "" "" "" "" "" "" ""
462588
exit 0
463589
fi
464590

@@ -546,7 +672,7 @@ main() {
546672

547673
# ── Step 5: Write comment and summary ──
548674
local comment_file="incremental-test-comment.md"
549-
writeComment "$comment_file" "$pl" "$dep_module_ids" "$all_changed_props" "$testedDependents" "$extraModules"
675+
writeComment "$comment_file" "$pl" "$dep_module_ids" "$all_changed_props" "$testedDependents" "$extraModules" "$scalpel_managed_deps" "$scalpel_managed_plugins"
550676

551677
# Check for tests disabled in CI via @DisabledIfSystemProperty(named = "ci.env.name")
552678
local disabled_tests

.mvn/extensions.xml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,9 @@
2020
<artifactId>common-custom-user-data-maven-extension</artifactId>
2121
<version>2.1.0</version>
2222
</extension>
23+
<extension>
24+
<groupId>eu.maveniverse.maven.scalpel</groupId>
25+
<artifactId>extension3</artifactId>
26+
<version>0.1.0</version>
27+
</extension>
2328
</extensions>

0 commit comments

Comments
 (0)