gnodet-bot commented on code in PR #1125:
URL: https://github.com/apache/maven/pull/1125#discussion_r4002856093
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultPluginDependenciesResolver.java:
##########
@@ -79,11 +91,103 @@ public class DefaultPluginDependenciesResolver implements
PluginDependenciesReso
private final List<MavenPluginDependenciesValidator>
dependenciesValidators;
+ private final String mavenVersion;
+
+ private final Set<String> mavenGoneCoreGAs;
+
+ private final Set<String> mavenCoreGAs;
+
+ private final Map<Dependency, Set<String>> otherCoreGAVs;
+
+ private final List<Dependency> mavenManagedDependencies;
+
+ private final Dependency mavenCompat;
+
+ private final List<Exclusion> mavenGlobalExclusions;
+
@Inject
public DefaultPluginDependenciesResolver(
- RepositorySystem repoSystem,
List<MavenPluginDependenciesValidator> dependenciesValidators) {
+ RepositorySystem repoSystem,
+ List<MavenPluginDependenciesValidator> dependenciesValidators,
+ RuntimeInformation runtimeInformation) {
this.repoSystem = repoSystem;
this.dependenciesValidators = dependenciesValidators;
+ this.mavenVersion = runtimeInformation.getMavenVersion();
+ this.mavenGoneCoreGAs = Collections.unmodifiableSet(Stream.of(
+ "org.apache.maven:maven-artifact-manager",
+ "org.apache.maven:maven-plugin-descriptor",
+ "org.apache.maven:maven-plugin-registry",
+ "org.apache.maven:maven-profile",
+ "org.apache.maven:maven-project",
+ "org.apache.maven:maven-toolchain")
+ .collect(Collectors.toSet()));
+ this.mavenCoreGAs = Collections.unmodifiableSet(Stream.of(
+ "org.apache.maven:maven-artifact",
+ "org.apache.maven:maven-builder-support",
+ "org.apache.maven:maven-compat",
+ "org.apache.maven:maven-core",
+ "org.apache.maven:maven-embedder",
+ "org.apache.maven:maven-model",
+ "org.apache.maven:maven-model-builder",
+ "org.apache.maven:maven-model-transform",
+ "org.apache.maven:maven-plugin-api",
+ "org.apache.maven:maven-repository-metadata",
+ "org.apache.maven:maven-resolver-provider",
+ "org.apache.maven:maven-settings",
+ "org.apache.maven:maven-settings-builder",
+ "org.apache.maven:maven-slf4j-provider",
+ "org.apache.maven:maven-slf4j-wrapper",
+ "org.apache.maven:maven-toolchain-builder",
+ "org.apache.maven:maven-toolchain-model")
+ .collect(Collectors.toSet()));
+
+ // here we "align" other deps by fixing their version (for runtime
scope), and rest are (should be) provided
+ // anyway
+ Map<Dependency, Set<String>> otherCoreGAVs = new HashMap<>();
+ otherCoreGAVs.put(
+ new Dependency(
+ new
DefaultArtifact("org.eclipse.sisu:org.eclipse.sisu.inject:0.3.5"),
JavaScopes.RUNTIME),
+ Collections.singleton("org.sonatype.sisu:sisu-inject-bean"));
+ otherCoreGAVs.put(
+ new Dependency(new
DefaultArtifact("com.google.inject:guice:5.1.0"), JavaScopes.RUNTIME),
+ Collections.singleton("org.sonatype.sisu:sisu-guice"));
+
+ otherCoreGAVs.put(
+ new Dependency(
+ new
DefaultArtifact("org.eclipse.sisu:org.eclipse.sisu.plexus:0.3.5"),
JavaScopes.PROVIDED),
+ new HashSet<>(Arrays.asList(
+ "org.sonatype.spice:spice-inject-plexus",
+ "org.sonatype.sisu:sisu-inject-plexus",
+ "org.codehaus.plexus:plexus-container-default",
+ "plexus:plexus-container-default")));
+ otherCoreGAVs.put(
+ new Dependency(
+ new
DefaultArtifact("org.codehaus.plexus:plexus-classworlds:2.6.0"),
JavaScopes.PROVIDED),
+ Collections.singleton("classworlds:classworlds"));
+ this.otherCoreGAVs = Collections.unmodifiableMap(otherCoreGAVs);
+
+ List<Dependency> mavenCoreDependencies =
Collections.unmodifiableList(mavenCoreGAs.stream()
+ .map(s -> new Dependency(new DefaultArtifact(s + ":" +
mavenVersion), JavaScopes.PROVIDED))
+ .collect(Collectors.toList()));
+ this.mavenCompat = mavenCoreDependencies.stream()
+ .filter(d ->
"maven-compat".equals(d.getArtifact().getArtifactId()))
+ .findFirst()
+ .orElseThrow(() -> new RuntimeException("maven-compat not
found among Maven Core dependencies"));
+
Review Comment:
💡 Same Guava import issue — use `Stream.concat()`:
```suggestion
this.mavenGlobalExclusions =
Collections.unmodifiableList(Stream.concat(
mavenGoneCoreGAs.stream(),
otherCoreGAVs.values().stream().flatMap(Collection::stream))
.map(s -> {
int idx = s.indexOf(':');
String g = s.substring(0, idx);
String a = s.substring(idx + 1);
```
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultPluginDependenciesResolver.java:
##########
@@ -183,27 +292,56 @@ private DependencyNode resolveInternal(
pluginArtifact = toArtifact(plugin, session);
}
- DependencyFilter collectionFilter = new
ScopeDependencyFilter("provided", "test");
- DependencyFilter resolutionFilter =
AndDependencyFilter.newInstance(collectionFilter, dependencyFilter);
+ DependencySelector dependencySelector =
session.getDependencySelector();
+ DependencyFilter resolutionFilter =
+ AndDependencyFilter.newInstance(new
ScopeDependencyFilter("provided", "test"), dependencyFilter);
DependencyNode node;
try {
DefaultRepositorySystemSession pluginSession = new
DefaultRepositorySystemSession(session);
-
pluginSession.setDependencySelector(session.getDependencySelector());
+ pluginSession.setDependencySelector(dependencySelector);
pluginSession.setDependencyGraphTransformer(session.getDependencyGraphTransformer());
CollectRequest request = new CollectRequest();
request.setRequestContext(REPOSITORY_CONTEXT);
request.setRepositories(repositories);
- request.setRoot(new
org.eclipse.aether.graph.Dependency(pluginArtifact, null));
- for (Dependency dependency : plugin.getDependencies()) {
- org.eclipse.aether.graph.Dependency pluginDep =
- RepositoryUtils.toDependency(dependency,
session.getArtifactTypeRegistry());
+ request.setManagedDependencies(mavenManagedDependencies);
+ Dependency rootDependency = new Dependency(pluginArtifact,
null).setExclusions(mavenGlobalExclusions);
+ request.setRoot(rootDependency);
+
+ // plugin dependencies from POM
+ ArtifactDescriptorResult descriptor =
readArtifactDescriptor(trace, plugin, session, repositories);
Review Comment:
⚠️ **Redundant descriptor read.** This calls `readArtifactDescriptor()` a
second time for the same plugin — the first call happens in `resolve()` (the
method that runs before `resolveInternal`). Each call triggers a remote POM
fetch or at minimum a local repo lookup. The original code on master avoids
this by relying solely on `plugin.getDependencies()` (model-level deps), which
are already parsed and in memory.
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultPluginDependenciesResolver.java:
##########
@@ -79,11 +91,103 @@ public class DefaultPluginDependenciesResolver implements
PluginDependenciesReso
private final List<MavenPluginDependenciesValidator>
dependenciesValidators;
+ private final String mavenVersion;
+
+ private final Set<String> mavenGoneCoreGAs;
+
+ private final Set<String> mavenCoreGAs;
+
+ private final Map<Dependency, Set<String>> otherCoreGAVs;
+
+ private final List<Dependency> mavenManagedDependencies;
+
+ private final Dependency mavenCompat;
+
+ private final List<Exclusion> mavenGlobalExclusions;
+
@Inject
public DefaultPluginDependenciesResolver(
- RepositorySystem repoSystem,
List<MavenPluginDependenciesValidator> dependenciesValidators) {
+ RepositorySystem repoSystem,
+ List<MavenPluginDependenciesValidator> dependenciesValidators,
+ RuntimeInformation runtimeInformation) {
this.repoSystem = repoSystem;
this.dependenciesValidators = dependenciesValidators;
+ this.mavenVersion = runtimeInformation.getMavenVersion();
+ this.mavenGoneCoreGAs = Collections.unmodifiableSet(Stream.of(
+ "org.apache.maven:maven-artifact-manager",
+ "org.apache.maven:maven-plugin-descriptor",
+ "org.apache.maven:maven-plugin-registry",
+ "org.apache.maven:maven-profile",
+ "org.apache.maven:maven-project",
+ "org.apache.maven:maven-toolchain")
+ .collect(Collectors.toSet()));
+ this.mavenCoreGAs = Collections.unmodifiableSet(Stream.of(
+ "org.apache.maven:maven-artifact",
+ "org.apache.maven:maven-builder-support",
+ "org.apache.maven:maven-compat",
+ "org.apache.maven:maven-core",
+ "org.apache.maven:maven-embedder",
+ "org.apache.maven:maven-model",
+ "org.apache.maven:maven-model-builder",
+ "org.apache.maven:maven-model-transform",
+ "org.apache.maven:maven-plugin-api",
+ "org.apache.maven:maven-repository-metadata",
+ "org.apache.maven:maven-resolver-provider",
+ "org.apache.maven:maven-settings",
+ "org.apache.maven:maven-settings-builder",
+ "org.apache.maven:maven-slf4j-provider",
+ "org.apache.maven:maven-slf4j-wrapper",
+ "org.apache.maven:maven-toolchain-builder",
+ "org.apache.maven:maven-toolchain-model")
+ .collect(Collectors.toSet()));
+
+ // here we "align" other deps by fixing their version (for runtime
scope), and rest are (should be) provided
+ // anyway
+ Map<Dependency, Set<String>> otherCoreGAVs = new HashMap<>();
+ otherCoreGAVs.put(
+ new Dependency(
+ new
DefaultArtifact("org.eclipse.sisu:org.eclipse.sisu.inject:0.3.5"),
JavaScopes.RUNTIME),
+ Collections.singleton("org.sonatype.sisu:sisu-inject-bean"));
+ otherCoreGAVs.put(
+ new Dependency(new
DefaultArtifact("com.google.inject:guice:5.1.0"), JavaScopes.RUNTIME),
+ Collections.singleton("org.sonatype.sisu:sisu-guice"));
+
+ otherCoreGAVs.put(
+ new Dependency(
+ new
DefaultArtifact("org.eclipse.sisu:org.eclipse.sisu.plexus:0.3.5"),
JavaScopes.PROVIDED),
+ new HashSet<>(Arrays.asList(
+ "org.sonatype.spice:spice-inject-plexus",
+ "org.sonatype.sisu:sisu-inject-plexus",
+ "org.codehaus.plexus:plexus-container-default",
+ "plexus:plexus-container-default")));
+ otherCoreGAVs.put(
+ new Dependency(
+ new
DefaultArtifact("org.codehaus.plexus:plexus-classworlds:2.6.0"),
JavaScopes.PROVIDED),
+ Collections.singleton("classworlds:classworlds"));
Review Comment:
💡 `Streams.concat()` is from Guava (`com.google.common.collect.Streams`),
but `maven-core` has no Guava dependency. Replace with `Stream.concat()` (JDK
standard):
```suggestion
this.mavenManagedDependencies = Collections.unmodifiableList(
Stream.concat(mavenCoreDependencies.stream(),
otherCoreGAVs.keySet().stream())
.collect(Collectors.toList()));
```
--
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]
