ascheman commented on code in PR #11639:
URL: https://github.com/apache/maven/pull/11639#discussion_r2807519480
##########
impl/maven-core/src/main/java/org/apache/maven/project/SourceHandlingContext.java:
##########
@@ -67,19 +65,30 @@ record SourceKey(Language language, ProjectScope scope,
String module, Path dire
private final ModelBuilderResult result;
private final Set<SourceKey> declaredSources;
- SourceHandlingContext(
- MavenProject project,
- Path baseDir,
- Set<String> modules,
- boolean modularProject,
- ModelBuilderResult result) {
+ SourceHandlingContext(MavenProject project, Path baseDir,
ModelBuilderResult result) {
+ super(project);
this.project = project;
this.baseDir = baseDir;
Review Comment:
Note: PR #11702 (AC8/AC9 legacy directory handling) also modifies this
constructor and `SourceHandlingContext` in general. Whichever PR merges first
will create a conflict for the other. Just a heads-up so we can coordinate the
merge order.
##########
impl/maven-core/src/test/java/org/apache/maven/internal/transformation/impl/ConsumerPomBuilderTest.java:
##########
@@ -108,39 +116,50 @@ void testTrivialConsumer() throws Exception {
MavenProject project = new MavenProject(orgModel);
project.setOriginalModel(new org.apache.maven.model.Model(orgModel));
+ return project;
+ }
+
+ @Test
+ void testTrivialConsumer() throws Exception {
+ setRootDirectory("trivial");
+ Path file =
Paths.get("src/test/resources/consumer/trivial/child/pom.xml");
+
+ MavenProject project = getEffectiveModel(file);
Model model = builder.build(session, project,
Sources.buildSource(file));
assertNotNull(model);
+ assertNotNull(model.getDependencies());
}
@Test
void testSimpleConsumer() throws Exception {
- MavenExecutionRequest request =
InternalMavenSession.from(InternalSession.from(session))
- .getMavenSession()
- .getRequest();
-
request.setRootDirectory(Paths.get("src/test/resources/consumer/simple"));
+ MavenExecutionRequest request = setRootDirectory("simple");
request.getUserProperties().setProperty("changelist", "MNG6957");
-
Path file =
Paths.get("src/test/resources/consumer/simple/simple-parent/simple-weather/pom.xml");
- ModelBuilder.ModelBuilderSession mbs = modelBuilder.newSession();
-
InternalSession.from(session).getData().set(SessionData.key(ModelBuilder.ModelBuilderSession.class),
mbs);
- Model orgModel = mbs.build(ModelBuilderRequest.builder()
- .session(InternalSession.from(session))
- .source(Sources.buildSource(file))
-
.requestType(ModelBuilderRequest.RequestType.BUILD_PROJECT)
- .build())
- .getEffectiveModel();
-
- MavenProject project = new MavenProject(orgModel);
- project.setOriginalModel(new org.apache.maven.model.Model(orgModel));
+ MavenProject project = getEffectiveModel(file);
request.setRootDirectory(Paths.get("src/test/resources/consumer/simple"));
Model model = builder.build(session, project,
Sources.buildSource(file));
assertNotNull(model);
+ assertFalse(model.getDependencies().isEmpty());
assertTrue(model.getProfiles().isEmpty());
}
+ @Test
+ void testMultiModuleConsumer() throws Exception {
+ setRootDirectory("multi-module");
+ Path file =
Paths.get("src/test/resources/consumer/multi-module/pom.xml");
+
+ MavenProject project = getEffectiveModel(file);
+ Model model = builder.build(session, project,
Sources.buildSource(file));
+
+ assertNotNull(model);
+ assertNull(model.getBuild());
+ assertTrue(model.getDependencies().isEmpty());
+
assertFalse(model.getDependencyManagement().getDependencies().isEmpty());
+ }
Review Comment:
This test validates the default case (`preserveModelVersion=false`). I added
a local companion test `testMultiModuleConsumerPreserveModelVersion` that
verifies `<build>` is preserved when `preserveModelVersion=true`:
```java
@Test
void testMultiModuleConsumerPreserveModelVersion() throws Exception {
setRootDirectory("multi-module");
Path file =
Paths.get("src/test/resources/consumer/multi-module/pom.xml");
MavenProject project = getEffectiveModel(file);
Model model = getEffectiveModel(file).getModel().getDelegate();
model = Model.newBuilder(model, true).preserveModelVersion(true).build();
Model transformed = DefaultConsumerPomBuilder.transformPom(model,
project);
assertNotNull(transformed);
assertNotNull(transformed.getBuild());
assertTrue(transformed.getDependencies().isEmpty());
assertFalse(transformed.getDependencyManagement().getDependencies().isEmpty());
}
```
Would you consider adding this or a similar test to cover the
`preserveModelVersion` gate?
##########
impl/maven-core/src/main/java/org/apache/maven/internal/transformation/impl/DefaultConsumerPomBuilder.java:
##########
@@ -369,19 +371,33 @@ static Model transformPom(Model model, MavenProject
project) {
// raw to consumer transform
model = model.withRoot(false).withModules(null).withSubprojects(null);
- if (model.getParent() != null) {
- model = model.withParent(model.getParent().withRelativePath(null));
+ Parent parent = model.getParent();
+ if (parent != null) {
+ model = model.withParent(parent.withRelativePath(null));
+ }
+ var sources = new ProjectSourcesHelper(project);
+ if (sources.useModuleSourceHierarchy()) {
+ // Dependencies are dispatched by maven-jar-plugin in the POM
generated for each module.
+ model = model.withDependencies(null).withPackaging(POM_PACKAGING);
}
-
if (!preserveModelVersion) {
+ /*
+ * If tne <build> contains <source> elements, it is not compatible
with the Maven 4.0.0 model.
+ * Remove the full <build> element instead of removing only the
<sources> element, because the
+ * build without sources does not mean much. Reminder: this
removal can be disabled by setting
Review Comment:
```suggestion
* If the <build> contains <source> elements, it is not
compatible with the Maven 4.0.0 model.
```
--
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]
