You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "slawekjaranowski (via GitHub)" <gi...@apache.org> on 2023/03/20 21:20:12 UTC

[GitHub] [maven-compiler-plugin] slawekjaranowski commented on a diff in pull request #180: [MCOMPILER-391] Use dep mgmt when resolving annotation processors and their deps

slawekjaranowski commented on code in PR #180:
URL: https://github.com/apache/maven-compiler-plugin/pull/180#discussion_r1142681687


##########
src/main/java/org/apache/maven/plugin/compiler/AbstractCompilerMojo.java:
##########
@@ -1584,42 +1605,91 @@ private List<String> resolveProcessorPathEntries() throws MojoExecutionException
             return null;
         }
 
-        Set<String> elements = new LinkedHashSet<>();
         try {
-            List<Dependency> dependencies = convertToDependencies(annotationProcessorPaths);
+            List<org.eclipse.aether.graph.Dependency> dependencies = convertToDependencies(annotationProcessorPaths);
+            List<org.eclipse.aether.graph.Dependency> managedDependencies =
+                    getManagedDependenciesForAnnotationProcessorPaths();
             CollectRequest collectRequest =
-                    new CollectRequest(dependencies, Collections.emptyList(), project.getRemoteProjectRepositories());
+                    new CollectRequest(dependencies, managedDependencies, project.getRemoteProjectRepositories());
             DependencyRequest dependencyRequest = new DependencyRequest();
             dependencyRequest.setCollectRequest(collectRequest);
             DependencyResult dependencyResult =
                     repositorySystem.resolveDependencies(session.getRepositorySession(), dependencyRequest);
 
-            for (ArtifactResult resolved : dependencyResult.getArtifactResults()) {
-                elements.add(resolved.getArtifact().getFile().getAbsolutePath());
-            }
-            return new ArrayList<>(elements);
+            return dependencyResult.getArtifactResults().stream()
+                    .map(resolved -> resolved.getArtifact().getFile().getAbsolutePath())
+                    .collect(Collectors.toList());
         } catch (Exception e) {
             throw new MojoExecutionException(
                     "Resolution of annotationProcessorPath dependencies failed: " + e.getLocalizedMessage(), e);
         }
     }
 
-    private List<Dependency> convertToDependencies(List<DependencyCoordinate> annotationProcessorPaths) {
-        List<Dependency> dependencies = new ArrayList<>();
+    private List<org.eclipse.aether.graph.Dependency> convertToDependencies(
+            List<DependencyCoordinate> annotationProcessorPaths) throws MojoExecutionException {
+        List<org.eclipse.aether.graph.Dependency> dependencies = new ArrayList<>();
         for (DependencyCoordinate annotationProcessorPath : annotationProcessorPaths) {
             ArtifactHandler handler = artifactHandlerManager.getArtifactHandler(annotationProcessorPath.getType());
+            String version = getAnnotationProcessorPathVersion(annotationProcessorPath);
             Artifact artifact = new DefaultArtifact(
                     annotationProcessorPath.getGroupId(),
                     annotationProcessorPath.getArtifactId(),
                     annotationProcessorPath.getClassifier(),
                     handler.getExtension(),
-                    annotationProcessorPath.getVersion());
+                    version);
             Set<Exclusion> exclusions = convertToAetherExclusions(annotationProcessorPath.getExclusions());
-            dependencies.add(new Dependency(artifact, JavaScopes.RUNTIME, false, exclusions));
+            dependencies.add(new org.eclipse.aether.graph.Dependency(artifact, JavaScopes.RUNTIME, false, exclusions));
         }
         return dependencies;
     }
 
+    private String getAnnotationProcessorPathVersion(DependencyCoordinate annotationProcessorPath)
+            throws MojoExecutionException {
+        String configuredVersion = annotationProcessorPath.getVersion();
+        if (configuredVersion != null) {
+            return configuredVersion;
+        } else {
+            List<Dependency> managedDependencies = getProjectManagedDependencies();
+            return findManagedVersion(annotationProcessorPath, managedDependencies)
+                    .orElseThrow(() -> new MojoExecutionException(String.format(
+                            "Cannot find version for annotation processor path '%s'. The version needs to be either"
+                                    + " provided directly in the plugin configuration or via dependency management.",
+                            annotationProcessorPath)));
+        }
+    }
+
+    private Optional<String> findManagedVersion(
+            DependencyCoordinate dependencyCoordinate, List<Dependency> managedDependencies) {
+        return managedDependencies.stream()
+                .filter(dep -> Objects.equals(dep.getGroupId(), dependencyCoordinate.getGroupId())
+                        && Objects.equals(dep.getArtifactId(), dependencyCoordinate.getArtifactId())
+                        && Objects.equals(dep.getClassifier(), dependencyCoordinate.getClassifier())
+                        && Objects.equals(dep.getType(), dependencyCoordinate.getType()))
+                .findAny()
+                .map(org.apache.maven.model.Dependency::getVersion);
+    }
+
+    private List<org.eclipse.aether.graph.Dependency> getManagedDependenciesForAnnotationProcessorPaths() {
+        if (!annotationProcessorPathsUseDepMgmt) {
+            return Collections.emptyList();
+        }
+        List<Dependency> projectManagedDependencies = getProjectManagedDependencies();
+        ArtifactTypeRegistry artifactTypeRegistry =
+                session.getRepositorySession().getArtifactTypeRegistry();
+
+        return projectManagedDependencies.stream()
+                .map(dep -> RepositoryUtils.toDependency(dep, artifactTypeRegistry))

Review Comment:
   If we need caching, it should be introduced in higher level not in plugin.



##########
src/main/java/org/apache/maven/plugin/compiler/AbstractCompilerMojo.java:
##########
@@ -1584,42 +1605,91 @@ private List<String> resolveProcessorPathEntries() throws MojoExecutionException
             return null;
         }
 
-        Set<String> elements = new LinkedHashSet<>();
         try {
-            List<Dependency> dependencies = convertToDependencies(annotationProcessorPaths);
+            List<org.eclipse.aether.graph.Dependency> dependencies = convertToDependencies(annotationProcessorPaths);
+            List<org.eclipse.aether.graph.Dependency> managedDependencies =
+                    getManagedDependenciesForAnnotationProcessorPaths();
             CollectRequest collectRequest =
-                    new CollectRequest(dependencies, Collections.emptyList(), project.getRemoteProjectRepositories());
+                    new CollectRequest(dependencies, managedDependencies, project.getRemoteProjectRepositories());
             DependencyRequest dependencyRequest = new DependencyRequest();
             dependencyRequest.setCollectRequest(collectRequest);
             DependencyResult dependencyResult =
                     repositorySystem.resolveDependencies(session.getRepositorySession(), dependencyRequest);
 
-            for (ArtifactResult resolved : dependencyResult.getArtifactResults()) {
-                elements.add(resolved.getArtifact().getFile().getAbsolutePath());
-            }
-            return new ArrayList<>(elements);
+            return dependencyResult.getArtifactResults().stream()
+                    .map(resolved -> resolved.getArtifact().getFile().getAbsolutePath())
+                    .collect(Collectors.toList());
         } catch (Exception e) {
             throw new MojoExecutionException(
                     "Resolution of annotationProcessorPath dependencies failed: " + e.getLocalizedMessage(), e);
         }
     }
 
-    private List<Dependency> convertToDependencies(List<DependencyCoordinate> annotationProcessorPaths) {
-        List<Dependency> dependencies = new ArrayList<>();
+    private List<org.eclipse.aether.graph.Dependency> convertToDependencies(
+            List<DependencyCoordinate> annotationProcessorPaths) throws MojoExecutionException {
+        List<org.eclipse.aether.graph.Dependency> dependencies = new ArrayList<>();
         for (DependencyCoordinate annotationProcessorPath : annotationProcessorPaths) {
             ArtifactHandler handler = artifactHandlerManager.getArtifactHandler(annotationProcessorPath.getType());
+            String version = getAnnotationProcessorPathVersion(annotationProcessorPath);
             Artifact artifact = new DefaultArtifact(
                     annotationProcessorPath.getGroupId(),
                     annotationProcessorPath.getArtifactId(),
                     annotationProcessorPath.getClassifier(),
                     handler.getExtension(),
-                    annotationProcessorPath.getVersion());
+                    version);
             Set<Exclusion> exclusions = convertToAetherExclusions(annotationProcessorPath.getExclusions());
-            dependencies.add(new Dependency(artifact, JavaScopes.RUNTIME, false, exclusions));
+            dependencies.add(new org.eclipse.aether.graph.Dependency(artifact, JavaScopes.RUNTIME, false, exclusions));
         }
         return dependencies;
     }
 
+    private String getAnnotationProcessorPathVersion(DependencyCoordinate annotationProcessorPath)
+            throws MojoExecutionException {
+        String configuredVersion = annotationProcessorPath.getVersion();
+        if (configuredVersion != null) {
+            return configuredVersion;
+        } else {
+            List<Dependency> managedDependencies = getProjectManagedDependencies();
+            return findManagedVersion(annotationProcessorPath, managedDependencies)
+                    .orElseThrow(() -> new MojoExecutionException(String.format(
+                            "Cannot find version for annotation processor path '%s'. The version needs to be either"
+                                    + " provided directly in the plugin configuration or via dependency management.",
+                            annotationProcessorPath)));
+        }
+    }
+
+    private Optional<String> findManagedVersion(
+            DependencyCoordinate dependencyCoordinate, List<Dependency> managedDependencies) {
+        return managedDependencies.stream()
+                .filter(dep -> Objects.equals(dep.getGroupId(), dependencyCoordinate.getGroupId())
+                        && Objects.equals(dep.getArtifactId(), dependencyCoordinate.getArtifactId())
+                        && Objects.equals(dep.getClassifier(), dependencyCoordinate.getClassifier())
+                        && Objects.equals(dep.getType(), dependencyCoordinate.getType()))
+                .findAny()
+                .map(org.apache.maven.model.Dependency::getVersion);
+    }

Review Comment:
   We can mention such case at annotationProcessorPaths documentation, that missing version can impact on performance with big dependency management.
   
   We can also consider use of `parallelStream`, there is no finish collector so should be safe



-- 
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: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org