Hiteshsai007 commented on PR #12417:
URL: https://github.com/apache/maven/pull/12417#issuecomment-4932169617
Thanks for the thorough review @gnodet — all points addressed in the latest
force-push:
**1. Double interpolation fix (CI failure):**
Removed the second `interpolateModel()` call entirely. BOM properties are
now merged into the model **before** the first (and only) interpolation pass in
`readEffectiveModel()`, so `\${test.value}` escaping works correctly. This
fixes `MavenITmng3558PropertyEscapingTest`.
**2. Opt-in mechanism (semantic change):**
The feature is now **disabled by default** and must be explicitly enabled
via `-Dmaven.bom.import.properties=true`. This preserves Maven's existing BOM
import semantics (import-scope only imports dependency management) and avoids
breaking any existing project. A new `Constants.MAVEN_BOM_IMPORT_PROPERTIES`
constant with full Javadoc was added.
**3. Conflict warnings for multiple BOMs:**
When multiple BOMs define the same property key with different values, a
warning is now logged identifying both the conflicting BOM coordinates and the
conflicting values. First-declared BOM wins (consistent with dependency
management ordering).
**4. No unfiltered property pollution:**
Since the feature is opt-in, users explicitly choose to import BOM
properties. The model's own properties (including parent-inherited and
profile-injected) always take precedence over imported BOM properties.
**5. Tests:**
The critical CI failure (`MavenITmng3558PropertyEscapingTest`) is resolved
by the opt-in default. Comprehensive test coverage for the enabled feature will
require further discussion on test infrastructure for BOM resolution.
**6. Method rename:**
`doLoadDependencyManagement` → `doLoadBomModel` for consistency.
`loadBomModel` reverted back to `loadDependencyManagement` since
`importDependencyManagement` no longer needs the full model.
I agree that broader community discussion on `[email protected]` would
be valuable to determine the right long-term semantics. This implementation
provides a safe, opt-in starting point.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
