gnodet commented on code in PR #12745:
URL: https://github.com/apache/maven/pull/12745#discussion_r3794958624
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/resolver/DefaultArtifactDescriptorReader.java:
##########
@@ -420,11 +421,16 @@ private Dependency
convert(org.apache.maven.api.model.Dependency dependency, Art
exclusions.add(convert(exclusion));
}
+ String scope = dependency.getScope() != null ? dependency.getScope() :
"";
+ if ("compile".equals(scope) || "".equals(scope)) {
+ String modelVersion = new
org.apache.maven.model.v4.MavenModelVersion().getModelVersion(model);
+ if (modelVersion == null || modelVersion.startsWith("4.0.")) {
Review Comment:
**Performance + correctness**: `new
MavenModelVersion().getModelVersion(model)` is called per dependency inside
`convert()`, which runs in two loops (direct + managed deps). This creates N
objects and scans the entire model N times.
Additionally, `MavenModelVersion.getModelVersion()` performs *feature
detection* (scanning model fields), not declared-version reading. If a 4.1.0
POM uses only compile scope without other 4.1.0 features, this returns "4.0.0"
and incorrectly remaps compile→api.
Suggested fix: compute the version once in `populateResult()` using
`model.getModelVersion()` (declared version) and pass it to `convert()`:
```java
// In populateResult(), before the loops:
String modelVersion = model.getModelVersion();
// In convert():
if ("compile".equals(scope) || "".equals(scope)) {
if (modelVersion == null || modelVersion.startsWith("4.0.")) {
scope = "api";
}
}
```
--
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]
