[backport maven-4.0.x] Fix BOM consumer POM leaving property references unresolved - #12627
Conversation
Route all BOMs through buildBom() which uses getEffectiveModel() (fully
interpolated), instead of buildBomWithoutFlatten() which used
getRawModel(). The latter was incompatible with transformBom() which
strips parent and properties — leaving dangling ${...} references in
the consumer POM. Delete the now-dead buildBomWithoutFlatten() method.
Reported by Karl Heinz Marbaise during the 4.0.0-rc-6 vote.
Closes #12625
gnodet
left a comment
There was a problem hiding this comment.
LGTM ✅
Clean backport of PR #12625 to maven-4.0.x. The diff is byte-for-byte identical to the original, the fix is correct (routing BOMs through getEffectiveModel() to prevent dangling property references after transformBom() strips parent/properties), and the integration tests comprehensively cover the bug scenario.
Details:
- The backport is a perfect cherry-pick: production code changes and all new test/resource files are identical between #12625 (master) and #12627 (maven-4.0.x).
- The fix is sound:
buildBomWithoutFlatten()usedgetRawModel()(no interpolation), buttransformBom()strips both the parent reference and properties viaprune(). Using the un-interpolated model with a parent/properties-stripping transform is fundamentally incompatible. Routing throughbuildBom()which usesgetEffectiveModel()resolves this correctly. - The deleted
buildBomWithoutFlatten()method has exactly one call site and no other references, so its removal is clean. - Test coverage is thorough: three test methods covering default (no-flatten), flatten=true, and project-local-repo consumer POM paths.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of gnodet
The maven-4.0.x branch uses the older IT helper API where extractResources() returns File, newVerifier() takes String, and getArtifactPath() returns String. The test also needs a constructor with a version range. Adapt accordingly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
gnodet
left a comment
There was a problem hiding this comment.
Re-reviewed after new commit — still LGTM ✅
The new commit (cdcca89f) is a straightforward API adaptation of the integration test for the maven-4.0.x branch. The production code change remains identical to #12625.
The test adaptations cover 5 API differences between master and maven-4.0.x:
extractResources()returnsFileinstead ofPathnewVerifier()only acceptsStringgetArtifactPath()returnsStringinstead ofPath- Constructor requires version range
super("[4.0.0-rc-4,)") Files.readAllLinesreplaced withFiles.lines()try-with-resources
All adaptations match patterns used by existing tests on the maven-4.0.x branch (e.g., MavenITgh11427BomConsumerPomTest).
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of gnodet
|
@gnodet Please assign appropriate label to PR according to the type of change. |
Backport of #12625
Cherry-pick of #12625 onto
maven-4.0.x.Original PR: #12625 - Fix BOM consumer POM leaving property references unresolved
Original author: @gnodet
Target branch:
maven-4.0.xOriginal description
Fixes the BOM consumer POM builder leaving
${...}property references unresolved when consumer POM flattening is disabled (the default).Reported by Karl Heinz Marbaise during the 4.0.0-rc-6 vote.
DefaultConsumerPomBuilder.buildBomWithoutFlatten()usedgetRawModel()(no interpolation), but thentransformBom()strips both the parent and properties sections. The result is a consumer POM with dangling property references that no consumer can resolve.Fix: Route all BOMs through
buildBom()which usesgetEffectiveModel()(fully interpolated). Delete the now-deadbuildBomWithoutFlatten()method.🤖 Generated with Claude Code