Skip to content

Fix #12640: BOM consumer POM must not include inherited dependency management - #12641

Open
gnodet wants to merge 1 commit into
masterfrom
fix/12640
Open

Fix #12640: BOM consumer POM must not include inherited dependency management#12641
gnodet wants to merge 1 commit into
masterfrom
fix/12640

Conversation

@gnodet

@gnodet gnodet commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Fixes the BOM packaging issue where the consumer POM included all inherited dependency management entries from the parent chain
  • Uses a hybrid raw-model + effective-model approach: the raw model (no inheritance) determines which entries belong in the BOM, while the effective model provides resolved/interpolated values
  • Import-scoped BOM entries are preserved as references (not flattened) with versions interpolated from the project's properties
  • Adds an IT reproducing Karl's scenario: parent declares dep management entries, BOM module declares only its own modules — verifies inherited entries don't leak into the consumer BOM

Example

Input

Parent POM — declares ext-lib-a and ext-lib-b in dependency management:

<groupId>org.example</groupId>
<artifactId>parent</artifactId>
<version>1.0.0-SNAPSHOT</version>
<packaging>pom</packaging>

<properties>
  <ext.lib.version>2.0.0</ext.lib.version>
</properties>

<dependencyManagement>
  <dependencies>
    <dependency>
      <groupId>org.example</groupId>
      <artifactId>ext-lib-a</artifactId>
      <version>${ext.lib.version}</version>
    </dependency>
    <dependency>
      <groupId>org.example</groupId>
      <artifactId>ext-lib-b</artifactId>
      <version>${ext.lib.version}</version>
    </dependency>
  </dependencies>
</dependencyManagement>

BOM module — declares only mod-1 and mod-2:

<parent>
  <groupId>org.example</groupId>
  <artifactId>parent</artifactId>
  <version>1.0.0-SNAPSHOT</version>
</parent>

<artifactId>bom</artifactId>
<packaging>bom</packaging>

<dependencyManagement>
  <dependencies>
    <dependency>
      <groupId>org.example</groupId>
      <artifactId>mod-1</artifactId>
      <!-- version inferred from reactor -->
    </dependency>
    <dependency>
      <groupId>org.example</groupId>
      <artifactId>mod-2</artifactId>
    </dependency>
  </dependencies>
</dependencyManagement>

Previous output (broken — after 4032fc8)

The consumer POM included all 4 entries — the 2 declared + 2 inherited from parent:

<project>
  <modelVersion>4.0.0</modelVersion>
  <groupId>org.example</groupId>
  <artifactId>bom</artifactId>
  <version>1.0.0-SNAPSHOT</version>
  <packaging>pom</packaging>
  <dependencyManagement>
    <dependencies>
      <dependency>
        <groupId>org.example</groupId>
        <artifactId>mod-1</artifactId>
        <version>1.0.0-SNAPSHOT</version>
      </dependency>
      <dependency>
        <groupId>org.example</groupId>
        <artifactId>mod-2</artifactId>
        <version>1.0.0-SNAPSHOT</version>
      </dependency>
      <!-- ❌ LEAKED from parent — not declared in this BOM -->
      <dependency>
        <groupId>org.example</groupId>
        <artifactId>ext-lib-a</artifactId>
        <version>2.0.0</version>
      </dependency>
      <!-- ❌ LEAKED from parent -->
      <dependency>
        <groupId>org.example</groupId>
        <artifactId>ext-lib-b</artifactId>
        <version>2.0.0</version>
      </dependency>
    </dependencies>
  </dependencyManagement>
</project>

Current output (fixed — this PR)

The consumer POM contains only the 2 entries declared in the BOM, with versions resolved:

<project>
  <modelVersion>4.0.0</modelVersion>
  <groupId>org.example</groupId>
  <artifactId>bom</artifactId>
  <version>1.0.0-SNAPSHOT</version>
  <packaging>pom</packaging>
  <dependencyManagement>
    <dependencies>
      <dependency>
        <groupId>org.example</groupId>
        <artifactId>mod-1</artifactId>
        <version>1.0.0-SNAPSHOT</version>
      </dependency>
      <dependency>
        <groupId>org.example</groupId>
        <artifactId>mod-2</artifactId>
        <version>1.0.0-SNAPSHOT</version>
      </dependency>
      <!-- ✅ No inherited entries — only what the BOM declared -->
    </dependencies>
  </dependencyManagement>
</project>

Context

The previous fix (4032fc8) switched from getRawModel() to getEffectiveModel() to solve dangling ${...} property references in the consumer BOM. That fixed property resolution but caused getEffectiveModel() to include ALL inherited dependency management entries from the parent POM chain — exactly the issue Karl reported.

The new filterToOwnDependencyManagement() method intersects both models:

  1. Raw model → set of declared dependency keys (no inheritance)
  2. Effective model → resolved values for non-import entries
  3. Import entries → preserved as BOM references with version resolved from project.getProperties()

Test plan

  • MavenITgh12640BomInheritedDepMgmtTest — verifies inherited entries don't leak (new)
  • MavenITBomConsumerPomPropertyResolutionTest — property resolution still works (existing, 3 tests)
  • MavenITgh11427BomConsumerPomTest — BOM packaging transform still works (existing)
  • MavenITmng8293BomImportFromReactor — reactor BOM import still works (existing)
  • MavenITConsumerPomBomFromSettingsRepoTest — settings repo BOM still works (existing)
  • 5 additional consumer POM ITs all pass with zero regressions

All 16 tests pass locally.

🤖 Generated with Claude Code

…nagement

BOM packaging was using getEffectiveModel() which includes all dependency
management entries inherited from the parent chain. This caused parent-declared
entries (and flattened BOM imports) to leak into the consumer BOM.

Use a hybrid approach: the raw model (no inheritance) determines WHICH entries
belong in the BOM, while the effective model provides resolved values. Import-
scoped entries are preserved as BOM references with versions interpolated from
the project properties, rather than being flattened.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@gnodet gnodet added this to the 4.0.0-rc-7 milestone Jul 31, 2026
@gnodet
gnodet marked this pull request as ready for review July 31, 2026 06:49

@gnodet gnodet left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. The hybrid raw-model + effective-model approach is the right architecture — using the raw model for declared entry identity and the effective model for resolved values correctly avoids both the inheritance leak and the property-resolution regression from #12625.

CI is fully green across all platforms and JDK versions.

Two suggestions for follow-up (non-blocking):

  1. interpolateVersion robustness — the hand-rolled property interpolator at line 262 handles project.getProperties(), ${project.version}, and ${project.groupId}, but misses ${project.artifactId}, system/environment properties, and nested references. The sibling PomInlinerTransformer in the same package uses the injected Interpolator service which handles all these cases. Consider using it for robustness.

  2. Test coverage for import-scoped BOM entries — the test uses plain dependency management entries, but the original report (#12640) specifically involves import-scoped BOM entries in the parent (junit-bom, assertj-bom) whose flattened contents leak into the consumer BOM. Adding a test variant with import-scoped entries would cover the exact reported scenario.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of gnodet

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants