ci: switch POM dependency detection to Scalpel-only#24579
Conversation
…-only Remove the grep-based POM dependency detection and the shadow comparison infrastructure, making Maveniverse Scalpel the sole mechanism for detecting modules affected by POM changes. The grep approach had known limitations that Scalpel resolves: - Managed deps without explicit <version> were missed - Plugin version changes were invisible to child modules - BOM imports were not linked to version properties - Transitive dependency changes were not detected - Non-property version changes in <dependencyManagement> were not caught Scalpel has been running in shadow mode alongside grep since April 2026, validating its detection across hundreds of PRs. The dual approach was designed as a gradual migration path (grep → dual → Scalpel-only), and this completes the final step. This removes ~330 lines of grep detection code and shadow comparison infrastructure that is no longer needed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
|
|
davsclaus
left a comment
There was a problem hiding this comment.
Nice cleanup completing the Scalpel migration. The code changes are clean — no stale variable references, correct function signatures, shell syntax passes bash -n. One documentation inaccuracy to fix (inline comment below).
This review does not replace specialized tools such as CodeRabbit or SonarCloud.
This review was generated by an AI agent (Claude Code on behalf of davsclaus) and may contain inaccuracies. Please verify all suggestions before applying.
| 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). | ||
|
|
||
| The grep-based script fetches the PR diff via the GitHub REST API (unchanged). Scalpel uses local git history to compare effective POM models and detect source file changes — the CI workflow progressively deepens the shallow clone (`50 → 200 → 1000 → full`) until the merge-base is reachable. Scalpel runs for **all PRs** (not just POM changes) so the shadow comparison covers source-only changes too, building confidence for a future switch to Scalpel-driven builds. Scalpel disables its built-in JGit fetch (`-Dscalpel.fetchBaseBranch=false`) because the CI workflow already pre-fetches the base branch with native git (Scalpel uses JGit 5.x which has limited shallow clone support). | ||
| Root `pom.xml` changes are excluded via `-Dscalpel.excludePaths=.github/**` because it contains build-infrastructure config (license plugin, checkstyle, etc.) that does not affect module compilation or test behavior. |
There was a problem hiding this comment.
This statement is inaccurate. -Dscalpel.excludePaths=.github/** excludes .github directory files from Scalpel's change detection — it does not exclude the root pom.xml.
After this PR, root pom.xml changes are no longer explicitly excluded. Scalpel will compare effective POM models and only flag modules with actual property/managed-dep differences, which is likely the correct behavior. But the documentation attributes this to a mechanism that doesn't do what it claims.
| Root `pom.xml` changes are excluded via `-Dscalpel.excludePaths=.github/**` because it contains build-infrastructure config (license plugin, checkstyle, etc.) that does not affect module compilation or test behavior. | |
| Root `pom.xml` changes are handled naturally by Scalpel's effective POM comparison — only modules with actual property, managed dependency, or managed plugin differences are flagged. The `excludePaths=.github/**` setting excludes CI infrastructure files from Scalpel's source change detection. |
Summary
Remove the grep-based POM dependency detection and shadow comparison infrastructure. Maveniverse Scalpel becomes the sole mechanism for detecting modules affected by POM changes.
extractPomDiff,detectChangedProperties,findAffectedModules,analyzePomDependencies) — ~330 linesBackground: Why Scalpel-only is ready
The dual grep+Scalpel approach was designed as a gradual migration path:
Analysis of all successful builds since Scalpel was introduced
An investigation of all merged PRs since the Scalpel integration (April 2026) found:
For parent POM property changes (the grep approach's target):
${property}, plugin versions, BOM imports)Known grep limitations resolved by Scalpel:
<version>— modules inheriting versions via<dependencyManagement>without declaring<version>${property}</version>were missedparent/pom.xmlvia<pluginManagement>were invisible to child modules<dependencyManagement>edits without property substitution were not caughtFor container upgrade PRs (neither grep nor Scalpel involved):
.propertiesfiles (test-infra/*/src/main/resources/.../container.properties), not Maven<properties>— neither approach detects these-amdexpansion, which works correctly since PR ci: fix incremental build to test dependents of test-infra modules #22514 (April 16, 2026)Historical context: the
src/test→src/mainfixThe investigation also uncovered a historical regression. On March 27, 2026, the script rewrite introduced a
src/testdirectory check for-amdeligibility. Since test-infra modules only havesrc/main(nosrc/test), they were misclassified as "pom-only" and their downstream component modules were silently dropped from the CI reactor. ~18 container upgrade PRs merged without downstream integration tests between March 27 and April 16, 2026 — including the Infinispan 16.2 upgrade (#22316) that later required a revert (#22954) due to flaky tests that were never caught by CI. This was fixed in PR #22514 by changing the check tosrc/main.Test plan
bash -nsyntax checkpaths-ignorefilter may skip the build — a/component-testrun or manual workflow dispatch can validate)Claude Code on behalf of gnodet
🤖 Generated with Claude Code