gnodet commented on code in PR #12745:
URL: https://github.com/apache/maven/pull/12745#discussion_r3795529169
##########
impl/maven-core/src/main/java/org/apache/maven/internal/transformation/impl/DefaultConsumerPomBuilder.java:
##########
@@ -291,7 +291,7 @@ private static boolean hasDependencyScope(Dependency
dependency) {
} else {
scope = DependencyScope.forId(scopeId);
}
- return scope == null || !scope.isTransitive();
+ return scope != DependencyScope.COMPILE && scope !=
DependencyScope.RUNTIME && scope != DependencyScope.API;
Review Comment:
**Non-blocking — stale comments**: The `hasDependencyScope()` method now
explicitly checks for `COMPILE`, `RUNTIME`, and `API` instead of using
`isTransitive()`. The comments at lines 266 and 284 still say _"Only keep
transitive scopes"_ but `COMPILE` is no longer transitive. Consider updating
them to _"Only keep consumer-visible scopes (compile, api, runtime)"_ to match
the new logic.
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/resolver/DefaultArtifactDescriptorReader.java:
##########
@@ -345,6 +345,12 @@ private int getPolicy(RepositorySystemSession session,
Artifact a, ArtifactDescr
private void populateResult(InternalSession session,
ArtifactDescriptorResult result, Model model) {
ArtifactTypeRegistry stereotypes =
session.getSession().getArtifactTypeRegistry();
+ // Compute once whether compile-scoped dependencies should be remapped
to api (transitive)
+ // for backward compatibility. Use the declared modelVersion rather
than feature detection
+ // to respect the developer's explicit intent.
+ String declaredModelVersion = model.getModelVersion();
+ boolean remapCompileToApi = declaredModelVersion == null ||
declaredModelVersion.startsWith("4.0.");
+
for (Repository repository : model.getRepositories()) {
result.addRepository(session.toRepository(
session.getService(RepositoryFactory.class).createRemote(repository)));
Review Comment:
**Non-blocking — test coverage gap**: The `remapCompileToApi` logic computed
here and applied in `convert()` is the cornerstone of backward compatibility,
but it has no direct unit test. Consider adding a test in
`DefaultArtifactDescriptorReaderTest` that verifies:
- 4.0.0 modelVersion → `compile` scope is remapped to `api`
- 4.1.0 modelVersion → `compile` scope is left as-is
--
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]
