diff --git a/.github/CI-ARCHITECTURE.md b/.github/CI-ARCHITECTURE.md index 72982d383d7d7..74a51a6d4162e 100644 --- a/.github/CI-ARCHITECTURE.md +++ b/.github/CI-ARCHITECTURE.md @@ -90,10 +90,14 @@ PR comment: /component-test kafka http The core test runner. Determines which modules to test using: 1. **File-path analysis**: Maps changed files to Maven modules -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) +2. **POM dependency analysis** (dual detection): + - **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 + - **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 3. **Extra modules**: Additional modules passed via `/component-test` -Results are merged, deduplicated, and tested. The script also: +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. + +The script also: - Detects tests disabled in CI (`@DisabledIfSystemProperty(named = "ci.env.name")`) - Applies an exclusion list for generated/meta modules @@ -119,21 +123,38 @@ Installs system packages required for the build. 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. -## Known Limitations of POM Dependency Detection +## POM Dependency Detection: Dual Approach + +### Grep-based detection (legacy) + +The grep approach searches for `${property-name}` references in module `pom.xml` files. It has known limitations: + +1. **Managed dependencies without explicit ``** — Modules inheriting versions via `` without declaring `${property}` are missed. +2. **Maven plugin version changes** — Plugin version properties consumed in `parent/pom.xml` via `` are invisible to child modules. +3. **BOM imports** — Modules using artifacts from a BOM are not linked to the BOM version property. +4. **Transitive dependency changes** — Only direct property references are detected. +5. **Non-property version changes** — Structural `` edits without property substitution are not caught. + +### Scalpel-based detection (new) -The property-grep approach has structural limitations that can cause missed modules: +[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: -1. **Managed dependencies without explicit** `` — Most Camel modules inherit dependency versions via `` in the parent POM and do not declare `${property}` themselves. When a managed dependency version property changes, only modules that explicitly reference the property are detected — modules relying on inheritance are missed. +- Reading old POM files from the merge-base commit (via JGit) +- Comparing properties, managed dependencies, and managed plugins between old and new POMs +- Resolving the full transitive dependency graph to find all affected modules +- Detecting plugin version changes via `project.getBuildPlugins()` comparison -2. **Maven plugin version changes are completely invisible** — Plugin version properties (e.g. ``) are both defined and consumed in `parent/pom.xml` via ``. 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`. +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`). -3. **BOM imports** — When a BOM version property changes (e.g. ``), modules using artifacts from that BOM are not detected because they reference the BOM's artifacts, not the BOM property. +### Dual-detection strategy -4. **Transitive dependency changes** — Modules affected only via transitive dependencies are not detected. +Both methods run in parallel. Results are merged (union) before testing. This lets us: -5. **Non-property version changes** — Direct edits to `` values (not using `${property}` substitution) or structural changes to `` sections are not caught. +1. **Validate Scalpel** — Compare what each method detects across many PRs +2. **No regression** — If Scalpel fails, grep results are still used +3. **Gradual migration** — Once Scalpel is validated, grep can be removed -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. +Scalpel is resolved as a `0.1.0-SNAPSHOT` from the [Sonatype Central Portal snapshots repository](https://central.sonatype.com/repository/maven-snapshots/). A temporary `settings.xml` is created to enable snapshot resolution. The `mvn validate` with report mode adds ~60-90 seconds. ## Manual Integration Test Advisories diff --git a/.github/actions/incremental-build/incremental-build.sh b/.github/actions/incremental-build/incremental-build.sh index 1938b7c8cefb5..177ef8fb08bca 100755 --- a/.github/actions/incremental-build/incremental-build.sh +++ b/.github/actions/incremental-build/incremental-build.sh @@ -19,10 +19,14 @@ # # Determines which modules to test by: # 1. File-path analysis: maps changed files to their Maven modules -# 2. POM dependency analysis: for changed pom.xml files, detects property -# changes and finds modules that reference the affected properties +# 2. POM dependency analysis (dual detection): +# a. Grep-based: detects property changes in parent/pom.xml and finds +# modules that explicitly reference the affected properties +# b. Scalpel-based: uses Maveniverse Scalpel extension for effective POM +# model comparison — catches managed deps, plugin changes, BOM imports, +# and transitive dependency impacts that grep misses # -# Both sets of affected modules are merged and deduplicated before testing. +# All sets of affected modules are merged and deduplicated before testing. set -euo pipefail @@ -185,6 +189,125 @@ analyzePomDependencies() { done <<< "$changed_props" } +# ── POM dependency analysis via Scalpel (parallel) ───────────────────── +# +# Uses Maveniverse Scalpel (Maven extension) for effective POM model +# comparison. Detects changed properties, managed dependencies, managed +# plugins, and transitive dependency impacts that the grep approach misses. +# Runs alongside grep — results are merged (union) for testing. +# See https://github.com/maveniverse/scalpel + +# Run Scalpel in report mode to detect modules affected by POM changes. +# Sets caller-visible variables: scalpel_module_ids, scalpel_module_paths, +# scalpel_props, scalpel_managed_deps, scalpel_managed_plugins +runScalpelDetection() { + echo " Running Scalpel change detection..." + + # Ensure sufficient git history for JGit merge-base detection + # (CI uses shallow clones; Scalpel needs to find the merge base) + git fetch origin main:refs/remotes/origin/main --depth=200 2>/dev/null || true + git fetch --deepen=200 2>/dev/null || true + + # Temporarily inject Scalpel extension and snapshot repository into .mvn/extensions.xml + # Scalpel 0.1.0-SNAPSHOT is deployed to Sonatype Central Portal snapshots. + cp .mvn/extensions.xml .mvn/extensions.xml.bak + trap 'mv -f .mvn/extensions.xml.bak .mvn/extensions.xml 2>/dev/null || true' RETURN + + awk '/<\/extensions>/{print " ";print " eu.maveniverse.maven.scalpel";print " extension3";print " 0.1.0-SNAPSHOT";print " "}1' \ + .mvn/extensions.xml.bak > .mvn/extensions.xml + + # Create a settings file with the Sonatype Central Portal snapshot repository + # so Maven can resolve the Scalpel extension during initialization. + cat > /tmp/scalpel-settings.xml << 'SETTINGS_EOF' + + + + scalpel-snapshots + + + sonatype-snapshots + https://central.sonatype.com/repository/maven-snapshots/ + false + true + + + + + sonatype-snapshots + https://central.sonatype.com/repository/maven-snapshots/ + false + true + + + + + + scalpel-snapshots + + +SETTINGS_EOF + + # Run Maven validate with Scalpel in report mode: + # - mode=report: write JSON report without trimming the reactor + # - fullBuildTriggers="": override .mvn/** default (we modified extensions.xml) + # - alsoMake/alsoMakeDependents=false: we only want directly affected modules + # (our script handles -amd expansion separately) + local scalpel_args="-Dscalpel.mode=report -Dscalpel.fullBuildTriggers= -Dscalpel.alsoMake=false -Dscalpel.alsoMakeDependents=false" + # For workflow_dispatch, GITHUB_BASE_REF may not be set + if [ -z "${GITHUB_BASE_REF:-}" ]; then + scalpel_args="$scalpel_args -Dscalpel.baseBranch=origin/main" + fi + + echo " Scalpel: running mvn validate (report mode)..." + ./mvnw -B -q validate -s /tmp/scalpel-settings.xml $scalpel_args -l /tmp/scalpel-validate.log 2>/dev/null || { + echo " WARNING: Scalpel detection failed (exit $?), skipping" + grep -i "scalpel" /tmp/scalpel-validate.log 2>/dev/null | head -5 || true + return + } + + # Restore extensions.xml immediately (don't wait for function return) + mv -f .mvn/extensions.xml.bak .mvn/extensions.xml + trap - RETURN + + # Parse the Scalpel report + local report="target/scalpel-report.json" + if [ ! -f "$report" ]; then + echo " WARNING: Scalpel report not found at $report" + grep -i "scalpel" /tmp/scalpel-validate.log 2>/dev/null | head -5 || true + return + fi + + # Check if full build was triggered + local full_build + full_build=$(jq -r '.fullBuildTriggered' "$report") + if [ "$full_build" = "true" ]; then + local trigger_file + trigger_file=$(jq -r '.triggerFile // "unknown"' "$report") + echo " Scalpel: Full build triggered by change to $trigger_file" + return + fi + + # Extract affected module artifactIds (colon-prefixed for Maven -pl compatibility) + scalpel_module_ids=$(jq -r '.affectedModules[].artifactId' "$report" 2>/dev/null | sort -u | sed 's/^/:/' | tr '\n' ',' | sed 's/,$//' || true) + scalpel_module_paths=$(jq -r '.affectedModules[].path' "$report" 2>/dev/null | sort -u | tr '\n' ',' | sed 's/,$//' || true) + scalpel_props=$(jq -r '(.changedProperties // []) | if length > 0 then join(", ") else "" end' "$report" 2>/dev/null || true) + scalpel_managed_deps=$(jq -r '(.changedManagedDependencies // []) | if length > 0 then join(", ") else "" end' "$report" 2>/dev/null || true) + scalpel_managed_plugins=$(jq -r '(.changedManagedPlugins // []) | if length > 0 then join(", ") else "" end' "$report" 2>/dev/null || true) + + local mod_count + mod_count=$(jq '.affectedModules | length' "$report" 2>/dev/null || echo "0") + echo " Scalpel detected $mod_count affected modules" + if [ -n "$scalpel_props" ]; then + echo " Changed properties: $scalpel_props" + fi + if [ -n "$scalpel_managed_deps" ]; then + echo " Changed managed deps: $scalpel_managed_deps" + fi + if [ -n "$scalpel_managed_plugins" ]; then + echo " Changed managed plugins: $scalpel_managed_plugins" + fi +} + # ── Disabled-test detection ───────────────────────────────────────────── # Scan tested modules for @DisabledIfSystemProperty(named = "ci.env.name") @@ -269,6 +392,8 @@ writeComment() { local changed_props_summary="$4" local testedDependents="$5" local extra_modules="$6" + local managed_deps_summary="${7:-}" + local managed_plugins_summary="${8:-}" echo "" > "$comment_file" @@ -288,21 +413,33 @@ writeComment() { # Section 2: pom dependency-detected modules if [ -n "$dep_ids" ]; then + echo "" >> "$comment_file" + echo ":white_check_mark: **POM dependency changes: targeted tests included**" >> "$comment_file" echo "" >> "$comment_file" if [ -n "$changed_props_summary" ]; then - echo ":white_check_mark: **POM dependency changes: targeted tests included**" >> "$comment_file" - echo "" >> "$comment_file" echo "Changed properties: ${changed_props_summary}" >> "$comment_file" echo "" >> "$comment_file" - local dep_count - dep_count=$(echo "$dep_ids" | tr ',' '\n' | wc -l | tr -d ' ') - echo "
Modules affected by dependency changes (${dep_count})" >> "$comment_file" + fi + if [ -n "$managed_deps_summary" ]; then + echo "Changed managed dependencies: ${managed_deps_summary}" >> "$comment_file" echo "" >> "$comment_file" - echo "$dep_ids" | tr ',' '\n' | while read -r m; do - echo "- \`$m\`" >> "$comment_file" - done + fi + if [ -n "$managed_plugins_summary" ]; then + echo "Changed managed plugins: ${managed_plugins_summary}" >> "$comment_file" echo "" >> "$comment_file" - echo "
" >> "$comment_file" + fi + local dep_count + dep_count=$(echo "$dep_ids" | tr ',' '\n' | wc -l | tr -d ' ') + echo "
Modules affected by dependency changes (${dep_count})" >> "$comment_file" + echo "" >> "$comment_file" + echo "$dep_ids" | tr ',' '\n' | while read -r m; do + echo "- \`$m\`" >> "$comment_file" + done + echo "" >> "$comment_file" + echo "
" >> "$comment_file" + if [ -n "$managed_deps_summary" ] || [ -n "$managed_plugins_summary" ]; then + echo "" >> "$comment_file" + echo "> :microscope: Detected via [Maveniverse Scalpel](https://github.com/maveniverse/scalpel) effective POM comparison" >> "$comment_file" fi fi @@ -389,20 +526,27 @@ main() { done pl="${pl:1}" # strip leading comma - # Only analyze parent/pom.xml for dependency detection - # (matches original detect-test.sh behavior; detection improvements deferred to follow-up PR) + # Only analyze parent/pom.xml for grep-based dependency detection + # (matches original detect-test.sh behavior) if echo "$diff_body" | grep -q '^diff --git a/parent/pom.xml'; then pom_files="parent/pom.xml" fi - # ── Step 2: POM dependency analysis ── + # ── Step 2: POM dependency analysis (dual: grep + Scalpel) ── # Variables shared with analyzePomDependencies/findAffectedModules local dep_module_ids="" local all_changed_props="" - + # Scalpel results (not local — set by runScalpelDetection) + scalpel_module_ids="" + scalpel_module_paths="" + scalpel_props="" + scalpel_managed_deps="" + scalpel_managed_plugins="" + + # Step 2a: Grep-based detection (existing approach) if [ -n "$pom_files" ]; then echo "" - echo "Analyzing parent POM dependency changes..." + echo "Analyzing parent POM dependency changes (grep)..." while read -r pom_file; do [ -z "$pom_file" ] && continue @@ -421,6 +565,29 @@ main() { done <<< "$pom_files" fi + # Step 2b: Scalpel detection (parallel, for any pom.xml change) + # Scalpel uses effective POM model comparison — catches managed deps, + # plugin changes, and transitive impacts that grep misses. + if echo "$diff_body" | grep -q '^diff --git a/.*pom\.xml'; then + echo "" + echo "Running Scalpel POM analysis..." + runScalpelDetection + fi + + # Step 2c: Merge grep and Scalpel results (union, deduplicated) + if [ -n "$scalpel_module_ids" ]; then + dep_module_ids="${dep_module_ids:+${dep_module_ids},}${scalpel_module_ids}" + dep_module_ids=$(echo "$dep_module_ids" | tr ',' '\n' | sort -u | tr '\n' ',' | sed 's/,$//') + fi + if [ -n "$scalpel_props" ]; then + if [ -z "$all_changed_props" ]; then + all_changed_props="$scalpel_props" + else + # Merge and deduplicate property names + all_changed_props=$(printf '%s, %s' "$all_changed_props" "$scalpel_props" | tr ',' '\n' | sed 's/^ *//' | sort -u | tr '\n' ',' | sed 's/,$//' | sed 's/,/, /g') + fi + fi + # ── Step 3: Merge and deduplicate ── # Separate file-path modules into testable (has src/test) and pom-only. # Pom-only modules (e.g. "parent") are kept in the build list but must NOT @@ -458,7 +625,7 @@ main() { if [ -z "$final_pl" ]; then echo "" echo "No modules to test" - writeComment "incremental-test-comment.md" "" "" "" "" "" + writeComment "incremental-test-comment.md" "" "" "" "" "" "" "" exit 0 fi @@ -546,7 +713,7 @@ main() { # ── Step 5: Write comment and summary ── local comment_file="incremental-test-comment.md" - writeComment "$comment_file" "$pl" "$dep_module_ids" "$all_changed_props" "$testedDependents" "$extraModules" + writeComment "$comment_file" "$pl" "$dep_module_ids" "$all_changed_props" "$testedDependents" "$extraModules" "$scalpel_managed_deps" "$scalpel_managed_plugins" # Check for tests disabled in CI via @DisabledIfSystemProperty(named = "ci.env.name") local disabled_tests