gnodet-bot commented on code in PR #13197:
URL: https://github.com/apache/maven/pull/13197#discussion_r4053163855
##########
impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/AbstractUpgradeStrategy.java:
##########
@@ -94,10 +143,86 @@ public final UpgradeResult apply(UpgradeContext context,
Map<Path, Document> pom
context.failure("Strategy execution failed: " + e.getMessage());
return UpgradeResult.failure(pomMap.keySet(), Set.of());
} finally {
+ effectiveModelCache = null;
+ sharedModelBuilderSession = null;
context.unindent();
}
}
+ /**
+ * Pre-builds effective models for every POM in the reactor with a single
+ * {@code BUILD_PROJECT} pass and stores them in {@link
#effectiveModelCache}.
+ *
+ * <p>{@code BUILD_PROJECT} is used because it:
+ * <ul>
+ * <li>calls {@code loadFromRoot}, which populates {@code mappedSources}
for the
+ * whole reactor before any single effective model is assembled —
this is what
+ * makes Maven 4 coordinate inference work across modules;</li>
+ * <li>applies full profile activation (file, property, condition)
rather than
+ * the reduced set used by {@code BUILD_CONSUMER};</li>
+ * <li>produces child results (via {@code getChildren()}) that map
1-to-1 to the
Review Comment:
⚠️ **Wrong claim**: `getChildren()` does NOT map 1-to-1 to reactor modules
here. `ModelBuilderRequest.recursive` defaults to `false`; without
`.recursive(true)` the `if (request.isRecursive())` gate in
`DefaultModelBuilder.loadFilePom` never adds child results to the list.
`allResults()` only sees root, and this cache always has one entry for
multi-module projects.
Fix: add `.recursive(true)` to the request in `prebuildReactorModels`, and
update this bullet to match.
##########
impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/AbstractUpgradeStrategy.java:
##########
@@ -94,10 +143,86 @@ public final UpgradeResult apply(UpgradeContext context,
Map<Path, Document> pom
context.failure("Strategy execution failed: " + e.getMessage());
return UpgradeResult.failure(pomMap.keySet(), Set.of());
} finally {
+ effectiveModelCache = null;
+ sharedModelBuilderSession = null;
context.unindent();
}
}
+ /**
+ * Pre-builds effective models for every POM in the reactor with a single
+ * {@code BUILD_PROJECT} pass and stores them in {@link
#effectiveModelCache}.
+ *
+ * <p>{@code BUILD_PROJECT} is used because it:
+ * <ul>
+ * <li>calls {@code loadFromRoot}, which populates {@code mappedSources}
for the
+ * whole reactor before any single effective model is assembled —
this is what
+ * makes Maven 4 coordinate inference work across modules;</li>
+ * <li>applies full profile activation (file, property, condition)
rather than
+ * the reduced set used by {@code BUILD_CONSUMER};</li>
+ * <li>produces child results (via {@code getChildren()}) that map
1-to-1 to the
+ * reactor modules, so we can populate the cache in a single
pass.</li>
+ * </ul>
+ *
+ * <p>The root POM is identified as the shallowest (fewest name parts)
path in
+ * {@code pomPaths}.</p>
+ *
+ * <p>If the build fails (e.g. the project has no network access and an
external
+ * parent cannot be resolved), the cache is left empty and
+ * {@link #buildEffectiveModel} falls back to individual {@code
BUILD_EFFECTIVE}
+ * calls.</p>
+ *
+ * @param context the upgrade context (used for debug/warning logging)
+ * @param pomPaths the set of POM paths that make up the reactor
+ */
+ protected void prebuildReactorModels(UpgradeContext context, Set<Path>
pomPaths) {
+ if (pomPaths.isEmpty()) {
+ return;
+ }
+ Session s = getSession();
+ ModelBuilder modelBuilder = s.getService(ModelBuilder.class);
+ sharedModelBuilderSession = modelBuilder.newSession();
+
+ // Use the shallowest path as the root (fewest name elements).
+ Path rootPom = pomPaths.stream()
+ .min(java.util.Comparator.comparingInt(Path::getNameCount))
+ .orElseThrow();
+
+ context.debug("Pre-building reactor effective models from root: " +
rootPom);
+
+ try {
+ ModelBuilderRequest request = ModelBuilderRequest.builder()
+ .session(s)
+ .source(Sources.buildSource(rootPom))
+ .requestType(ModelBuilderRequest.RequestType.BUILD_PROJECT)
+ .build();
Review Comment:
⚠️ **Missing `.recursive(true)`**: Without this, `isRecursive()` returns
`false` and child module results are never added to `getChildren()`.
`allResults()` traversal only sees the root POM. The effective model cache is
populated with one entry. Add `.recursive(true)` here:
```suggestion
ModelBuilderRequest request = ModelBuilderRequest.builder()
.session(s)
.source(Sources.buildSource(rootPom))
.requestType(ModelBuilderRequest.RequestType.BUILD_PROJECT)
.recursive(true)
.build();
```
--
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]
