Skip to content

[MNG-8287] Dependencies should always use consumer pom - #12744

Open
Hiteshsai007 wants to merge 4 commits into
apache:masterfrom
Hiteshsai007:mng-8287-consumer-pom-dependencies
Open

[MNG-8287] Dependencies should always use consumer pom#12744
Hiteshsai007 wants to merge 4 commits into
apache:masterfrom
Hiteshsai007:mng-8287-consumer-pom-dependencies

Conversation

@Hiteshsai007

@Hiteshsai007 Hiteshsai007 commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Resolves #10535 (MNG-8287)

Maven 4 introduces support for activating profiles based on the project's <packaging> type. However, when a dependency built with Maven 4 is consumed by tools such as Maven 3, Gradle, or IDEs that do not understand the 4.1.0+ <packaging> activation condition, dependency resolution can differ between tools.

This change updates consumer POM generation to resolve packaging-based profile activation before the consumer POM is published, ensuring that downstream consumers see a consistent and predictable set of dependencies.

How this solves the problem

The consumer POM generation process in DefaultConsumerPomBuilder now explicitly handles profiles activated by <packaging>:

  1. Inline matching profiles

    When a profile is activated by <packaging> and the packaging value matches the current project's packaging, the profile's relevant contents are merged directly into the main consumer POM, including:

    • Dependencies
    • Dependency management
    • Repositories
  2. Remove resolved packaging activation

    After inlining a matching profile:

    • If <packaging> is the only activation condition, the profile is completely removed from the consumer POM.
    • If the profile has multiple activation conditions (for example, <packaging> and <jdk>), the profile remains in the consumer POM with its other activation conditions intact, while the already-resolved <packaging> condition is removed.
  3. Preserve non-matching profiles

    Profiles activated by a packaging type that does not match the current project's packaging are left unchanged in the consumer POM.

Why this matters

Packaging-based profile activation is understood by Maven 4, but older consumers and other build tools may not understand the <packaging> activation element introduced with model version 4.1.0.

By resolving matching packaging-based profiles during consumer POM generation, the published consumer POM contains the effective dependency information needed by downstream tools. This ensures that consumers using older Maven versions, Gradle, or IDE tooling see the same effective dependencies without having to understand or resolve Maven 4's packaging-based activation themselves.

Testing

Added comprehensive unit tests in ConsumerPomBuilderTest covering:

  • Matching packaging profiles are correctly inlined.
  • Profiles whose only activation condition is <packaging> are removed after resolution.
  • Profiles with mixed activation conditions retain their other conditions while removing <packaging>.
  • Non-matching packaging profiles are preserved unchanged.

Checklist

  • Your pull request should address just one issue, without pulling in other changes.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Each commit in the pull request should have a meaningful subject line and body.
  • Write unit tests that match behavioral changes, where the tests fail if the changes to the runtime are not applied.
  • Run mvn verify to make sure basic checks pass.
  • You have run the Core IT successfully.

To make clear that you license your contribution under the Apache License Version 2.0, January 2004, you have to acknowledge this by using the following checkbox.

  • I hereby declare this contribution to be licenced under the Apache License Version 2.0, January 2004
  • In any other case, please file an Apache Individual Contributor License Agreement.

When generating a consumer POM, profiles activated by packaging that match the current project's packaging should have their content inlined into the consumer POM and the activation should be removed. This ensures consistent dependency resolution across different tools.

@gnodet gnodet left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for working on this, @Hiteshsai007! The overall direction of inlining packaging-activated profiles in the consumer POM is architecturally sound — BUILD_CONSUMER does not activate profiles (isBuildRequestWithActivation returns false), so explicit handling is needed. A few issues to address before this can be merged:


🔴 High: Test assertion is incorrect (likely never executed)

In ConsumerPomBuilderTest (line ~325), the assertion:

assertEquals(1, transformed.getDependencies().size())

should be assertEquals(2, ...). The profileMixed has packaging="jar" which matches the model's packaging, so dep2 is also inlined alongside dep1 from profileMatching. Both are added to additionalDeps, yielding size 2.


🔴 High: Mixed-activation profiles (packaging + other conditions) handled incorrectly

When a profile has packaging=jar AND jdk=11 (AND semantics in Maven 4), the current code:

  1. Inlines all dependencies into model.dependencies unconditionally
  2. Keeps the profile with the remaining jdk=11 activation, still containing those same dependencies

This breaks AND semantics — the dependencies become unconditional regardless of JDK version. When jdk=11 is active, they appear twice (once from inlining, once from profile activation).

Fix: For mixed-activation profiles, do NOT inline the content. Only strip the <packaging> activation condition; the remaining conditions should still gate the profile's content. Inlining should only happen when packaging is the sole activation condition.


🟡 Medium: Inlined dependencies bypass scope filtering

buildNonPom calls buildEffectiveModel (which filters out test/provided/system-scoped dependencies), then calls transformNonPominlinePackagingActivatedProfiles. Dependencies moved from profiles to model.dependencies at this point skip the scope filter — a test-scoped dependency in a packaging-activated profile would leak into the consumer POM.


🟡 Medium: Non-matching profiles should be dropped

Profiles with packaging activation for a different type (e.g., packaging=war in a jar project) are kept as-is in the consumer POM. These can never activate (consumer POM has a fixed packaging) and their <packaging> activation element is a 4.1.0 feature that prevents model version downgrade to 4.0.0 — the very problem this PR aims to solve. These dead-code profiles should be removed.


Suggestions:

  • For mixed-activation profiles: strip only the <packaging> condition, keep content gated by remaining conditions, don't inline
  • For non-matching profiles: drop them entirely from the consumer POM
  • Add scope filtering for inlined dependencies in the non-POM path
  • Add integration tests using actual POM files in src/test/resources/consumer/ (following the pattern of testTrivialConsumer)

The architectural insight is correct — just needs refinement in the implementation details. Looking forward to the next iteration! 🙂

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

Claude Code on behalf of gnodet

- Inline packaging-activated profiles during transformPom for non-flattened builds

- Drop profiles activated by non-matching packaging entirely

- Update ConsumerPomBuilderTest to enable flattening and assert on dropped profiles

Signed-off-by: Hitesh <hiteshsaibv.24cs@saividya.ac.in>
@Hiteshsai007

Copy link
Copy Markdown
Contributor Author

Addressed Review Feedback & Fixes

Thanks for the feedback! I have pushed a new commit to address the review comments and resolve the integration test issues.

Here is a summary of the changes:

1. Filter Out Non-Transitive Scope Dependencies

As requested, the inline logic now intentionally filters out dependencies with non-transitive scopes (like test and provided) using DefaultConsumerPomBuilder::hasDependencyScope. This ensures that dependencies originating from inlined packaging-activated profiles do not inadvertently leak into the Consumer POM if they aren't meant for transitive consumption.

2. Handle Non-Flattened Consumer POM Generation

Previously, when the maven.consumer.pom.flatten property was disabled (which is the default), the Consumer POM generation fell back to transformPom(), bypassing the new packaging-profile handling logic entirely.

  • I added a call to inlinePackagingActivatedProfiles directly inside transformPom() so that packaging activation is consistently resolved and stripped. This ensures that the generated POM can still be safely downgraded to 4.0.0 for backwards compatibility with Maven 3 consumers, even when flattening is skipped.

3. Drop Profiles with Non-Matching Packaging

The logic was refined to explicitly drop profiles that have packaging activation criteria that do not match the project's current packaging. Since a consumer resolving the artifact will always see its fixed packaging (e.g., jar), a profile activated solely by a mismatched packaging (e.g., war) will never evaluate to true and is correctly discarded.

4. Expanded Test Coverage

I've updated ConsumerPomBuilderTest and its corresponding pom.xml to fully validate these scenarios:

  • The integration test explicitly sets maven.consumer.pom.flatten=true in the MavenExecutionRequest session to force the full interpolation and flattening pipeline.
  • It asserts that test-scoped dependencies (e.g., slf4j-simple) from matching profiles are stripped.
  • It asserts that profiles with non-matching packaging conditions are completely dropped from the final Consumer POM.
  • It asserts that "mixed-activation" profiles retain their non-packaging conditions while properly stripping the <packaging> field.

@Hiteshsai007
Hiteshsai007 requested a review from gnodet August 13, 2026 16:07

@gnodet gnodet left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the thorough update, @Hiteshsai007! Great progress — all four previous findings are properly addressed:

✅ Mixed-activation profiles now correctly strip only the packaging condition without inlining content
✅ Scope filtering applied to inlined dependencies (test-scoped deps filtered out)
✅ Non-matching profiles are now dropped
✅ Test assertion is correct

However, the second commit introduces a new issue:


🔴 Parent POM profiles silently dropped in transformPom

transformPom now unconditionally calls inlinePackagingActivatedProfiles(model, model.getPackaging()). For a parent POM (packaging=pom), a profile with <activation><packaging>jar</packaging></activation> does not match "pom", so it enters the non-matching branch and is silently dropped.

These profiles are designed to activate in child projects that have packaging=jar. By dropping them from the consumer POM, child projects consuming this parent from a repository will never see the profile-based dependencies.

This is a behavioral regression: before this PR, transformPom did not touch profiles at all, so packaging-activated profiles were preserved as-is in the consumer POM.

Fix: Either skip calling inlinePackagingActivatedProfiles in transformPom (which was the correct behavior before — our original review explicitly identified this omission as a false positive), or handle the parent-POM case differently by preserving non-matching profiles instead of dropping them.


🟡 Same issue in transformBom

transformBom has the same pattern. BOMs always have packaging=pom, so child-targeted profiles (e.g., <packaging>jar</packaging>) are dropped. The practical impact is lower since BOMs consumed via <scope>import</scope> only import dependency management, not profiles — but the code bug is still present.


🟡 Missing test coverage for parent POM path

The integration test (testPackagingActivatedProfiles) uses packaging=jar, which routes through buildNonPomtransformNonPom. No test exercises the parent POM scenario where packaging=pom is passed to inlinePackagingActivatedProfiles, which is the scenario where the regression manifests.


Suggestion: The simplest fix would be to guard the call in transformPom:

if (!POM_PACKAGING.equals(packaging)) {
    model = inlinePackagingActivatedProfiles(model, packaging);
}

And similarly in transformBom, or remove the call there entirely.

Getting close! 🙂

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

Claude Code on behalf of gnodet

@Hiteshsai007

Copy link
Copy Markdown
Contributor Author

Thanks for catching the parent-POM/BOM regression. Addressed in 6a10a22.

Packaging-profile resolution now runs only for non-POM consumer artifacts. transformPom and transformBom preserve profiles unchanged, so a parent or BOM can retain a profile targeting a child with packaging=jar.

Added a parent-POM regression fixture that verifies the jar-activated profile and its dependency are preserved. The existing non-POM test now builds with the same BUILD_CONSUMER request type used in production, continuing to verify inlining, mixed activation handling, and removal of non-matching profiles.

Verified with:
mvn -pl impl/maven-core -DskipITs -Dtest=ConsumerPomBuilderTest test

@Hiteshsai007
Hiteshsai007 requested a review from gnodet August 14, 2026 05:42

@gnodet gnodet left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All previous findings addressed — nice work across three iterations, @Hiteshsai007! 🎉

Round 2 fixes confirmed:
transformPom and transformBom no longer call inlinePackagingActivatedProfiles — parent POM profiles targeting child packagings are preserved as intended
✅ New testParentPomPackagingActivatedProfilesArePreserved test validates the regression scenario
testPackagingActivatedProfiles refactored to test through transformNonPom directly with the correct BUILD_CONSUMER request type

Minor observations (non-blocking):

  • The method-level Javadoc for inlinePackagingActivatedProfiles could document that non-matching profiles are dropped and that mixed-activation profiles have content preserved (not inlined). The inline comments at lines ~492 and ~504 do explain this, so it's a completeness suggestion only.
  • A parallel test for the transformBom path (similar to the parent POM test) would strengthen confidence, though the fix was a simple call removal so the risk is minimal.

The approach is clean and correct: packaging profile resolution runs only in transformNonPom where the packaging type is fixed, while parent POMs and BOMs preserve profiles as-is for child consumption. The stripPackagingActivation helper is thorough, and scope filtering properly prevents non-transitive dependencies from leaking into consumer POMs.

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 gnodet left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

APPROVE — Well-implemented consumer POM packaging profile resolution.

Three review iterations with the maintainer have addressed all major concerns. The code correctly inlines matching packaging-activated profiles, drops non-matching ones, and preserves mixed-activation profiles with only the packaging condition stripped.

The approach is architecturally sound — only transformNonPom calls the new logic, while transformPom and transformBom are untouched, preserving parent POM profiles for child consumption.

Minor observations (non-blocking):

  1. Duplicate dependency guard (low): Inlined dependencies are appended without deduplication at line 505. If a packaging-only profile re-declares a dependency already in the model, the consumer POM would contain duplicates. A defensive key-based check could prevent surprise.

  2. Test coverage for DM/repos paths (low): The unit test testInlinePackagingActivatedProfiles covers dependency inlining and profile stripping but does not exercise the dependencyManagement or repositories inlining paths separately.

  3. Javadoc completeness (low): The inlinePackagingActivatedProfiles Javadoc documents inlining and mixed-activation behavior but does not mention that non-matching profiles are dropped entirely.

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

Claude Code on behalf of Guillaume Nodet

- Add key-based deduplication when merging inlined dependencies,
  managed dependencies, and repositories from packaging-activated
  profiles to prevent duplicates in the consumer POM
- Add unit tests for dependency management and repository inlining
  paths in inlinePackagingActivatedProfiles
- Add unit test for duplicate dependency guard (model deps take
  precedence over profile duplicates)
- Widen transformBom visibility to package-private to match
  transformNonPom and transformPom

Signed-off-by: Hitesh <hiteshsaibv.24cs@saividya.ac.in>
@Hiteshsai007
Hiteshsai007 requested a review from gnodet August 17, 2026 04:53
@Hiteshsai007

Copy link
Copy Markdown
Contributor Author

Thanks for the detailed review and approval, @gnodet! I've addressed the three non-blocking observations in the latest commit:

1. Duplicate dependency guard ✅

Replaced the naive List.addAll() merging with key-based deduplication using LinkedHashMap for all three merge paths in inlinePackagingActivatedProfiles:

  • Dependencies: Deduplicated by groupId:artifactId:type:classifier — existing model dependencies take precedence over profile duplicates via putIfAbsent
  • Managed dependencies: Same key-based deduplication
  • Repositories: Deduplicated by repository id

2. Test coverage for DM/repos paths ✅

Added three new unit tests to exercise the previously untested merge paths:

  • testInlinePackagingActivatedProfilesDependencyManagement — verifies dependency management entries are correctly inlined from packaging-only profiles
  • testInlinePackagingActivatedProfilesRepositories — verifies repositories are correctly inlined
  • testInlinePackagingActivatedProfilesDeduplication — verifies the duplicate guard (model dependency at version 1.0 takes precedence over profile duplicate at version 2.0)

3. Javadoc completeness ✅

This was already documented in the existing Javadoc — the method-level doc at lines 475–477 explicitly states: "Profiles with a non-matching packaging activation are dropped entirely, since they can never activate for this artifact's fixed packaging and their presence would block model version downgrade to 4.0.0."
Additionally, I widened transformBom visibility from private static to static (package-private) to match transformNonPom and transformPom, fixing a pre-existing compilation error in testBomPackagingActivatedProfilesArePreserved.
All 14 tests pass. 🟢

gnodet added a commit to gnodet/maven that referenced this pull request Aug 17, 2026
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.

[MNG-8287] Dependencies should always use consumer pom

2 participants