gnodet-bot commented on code in PR #13196:
URL: https://github.com/apache/maven/pull/13196#discussion_r4054398356
##########
impl/maven-impl/src/test/java/org/apache/maven/impl/model/ParentCycleDetectionTest.java:
##########
@@ -322,4 +325,79 @@ void
testMultipleModulesWithSameParentDoNotCauseCycle(@TempDir Path tempDir) thr
assertNotNull(resultA);
assertNotNull(resultB);
}
+
+ /**
+ * Reproduces GH-13191: maven-shade-plugin generates
dependency-reduced-pom.xml with
+ * {@code <relativePath>pom.xml</relativePath>} copied from the original
POM. When Maven 4
+ * processes this generated POM, the relativePath resolves to the
project's own pom.xml (wrong GA).
+ * This must produce a FATAL with a message pointing users to upgrade the
tool or use
+ * {@code --maven3-personality} as a workaround.
Review Comment:
⚠️ **Javadoc still documents the old wrong CLI flag.**
Line 334 says `{@code --maven3-personality}` — the same wrong form fixed in
the production message. This Javadoc will document incorrect behaviour after
merge.
```suggestion
* {@code -Dmaven.maven3Personality=true} as a workaround.
```
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelBuilder.java:
##########
@@ -1350,7 +1350,9 @@ private void mismatchRelativePathAndGA(Model childModel,
Parent parent, String g
+ (maven3Mode
? ". Please verify your project structure."
: ". Correct the <relativePath> value or
remove it to let Maven resolve the parent"
- + " from the repository.");
+ + " from the repository."
+ + " If this POM was generated by a
build tool (e.g. maven-shade-plugin),"
+ + " upgrade that tool or use
-Dmaven.maven3Personality as a temporary workaround.");
Review Comment:
⚠️ **Missing `=true` — `-Dmaven.maven3Personality` does not activate maven3
mode.**
`Features.mavenMaven3Personality()` calls
`Boolean.parseBoolean(val.toString())`. When users run `mvn
-Dmaven.maven3Personality` without a value, Maven sets the property to an empty
string `""`, and `Boolean.parseBoolean("")` returns `false` — the flag has no
effect. Users following this hint will hit the same FATAL and think the
workaround doesn't work.
The `Constants.MAVEN_MAVEN3_PERSONALITY = "maven.maven3Personality"`
constant doesn't help here; the issue is the missing `=true` suffix in the
user-facing string.
```suggestion
+ " upgrade that tool or use
-Dmaven.maven3Personality=true as a temporary workaround.");
```
##########
impl/maven-impl/src/test/java/org/apache/maven/impl/model/ParentCycleDetectionTest.java:
##########
@@ -322,4 +325,79 @@ void
testMultipleModulesWithSameParentDoNotCauseCycle(@TempDir Path tempDir) thr
assertNotNull(resultA);
assertNotNull(resultB);
}
+
+ /**
+ * Reproduces GH-13191: maven-shade-plugin generates
dependency-reduced-pom.xml with
+ * {@code <relativePath>pom.xml</relativePath>} copied from the original
POM. When Maven 4
+ * processes this generated POM, the relativePath resolves to the
project's own pom.xml (wrong GA).
+ * This must produce a FATAL with a message pointing users to upgrade the
tool or use
+ * {@code --maven3-personality} as a workaround.
+ */
+ @Test
+ void testWrongRelativePathInGeneratedPomIsFatalWithMaven3Hint(@TempDir
Path tempDir) throws IOException {
+ Files.createDirectories(tempDir.resolve(".mvn"));
+
+ // Simulates the original project pom.xml (the child artifact itself)
+ Path projectPom = tempDir.resolve("pom.xml");
+ Files.writeString(projectPom, """
+ <project xmlns="http://maven.apache.org/POM/4.0.0";>
+ <modelVersion>4.0.0</modelVersion>
+ <parent>
+ <groupId>org.apache.sling</groupId>
+ <artifactId>sling-bundle-parent</artifactId>
+ <version>57</version>
+ <relativePath/>
+ </parent>
+ <groupId>org.apache.sling</groupId>
+ <artifactId>org.apache.sling.models.impl</artifactId>
+ <version>2.0.3-SNAPSHOT</version>
+ <packaging>jar</packaging>
+ </project>
+ """);
+
+ // Simulates dependency-reduced-pom.xml generated by
maven-shade-plugin.
+ // It contains <relativePath>pom.xml</relativePath> which resolves to
the project's own
+ // pom.xml — a different GA than the declared parent.
+ Path reducedPom = tempDir.resolve("dependency-reduced-pom.xml");
+ Files.writeString(reducedPom, """
+ <project xmlns="http://maven.apache.org/POM/4.0.0";>
+ <modelVersion>4.0.0</modelVersion>
+ <parent>
+ <groupId>org.apache.sling</groupId>
+ <artifactId>sling-bundle-parent</artifactId>
+ <version>57</version>
+ <relativePath>pom.xml</relativePath>
+ </parent>
+ <groupId>org.apache.sling</groupId>
+ <artifactId>org.apache.sling.models.impl</artifactId>
+ <version>2.0.3-SNAPSHOT</version>
+ <packaging>jar</packaging>
+ </project>
+ """);
+
+ ModelBuilderRequest request = ModelBuilderRequest.builder()
+ .session(session)
+ .source(Sources.buildSource(reducedPom))
+ .requestType(ModelBuilderRequest.RequestType.BUILD_PROJECT)
+ .build();
+
+ ModelBuilderResult result;
+ try {
+ result = modelBuilder.newSession().build(request);
+ // If somehow no exception, still check problems below
+ } catch (ModelBuilderException e) {
+ result = e.getResult();
+ }
+
+ assertNotNull(result);
+
+ // Must produce a FATAL about the wrong relativePath — not silently
demoted to WARNING
+ boolean hasFatalWithHint = Stream.concat(Stream.of(result),
result.getChildren().stream())
+ .flatMap(r -> r.getProblemCollector().problems())
+ .anyMatch(p -> p.getSeverity() == BuilderProblem.Severity.FATAL
+ && p.getMessage().contains("relativePath")
+ && p.getMessage().contains("pom.xml")
+ && p.getMessage().contains("maven.maven3Personality"));
+ assertTrue(hasFatalWithHint, "Expected a FATAL about the wrong
relativePath with a --maven3-personality hint");
Review Comment:
💡 **Failure message still references the old flag.**
The assertion at line 400 now checks for `"maven.maven3Personality"`
(correct), but the `assertTrue` failure message on line 401 still says
`"--maven3-personality hint"`. Minor but inconsistent — if this assertion ever
fires the error message will be misleading.
```suggestion
assertTrue(hasFatalWithHint, "Expected a FATAL about the wrong
relativePath with a -Dmaven.maven3Personality=true hint");
```
--
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]
