Skip to content

Conversation

@gnodet
Copy link
Contributor

@gnodet gnodet commented Nov 18, 2025

Description

This PR adds validation to prevent creating consumer POMs when mixins are present without flattening enabled.

Changes

  • Added validation in DefaultConsumerPomBuilder: Checks for mixins when flattening is disabled and throws MavenException with a helpful error message
  • Added integration test MavenITgh11456MixinsConsumerPomTest with two scenarios:
    1. testMixinsWithoutFlattening: Verifies build fails with proper error message
    2. testMixinsWithFlattening: Verifies build succeeds and mixins are removed from consumer POM when flattening is enabled
    3. testMixinsWithPreserveModelerVersion: Verifies that mixins are kept when preserving the modelVersion in the consumer POM.
  • Created test resources: POMs using mixins with local repository containing mixin artifacts

Problem

Mixins require model version 4.2.0 and cannot be part of the consumer POM. When consumer POM flattening is disabled (maven.consumer.pom.flatten=false), mixins would remain in the consumer POM, which is invalid.

Solution

Maven now fails early with a clear error message that guides users to either:

  • Enable flattening by setting maven.consumer.pom.flatten=true, or
  • Set the preserve.model.version=true flag on the POM, or
  • Remove mixins from their POM

Testing

  • Full build passes: mvn clean install -DskipTests
  • Code formatting verified: mvn spotless:check
  • Integration tests created to verify both failure and success scenarios

Fixes #11456


Pull Request opened by Augment Code with guidance from the PR author

@gnodet gnodet added this to the 4.1.0 milestone Nov 18, 2025
…attening

Maven now fails with a clear error message when a POM contains mixins
but consumer POM flattening is disabled. Mixins require model version 4.2.0
and cannot be part of the consumer POM, so they must be removed during
transformation through flattening.

Changes:
- Added validation in DefaultConsumerPomBuilder to check for mixins when
  flattening is disabled and throw MavenException with helpful message
- Added integration test MavenITgh11456MixinsConsumerPomTest with two scenarios:
  1. testMixinsWithoutFlattening: Verifies build fails with proper error message
  2. testMixinsWithFlattening: Verifies build succeeds and mixins are removed
     from consumer POM when flattening is enabled
- Created test resources with POMs using mixins and local repository

The error message guides users to either enable flattening by setting
maven.consumer.pom.flatten=true or remove mixins from their POM.

Fixes apache#11456
@gnodet gnodet force-pushed the gh-11456-mixins-consumer-pom branch from 40f09e3 to d566cc1 Compare November 18, 2025 14:05
@gnodet gnodet changed the title [GH-11456] Add validation for mixins in consumer POM without flattening Add validation for mixins in consumer POM without flattening Nov 18, 2025
@gnodet gnodet changed the title Add validation for mixins in consumer POM without flattening Add validation for mixins in consumer POM without flattening (fixes #11456) Nov 18, 2025
@gnodet gnodet force-pushed the gh-11456-mixins-consumer-pom branch from 9318f82 to 2639425 Compare November 26, 2025 10:50
Copy link
Member

@cstamas cstamas left a comment

Choose a reason for hiding this comment

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

Note: I keep coming back to this, but we may tie these features to "maven level" (model version?) instead to have these freely (and independent) flags for ON/OFF them?

@gnodet
Copy link
Contributor Author

gnodet commented Nov 26, 2025

Note: I keep coming back to this, but we may tie these features to "maven level" (model version?) instead to have these freely (and independent) flags for ON/OFF them?

This is purely bulld time, as it's used when unit testing Maven 4 plugins. The flag has been added to avoid having to escape XML-reserved chars in some cases (<, >, ', &...), that's all. It's on by default, so that plugins migrating from 4.0 to 4.1 may have to adapt if they're using such characters in their values.

@gnodet
Copy link
Contributor Author

gnodet commented Nov 26, 2025

Note: I keep coming back to this, but we may tie these features to "maven level" (model version?) instead to have these freely (and independent) flags for ON/OFF them?

This is purely bulld time, as it's used when unit testing Maven 4 plugins. The flag has been added to avoid having to escape XML-reserved chars in some cases (<, >, ', &...), that's all. It's on by default, so that plugins migrating from 4.0 to 4.1 may have to adapt if they're using such characters in their values.

Sorry, I mixed the PRs.
What do you want to tie to maven level ? We disabled flattening by default in rc-5 because the behaviour is less controlable. This is currently incompatible with mixins. Are you suggesting we enable back flattening for 4.1.0 ? @rmannibucau

@gnodet
Copy link
Contributor Author

gnodet commented Nov 26, 2025

Note: I keep coming back to this, but we may tie these features to "maven level" (model version?) instead to have these freely (and independent) flags for ON/OFF them?

This is purely bulld time, as it's used when unit testing Maven 4 plugins. The flag has been added to avoid having to escape XML-reserved chars in some cases (<, >, ', &...), that's all. It's on by default, so that plugins migrating from 4.0 to 4.1 may have to adapt if they're using such characters in their values.

Sorry, I mixed the PRs. What do you want to tie to maven level ? We disabled flattening by default in rc-5 because the behaviour is less controlable. This is currently incompatible with mixins. Are you suggesting we enable back flattening for 4.1.0 ? @rmannibucau

In any case, this can be done in a follow-up PR. The point is that mixins require flattening so that they can be translated into 4.0.0 model.

@gnodet
Copy link
Contributor Author

gnodet commented Nov 27, 2025

FTR, I've slightly modified the check to take into account the preserveModelVersion which can be used to keep the consumer POM with a modelVersion of 4.2.0, in which case we can safely keep mixin elements.

@gnodet gnodet merged commit 8f63dcc into apache:master Nov 27, 2025
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add ITs for mixins and flattened / non-flattened Consumer POM

3 participants