Lazy providers and better error reporting (#361)

* ITs need plugin dependency to plexus-compiler-manager While testing #347, changes in compiler manager were not pulled into ITs, because Maven Compiler has a dependency on it, which must be overridden in all ITs or in projects using Plexus Compiler generally, if they need to override the version predefined by Maven Compiler. * Lazy providers and better error reporting If scanning, injection or construction fails, log a comprehensive error message on top of throwing a NoSuchCompilerException. Fixes #347. Co-authored-by: Alexander Kriegisch <[email protected]> * Code review: throw exception with cause In order to be able to do that at all, I had to add a constructor taking a throwable first. Now, even though a cause is propagated, at the time of writing this Maven Compiler will just catch the NoSuchCompilerException we throw, ignore its message and root cause and throw a new MojoExecutionException instead. :-/ Relates to #347. * Code review: improve DefaultCompilerManager.ERROR_MESSAGE Add more detail concerning possible user errors like misspelling the compiler ID or missing dependencies for a compiler. Relates to #347. --------- Co-authored-by: Tamas Cservenak <[email protected]>
codehaus-plexus · Feb 21, 2024 · 2248255 · 2248255
1 parent 99014e5
commit 2248255
Show file tree

Hide file tree

Showing 12 changed files with 81 additions and 6 deletions.
diff --git a/plexus-compiler-its/src/main/it/MCOMPILER-346-mre/pom.xml b/plexus-compiler-its/src/main/it/MCOMPILER-346-mre/pom.xml
@@ -64,6 +64,11 @@
             <artifactId>plexus-compiler-api</artifactId>
             <version>${plexus.compiler.version}</version>
           </dependency>
+          <dependency>
+            <groupId>org.codehaus.plexus</groupId>
+            <artifactId>plexus-compiler-manager</artifactId>
+            <version>${plexus.compiler.version}</version>
+          </dependency>
           <dependency>
             <groupId>org.codehaus.plexus</groupId>
             <artifactId>plexus-compiler-javac</artifactId>

diff --git a/plexus-compiler-its/src/main/it/aspectj-compiler/pom.xml b/plexus-compiler-its/src/main/it/aspectj-compiler/pom.xml
@@ -48,6 +48,11 @@
             <artifactId>plexus-compiler-api</artifactId>
             <version>${plexus.compiler.version}</version>
           </dependency>
+          <dependency>
+            <groupId>org.codehaus.plexus</groupId>
+            <artifactId>plexus-compiler-manager</artifactId>
+            <version>${plexus.compiler.version}</version>
+          </dependency>
           <dependency>
             <groupId>org.codehaus.plexus</groupId>
             <artifactId>plexus-compiler-aspectj</artifactId>

diff --git a/plexus-compiler-its/src/main/it/eclipse-compiler-mapstruct/pom.xml b/plexus-compiler-its/src/main/it/eclipse-compiler-mapstruct/pom.xml
@@ -59,6 +59,11 @@
             <artifactId>plexus-compiler-api</artifactId>
             <version>${plexus.compiler.version}</version>
           </dependency>
+          <dependency>
+            <groupId>org.codehaus.plexus</groupId>
+            <artifactId>plexus-compiler-manager</artifactId>
+            <version>${plexus.compiler.version}</version>
+          </dependency>
           <dependency>
             <groupId>org.codehaus.plexus</groupId>
             <artifactId>plexus-compiler-eclipse</artifactId>

diff --git a/plexus-compiler-its/src/main/it/eclipse-compiler-procpath/pom.xml b/plexus-compiler-its/src/main/it/eclipse-compiler-procpath/pom.xml
@@ -66,6 +66,11 @@
             <artifactId>plexus-compiler-api</artifactId>
             <version>${plexus.compiler.version}</version>
           </dependency>
+          <dependency>
+            <groupId>org.codehaus.plexus</groupId>
+            <artifactId>plexus-compiler-manager</artifactId>
+            <version>${plexus.compiler.version}</version>
+          </dependency>
           <dependency>
             <groupId>org.codehaus.plexus</groupId>
             <artifactId>plexus-compiler-eclipse</artifactId>

diff --git a/plexus-compiler-its/src/main/it/error-prone-compiler/pom.xml b/plexus-compiler-its/src/main/it/error-prone-compiler/pom.xml
@@ -83,6 +83,11 @@
             <artifactId>plexus-compiler-api</artifactId>
             <version>${plexus.compiler.version}</version>
           </dependency>
+          <dependency>
+            <groupId>org.codehaus.plexus</groupId>
+            <artifactId>plexus-compiler-manager</artifactId>
+            <version>${plexus.compiler.version}</version>
+          </dependency>
           <dependency>
             <groupId>org.codehaus.plexus</groupId>
             <artifactId>plexus-compiler-javac-errorprone</artifactId>

diff --git a/plexus-compiler-its/src/main/it/missing-warnings/pom.xml b/plexus-compiler-its/src/main/it/missing-warnings/pom.xml
@@ -25,6 +25,11 @@
 							<artifactId>plexus-compiler-api</artifactId>
 							<version>${plexus.compiler.version}</version>
 						</dependency>
+						<dependency>
+							<groupId>org.codehaus.plexus</groupId>
+							<artifactId>plexus-compiler-manager</artifactId>
+							<version>${plexus.compiler.version}</version>
+						</dependency>
 						<dependency>
 							<groupId>org.codehaus.plexus</groupId>
 							<artifactId>plexus-compiler-javac</artifactId>
@@ -42,4 +47,4 @@
 		<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
 		<plexus.compiler.version>@pom.version@</plexus.compiler.version>
 	</properties>
-</project>
+</project>
diff --git a/plexus-compiler-its/src/main/it/simple-eclipse-compiler-fail/pom.xml b/plexus-compiler-its/src/main/it/simple-eclipse-compiler-fail/pom.xml
@@ -61,6 +61,11 @@
             <artifactId>plexus-compiler-api</artifactId>
             <version>${plexus.compiler.version}</version>
           </dependency>
+          <dependency>
+            <groupId>org.codehaus.plexus</groupId>
+            <artifactId>plexus-compiler-manager</artifactId>
+            <version>${plexus.compiler.version}</version>
+          </dependency>
           <dependency>
             <groupId>org.codehaus.plexus</groupId>
             <artifactId>plexus-compiler-eclipse</artifactId>

diff --git a/plexus-compiler-its/src/main/it/simple-eclipse-compiler/pom.xml b/plexus-compiler-its/src/main/it/simple-eclipse-compiler/pom.xml
@@ -61,6 +61,11 @@
             <artifactId>plexus-compiler-api</artifactId>
             <version>${plexus.compiler.version}</version>
           </dependency>
+          <dependency>
+            <groupId>org.codehaus.plexus</groupId>
+            <artifactId>plexus-compiler-manager</artifactId>
+            <version>${plexus.compiler.version}</version>
+          </dependency>
           <dependency>
             <groupId>org.codehaus.plexus</groupId>
             <artifactId>plexus-compiler-eclipse</artifactId>

diff --git a/plexus-compiler-its/src/main/it/simple-javac-fork/pom.xml b/plexus-compiler-its/src/main/it/simple-javac-fork/pom.xml
@@ -63,6 +63,11 @@
             <artifactId>plexus-compiler-api</artifactId>
             <version>${plexus.compiler.version}</version>
           </dependency>
+          <dependency>
+            <groupId>org.codehaus.plexus</groupId>
+            <artifactId>plexus-compiler-manager</artifactId>
+            <version>${plexus.compiler.version}</version>
+          </dependency>
           <dependency>
             <groupId>org.codehaus.plexus</groupId>
             <artifactId>plexus-compiler-javac</artifactId>

diff --git a/plexus-compiler-its/src/main/it/simple-javac/pom.xml b/plexus-compiler-its/src/main/it/simple-javac/pom.xml
@@ -63,6 +63,11 @@
             <artifactId>plexus-compiler-api</artifactId>
             <version>${plexus.compiler.version}</version>
           </dependency>
+          <dependency>
+            <groupId>org.codehaus.plexus</groupId>
+            <artifactId>plexus-compiler-manager</artifactId>
+            <version>${plexus.compiler.version}</version>
+          </dependency>
           <dependency>
             <groupId>org.codehaus.plexus</groupId>
             <artifactId>plexus-compiler-javac</artifactId>

diff --git a/...er-manager/src/main/java/org/codehaus/plexus/compiler/manager/DefaultCompilerManager.java b/...er-manager/src/main/java/org/codehaus/plexus/compiler/manager/DefaultCompilerManager.java
@@ -25,30 +25,52 @@
  */
 import javax.inject.Inject;
 import javax.inject.Named;
+import javax.inject.Provider;
 
 import java.util.Map;
 
 import org.codehaus.plexus.compiler.Compiler;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * @author <a href="mailto:[email protected]">Trygve Laugst&oslash;l</a>
  */
 @Named
 public class DefaultCompilerManager implements CompilerManager {
+    private static final String ERROR_MESSAGE = "Compiler '{}' could not be instantiated or injected properly. "
+            + "If you spelled the compiler ID correctly and all necessary dependencies are on the classpath, "
+            + "then next you can try running the build with -Dsisu.debug, looking for exceptions.";
+    private static final String ERROR_MESSAGE_DETAIL = "TypeNotPresentException caused by UnsupportedClassVersionError "
+            + "might indicate, that the compiler needs a more recent Java runtime. "
+            + "IllegalArgumentException in ClassReader.<init> might mean, that you need to upgrade Maven.";
+
     @Inject
-    private Map<String, Compiler> compilers;
+    private Map<String, Provider<Compiler>> compilers;
+
+    private final Logger log = LoggerFactory.getLogger(getClass());
 
     // ----------------------------------------------------------------------
     // CompilerManager Implementation
     // ----------------------------------------------------------------------
 
     public Compiler getCompiler(String compilerId) throws NoSuchCompilerException {
-        Compiler compiler = compilers.get(compilerId);
+        // Provider<Class> is lazy -> presence of provider means compiler is present, but not yet constructed
+        Provider<Compiler> compilerProvider = compilers.get(compilerId);
 
-        if (compiler == null) {
+        if (compilerProvider == null) {
+            // Compiler could not be injected for some reason
+            log.error(ERROR_MESSAGE + " " + ERROR_MESSAGE_DETAIL, compilerId);
             throw new NoSuchCompilerException(compilerId);
         }
 
-        return compiler;
+        // Provider exists, but compiler was not created yet
+        try {
+            return compilerProvider.get();
+        } catch (Exception e) {
+            // DI could not construct compiler
+            log.error(ERROR_MESSAGE, compilerId);
+            throw new NoSuchCompilerException(compilerId, e);
+        }
     }
 }
diff --git a/...r-manager/src/main/java/org/codehaus/plexus/compiler/manager/NoSuchCompilerException.java b/...r-manager/src/main/java/org/codehaus/plexus/compiler/manager/NoSuchCompilerException.java
@@ -31,8 +31,11 @@ public class NoSuchCompilerException extends Exception {
     private final String compilerId;
 
     public NoSuchCompilerException(String compilerId) {
-        super("No such compiler '" + compilerId + "'.");
+        this(compilerId, null);
+    }
 
+    public NoSuchCompilerException(String compilerId, Throwable cause) {
+        super("No such compiler '" + compilerId + "'", cause);
         this.compilerId = compilerId;
     }