gnodet-bot commented on code in PR #13086:
URL: https://github.com/apache/maven/pull/13086#discussion_r3983491941
##########
compat/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java:
##########
@@ -970,10 +970,11 @@ private ModelData readParentLocally(
version = candidateModel.getParent().getVersion();
}
- if (groupId == null
- || !groupId.equals(parent.getGroupId())
- || artifactId == null
- || !artifactId.equals(parent.getArtifactId())) {
+ if (parent.getRelativePath() != null
+ && (groupId == null
+ || !groupId.equals(parent.getGroupId())
+ || artifactId == null
+ || !artifactId.equals(parent.getArtifactId()))) {
StringBuilder buffer = new StringBuilder(256);
Review Comment:
🔴 **Critical — silent use of wrong parent POM.**
When `getRelativePath()` returns `null` (i.e. `<relativePath>` was not
specified in the POM), this new guard skips the GAV mismatch check entirely.
But `getParentPomFile()` (line 1055) still defaults `null` to `"../pom.xml"`
and resolves a candidate POM. If that candidate has a different
`groupId:artifactId`, control falls through to the version check (line 992) and
— if versions happen to match — the method returns `candidateModel` at line
1038, **silently using the wrong parent**.
The original code correctly returned `null` on mismatch, causing Maven to
fall back to repository resolution.
**What MNG-5146 actually asks for** is a better *warning message*, not
disabling the safety check. The Maven 4 impl already handles this correctly in
`mismatchRelativePathAndGA()` — when `getRelativePath() == null`, it changes
the severity from `FATAL` to `WARNING` but still detects the mismatch and
returns `null`:
```java
boolean warn = MODEL_VERSION_4_0_0.equals(childModel.getModelVersion())
|| childModel.getParent().getRelativePath() == null;
add(warn ? Severity.WARNING : Severity.FATAL, ...);
return null;
```
The compat layer should follow the same approach — keep the mismatch
detection but change severity to `WARNING` when `relativePath` was not
explicitly set:
```suggestion
if (groupId == null
|| !groupId.equals(parent.getGroupId())
|| artifactId == null
|| !artifactId.equals(parent.getArtifactId())) {
```
Then change the severity on line 987 from unconditional `Severity.WARNING`
to conditional based on whether `relativePath` was explicitly set. This
preserves the `return null` safety net while addressing MNG-5146's concern
about misleading warnings.
--
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]
