From e7282ba51a1908ca22e278c20fa7d20a0e0de5f5 Mon Sep 17 00:00:00 2001 From: Alexander Kriegisch Date: Sat, 3 Feb 2024 07:45:19 +0700 Subject: [PATCH 1/4] 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. --- plexus-compiler-its/src/main/it/MCOMPILER-346-mre/pom.xml | 5 +++++ plexus-compiler-its/src/main/it/aspectj-compiler/pom.xml | 5 +++++ .../src/main/it/eclipse-compiler-mapstruct/pom.xml | 5 +++++ .../src/main/it/eclipse-compiler-procpath/pom.xml | 5 +++++ .../src/main/it/error-prone-compiler/pom.xml | 5 +++++ plexus-compiler-its/src/main/it/missing-warnings/pom.xml | 7 ++++++- .../src/main/it/simple-eclipse-compiler-fail/pom.xml | 5 +++++ .../src/main/it/simple-eclipse-compiler/pom.xml | 5 +++++ plexus-compiler-its/src/main/it/simple-javac-fork/pom.xml | 5 +++++ plexus-compiler-its/src/main/it/simple-javac/pom.xml | 5 +++++ 10 files changed, 51 insertions(+), 1 deletion(-) 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 index c8a739f3..661cafa8 100644 --- 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 @@ plexus-compiler-api ${plexus.compiler.version} + + org.codehaus.plexus + plexus-compiler-manager + ${plexus.compiler.version} + org.codehaus.plexus plexus-compiler-javac diff --git a/plexus-compiler-its/src/main/it/aspectj-compiler/pom.xml b/plexus-compiler-its/src/main/it/aspectj-compiler/pom.xml index 4638fbcb..f9c68382 100644 --- 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 @@ plexus-compiler-api ${plexus.compiler.version} + + org.codehaus.plexus + plexus-compiler-manager + ${plexus.compiler.version} + org.codehaus.plexus plexus-compiler-aspectj 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 index 6ffa414e..77c88a83 100644 --- 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 @@ plexus-compiler-api ${plexus.compiler.version} + + org.codehaus.plexus + plexus-compiler-manager + ${plexus.compiler.version} + org.codehaus.plexus plexus-compiler-eclipse 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 index c23ec93b..b3238814 100644 --- 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 @@ plexus-compiler-api ${plexus.compiler.version} + + org.codehaus.plexus + plexus-compiler-manager + ${plexus.compiler.version} + org.codehaus.plexus plexus-compiler-eclipse 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 index 1a63924f..87f7947f 100644 --- 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 @@ plexus-compiler-api ${plexus.compiler.version} + + org.codehaus.plexus + plexus-compiler-manager + ${plexus.compiler.version} + org.codehaus.plexus plexus-compiler-javac-errorprone diff --git a/plexus-compiler-its/src/main/it/missing-warnings/pom.xml b/plexus-compiler-its/src/main/it/missing-warnings/pom.xml index 42af0b33..81200739 100644 --- 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 @@ plexus-compiler-api ${plexus.compiler.version} + + org.codehaus.plexus + plexus-compiler-manager + ${plexus.compiler.version} + org.codehaus.plexus plexus-compiler-javac @@ -42,4 +47,4 @@ UTF-8 @pom.version@ - \ No newline at end of file + 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 index 03a77f48..d8a2ce41 100644 --- 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 @@ plexus-compiler-api ${plexus.compiler.version} + + org.codehaus.plexus + plexus-compiler-manager + ${plexus.compiler.version} + org.codehaus.plexus plexus-compiler-eclipse 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 index 54694e38..365df38a 100644 --- 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 @@ plexus-compiler-api ${plexus.compiler.version} + + org.codehaus.plexus + plexus-compiler-manager + ${plexus.compiler.version} + org.codehaus.plexus plexus-compiler-eclipse 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 index 2ed55a79..2e654657 100644 --- 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 @@ plexus-compiler-api ${plexus.compiler.version} + + org.codehaus.plexus + plexus-compiler-manager + ${plexus.compiler.version} + org.codehaus.plexus plexus-compiler-javac diff --git a/plexus-compiler-its/src/main/it/simple-javac/pom.xml b/plexus-compiler-its/src/main/it/simple-javac/pom.xml index a400f268..ca0f36d7 100644 --- 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 @@ plexus-compiler-api ${plexus.compiler.version} + + org.codehaus.plexus + plexus-compiler-manager + ${plexus.compiler.version} + org.codehaus.plexus plexus-compiler-javac From a3c52c6d8419851f8ca28221c577cd7d39b22803 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Fri, 2 Feb 2024 08:58:07 +0100 Subject: [PATCH 2/4] 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 --- .../manager/DefaultCompilerManager.java | 29 ++++++++++++++++--- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/plexus-compiler-manager/src/main/java/org/codehaus/plexus/compiler/manager/DefaultCompilerManager.java b/plexus-compiler-manager/src/main/java/org/codehaus/plexus/compiler/manager/DefaultCompilerManager.java index 29af62f7..298ba294 100644 --- a/plexus-compiler-manager/src/main/java/org/codehaus/plexus/compiler/manager/DefaultCompilerManager.java +++ b/plexus-compiler-manager/src/main/java/org/codehaus/plexus/compiler/manager/DefaultCompilerManager.java @@ -25,30 +25,51 @@ */ 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 Trygve Laugstøl */ @Named public class DefaultCompilerManager implements CompilerManager { + private static final String ERROR_MESSAGE = "Compiler '{}' could not be instantiated or injected properly. " + + "Running the build with -Dsisu.debug, looking for exceptions might help."; + 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. might mean, that you need to upgrade Maven."; + @Inject - private Map compilers; + private Map> compilers; + + private final Logger log = LoggerFactory.getLogger(getClass()); // ---------------------------------------------------------------------- // CompilerManager Implementation // ---------------------------------------------------------------------- public Compiler getCompiler(String compilerId) throws NoSuchCompilerException { - Compiler compiler = compilers.get(compilerId); + // Provider is lazy -> presence of provider means compiler is present, but not yet constructed + Provider 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); + } } } From ad1164bcfda43e19d222544514458b43a4a8479e Mon Sep 17 00:00:00 2001 From: Alexander Kriegisch Date: Sat, 3 Feb 2024 16:19:40 +0700 Subject: [PATCH 3/4] 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. --- .../plexus/compiler/manager/DefaultCompilerManager.java | 2 +- .../plexus/compiler/manager/NoSuchCompilerException.java | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/plexus-compiler-manager/src/main/java/org/codehaus/plexus/compiler/manager/DefaultCompilerManager.java b/plexus-compiler-manager/src/main/java/org/codehaus/plexus/compiler/manager/DefaultCompilerManager.java index 298ba294..4937ae21 100644 --- a/plexus-compiler-manager/src/main/java/org/codehaus/plexus/compiler/manager/DefaultCompilerManager.java +++ b/plexus-compiler-manager/src/main/java/org/codehaus/plexus/compiler/manager/DefaultCompilerManager.java @@ -69,7 +69,7 @@ public Compiler getCompiler(String compilerId) throws NoSuchCompilerException { } catch (Exception e) { // DI could not construct compiler log.error(ERROR_MESSAGE, compilerId); - throw new NoSuchCompilerException(compilerId); + throw new NoSuchCompilerException(compilerId, e); } } } diff --git a/plexus-compiler-manager/src/main/java/org/codehaus/plexus/compiler/manager/NoSuchCompilerException.java b/plexus-compiler-manager/src/main/java/org/codehaus/plexus/compiler/manager/NoSuchCompilerException.java index e40c6462..135f0950 100644 --- a/plexus-compiler-manager/src/main/java/org/codehaus/plexus/compiler/manager/NoSuchCompilerException.java +++ b/plexus-compiler-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; } From 90960d17b291ea7c180924af9fdb84122ba82c64 Mon Sep 17 00:00:00 2001 From: Alexander Kriegisch Date: Mon, 5 Feb 2024 06:39:29 +0700 Subject: [PATCH 4/4] 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. --- .../plexus/compiler/manager/DefaultCompilerManager.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plexus-compiler-manager/src/main/java/org/codehaus/plexus/compiler/manager/DefaultCompilerManager.java b/plexus-compiler-manager/src/main/java/org/codehaus/plexus/compiler/manager/DefaultCompilerManager.java index 4937ae21..b8024a22 100644 --- a/plexus-compiler-manager/src/main/java/org/codehaus/plexus/compiler/manager/DefaultCompilerManager.java +++ b/plexus-compiler-manager/src/main/java/org/codehaus/plexus/compiler/manager/DefaultCompilerManager.java @@ -39,7 +39,8 @@ @Named public class DefaultCompilerManager implements CompilerManager { private static final String ERROR_MESSAGE = "Compiler '{}' could not be instantiated or injected properly. " - + "Running the build with -Dsisu.debug, looking for exceptions might help."; + + "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. might mean, that you need to upgrade Maven.";