From d372db67964e6537e3bc716073623a0944cb555d Mon Sep 17 00:00:00 2001 From: Alexander Kriegisch Date: Fri, 2 Feb 2024 11:23:14 +0700 Subject: [PATCH 1/2] Make Eclipse and AspectJ compilers run on JDK 11 again AspectJ can compile many projects on JDK 11, because it is not always using Java 17 classes from JDT Core. If it does not work, there will be a runtime error, and the user can figure out what is wrong. For ECJ, the case is more complicated, because it directly imports Java 17 classes from JDT Core. There were isolated in new class EclipseJavaCompilerDelegate, which is accessed using Class::forName and method handles from EclipseJavaCompiler. I.e., Sisu Inject can scan the annotations on EclipseJavaCompiler on JDK 11, and there is no more confusing "No such compiler 'eclipse'" error, but rather: Error injecting constructor, ... EcjFailureException: Failed to run the ecj compiler: ECJ only works on Java 17+ at org.codehaus.plexus.compiler.eclipse.EclipseJavaCompiler. This explicitly tells the user what is wrong. A multi-release JAR solution was tested and worked well in Maven, but was dismissed due to the terrible developer experience in IDEs. Fixes #347. --- .../plexus-compiler-aspectj/pom.xml | 2 +- .../plexus-compiler-eclipse/pom.xml | 33 ++++++++++- .../compiler/eclipse/EclipseJavaCompiler.java | 56 ++++++++++--------- .../eclipse/EclipseJavaCompilerDelegate.java | 44 +++++++++++++++ pom.xml | 11 +++- 5 files changed, 116 insertions(+), 30 deletions(-) create mode 100644 plexus-compilers/plexus-compiler-eclipse/src/main/java/org/codehaus/plexus/compiler/eclipse/EclipseJavaCompilerDelegate.java diff --git a/plexus-compilers/plexus-compiler-aspectj/pom.xml b/plexus-compilers/plexus-compiler-aspectj/pom.xml index 38681fc4..2d56e4f3 100644 --- a/plexus-compilers/plexus-compiler-aspectj/pom.xml +++ b/plexus-compilers/plexus-compiler-aspectj/pom.xml @@ -14,7 +14,7 @@ AspectJ Compiler support for Plexus Compiler component. - 17 + 11 diff --git a/plexus-compilers/plexus-compiler-eclipse/pom.xml b/plexus-compilers/plexus-compiler-eclipse/pom.xml index 5017ceab..d05bb6b5 100644 --- a/plexus-compilers/plexus-compiler-eclipse/pom.xml +++ b/plexus-compilers/plexus-compiler-eclipse/pom.xml @@ -14,7 +14,7 @@ Eclipse Compiler support for Plexus Compiler component. - 17 + 11 @@ -57,4 +57,35 @@ + + + + org.apache.maven.plugins + maven-compiler-plugin + + + default-compile + + + **/EclipseJavaCompilerDelegate.java + + + + + compile-java-17 + + compile + + + 17 + + **/EclipseJavaCompilerDelegate.java + + + + + + + + diff --git a/plexus-compilers/plexus-compiler-eclipse/src/main/java/org/codehaus/plexus/compiler/eclipse/EclipseJavaCompiler.java b/plexus-compilers/plexus-compiler-eclipse/src/main/java/org/codehaus/plexus/compiler/eclipse/EclipseJavaCompiler.java index f14db674..d6bbb9f1 100644 --- a/plexus-compilers/plexus-compiler-eclipse/src/main/java/org/codehaus/plexus/compiler/eclipse/EclipseJavaCompiler.java +++ b/plexus-compilers/plexus-compiler-eclipse/src/main/java/org/codehaus/plexus/compiler/eclipse/EclipseJavaCompiler.java @@ -34,6 +34,9 @@ import java.io.File; import java.io.PrintWriter; import java.io.StringWriter; +import java.lang.invoke.MethodHandle; +import java.lang.invoke.MethodHandles; +import java.lang.invoke.MethodType; import java.nio.charset.Charset; import java.nio.charset.IllegalCharsetNameException; import java.nio.charset.UnsupportedCharsetException; @@ -52,8 +55,6 @@ import org.codehaus.plexus.compiler.CompilerOutputStyle; import org.codehaus.plexus.compiler.CompilerResult; import org.codehaus.plexus.util.StringUtils; -import org.eclipse.jdt.core.compiler.CompilationProgress; -import org.eclipse.jdt.core.compiler.batch.BatchCompiler; /** * @@ -63,12 +64,27 @@ public class EclipseJavaCompiler extends AbstractCompiler { public EclipseJavaCompiler() { super(CompilerOutputStyle.ONE_OUTPUT_FILE_PER_INPUT_FILE, ".java", ".class", null); + if (Runtime.version().feature() < 17) throw new EcjFailureException("ECJ only works on Java 17+"); + try { + // Do not directly import EclipseJavaCompilerDelegate or any ECJ classes compiled to target 17. + // This ensures that the plugin still runs on Java 11 and can report the error above. + Class delegateClass = Class.forName("org.codehaus.plexus.compiler.eclipse.EclipseJavaCompilerDelegate"); + MethodHandles.Lookup lookup = MethodHandles.lookup(); + MethodType getClassLoaderMT = MethodType.methodType(ClassLoader.class); + MethodType batchCompileMT = MethodType.methodType(boolean.class, List.class, PrintWriter.class); + getClassLoaderMH = lookup.findStatic(delegateClass, "getClassLoader", getClassLoaderMT); + batchCompileMH = lookup.findStatic(delegateClass, "batchCompile", batchCompileMT); + } catch (ClassNotFoundException | NoSuchMethodException | IllegalAccessException e) { + throw new RuntimeException(e); + } } // ---------------------------------------------------------------------- // Compiler Implementation // ---------------------------------------------------------------------- boolean errorsAsWarnings = false; + private final MethodHandle getClassLoaderMH; + private final MethodHandle batchCompileMH; @Override public String getCompilerId() { @@ -324,31 +340,15 @@ public void report(Diagnostic diagnostic) { getLog().debug("ecj command line: " + args); - success = BatchCompiler.compile( - args.toArray(new String[args.size()]), devNull, devNull, new CompilationProgress() { - @Override - public void begin(int i) {} - - @Override - public void done() {} - - @Override - public boolean isCanceled() { - return false; - } - - @Override - public void setTaskName(String s) {} - - @Override - public void worked(int i, int i1) {} - }); + success = (boolean) batchCompileMH.invoke(args, devNull); getLog().debug(sw.toString()); if (errorF.length() < 80) { throw new EcjFailureException(sw.toString()); } messageList = new EcjResponseParser().parse(errorF, errorsAsWarnings); + } catch (Throwable e) { + throw new Exception(e); } finally { if (null != errorF) { try { @@ -508,14 +508,16 @@ private static boolean haveSourceOrReleaseArgument(List args) { } private JavaCompiler getEcj() { - ServiceLoader javaCompilerLoader = - ServiceLoader.load(JavaCompiler.class, BatchCompiler.class.getClassLoader()); + ClassLoader classLoader; + try { + classLoader = (ClassLoader) getClassLoaderMH.invoke(); + } catch (Throwable e) { + throw new RuntimeException(e); + } + ServiceLoader javaCompilerLoader = ServiceLoader.load(JavaCompiler.class, classLoader); Class c = null; try { - c = Class.forName( - "org.eclipse.jdt.internal.compiler.tool.EclipseCompiler", - false, - BatchCompiler.class.getClassLoader()); + c = Class.forName("org.eclipse.jdt.internal.compiler.tool.EclipseCompiler", false, classLoader); } catch (ClassNotFoundException e) { // Ignore } diff --git a/plexus-compilers/plexus-compiler-eclipse/src/main/java/org/codehaus/plexus/compiler/eclipse/EclipseJavaCompilerDelegate.java b/plexus-compilers/plexus-compiler-eclipse/src/main/java/org/codehaus/plexus/compiler/eclipse/EclipseJavaCompilerDelegate.java new file mode 100644 index 00000000..663e255d --- /dev/null +++ b/plexus-compilers/plexus-compiler-eclipse/src/main/java/org/codehaus/plexus/compiler/eclipse/EclipseJavaCompilerDelegate.java @@ -0,0 +1,44 @@ +package org.codehaus.plexus.compiler.eclipse; + +import java.io.PrintWriter; +import java.util.List; + +import org.eclipse.jdt.core.compiler.CompilationProgress; +import org.eclipse.jdt.core.compiler.batch.BatchCompiler; + +/** + * Wraps API calls involving Java 17 class files from ECJ. {@link EclipseJavaCompiler} delegates to this class. + *

+ * Note: This class needs to be compiled with target 17, while all the other classes in this module can be + * compiled with target 11, as long as they do not directly import this class but use {@link Class#forName(String)} and + * method handles to invoke any methods from here. + */ +public class EclipseJavaCompilerDelegate { + static ClassLoader getClassLoader() { + return BatchCompiler.class.getClassLoader(); + } + + static boolean batchCompile(List args, PrintWriter devNull) { + return BatchCompiler.compile( + args.toArray(new String[0]), devNull, devNull, new BatchCompilerCompilationProgress()); + } + + private static class BatchCompilerCompilationProgress extends CompilationProgress { + @Override + public void begin(int i) {} + + @Override + public void done() {} + + @Override + public boolean isCanceled() { + return false; + } + + @Override + public void setTaskName(String s) {} + + @Override + public void worked(int i, int i1) {} + } +} diff --git a/pom.xml b/pom.xml index 13171985..1e36098a 100644 --- a/pom.xml +++ b/pom.xml @@ -182,7 +182,7 @@ maven-enforcer-plugin - enforce-java + enforce-maven-and-java-bytecode enforce @@ -192,6 +192,15 @@ [17,) [ERROR] OLD JDK [${java.version}] in use. This projects requires JDK 17 or newer + + 11 + + + org.aspectj:aspectjtools + org.eclipse.jdt:ecj + org.codehaus.plexus:plexus-compiler-eclipse + + From 8804303cfd498145f0f441c7afe1df5a1f1eccda Mon Sep 17 00:00:00 2001 From: Alexander Kriegisch Date: Fri, 2 Feb 2024 12:39:06 +0700 Subject: [PATCH 2/2] Throw CompilerException in performCompile|getEcj, not in constructor This shifts the focus from a supposed Maven component wiring error to the root cause, expressed in the error message: Fatal error compiling: ECJ needs JRE 17+ We need to add safeguards throwing the exception in both methods accessing class EclipseJavaCompilerDelegate, because we cannot predict who calls which one first. Omitting one safeguard might lead to the return of the infamous "No such compiler 'eclipse'" error. The new static boolean field EclipseJavaCompiler.isJdkSupported also enables us to write a simple unit test, asserting on the error message, if the field value is false. Relates to #347. --- .../compiler/eclipse/EclipseJavaCompiler.java | 14 +++-- .../EclipseCompilerUnsupportedJdkTest.java | 63 +++++++++++++++++++ 2 files changed, 73 insertions(+), 4 deletions(-) create mode 100644 plexus-compilers/plexus-compiler-eclipse/src/test/java/org/codehaus/plexus/compiler/eclipse/EclipseCompilerUnsupportedJdkTest.java diff --git a/plexus-compilers/plexus-compiler-eclipse/src/main/java/org/codehaus/plexus/compiler/eclipse/EclipseJavaCompiler.java b/plexus-compilers/plexus-compiler-eclipse/src/main/java/org/codehaus/plexus/compiler/eclipse/EclipseJavaCompiler.java index d6bbb9f1..4523919f 100644 --- a/plexus-compilers/plexus-compiler-eclipse/src/main/java/org/codehaus/plexus/compiler/eclipse/EclipseJavaCompiler.java +++ b/plexus-compilers/plexus-compiler-eclipse/src/main/java/org/codehaus/plexus/compiler/eclipse/EclipseJavaCompiler.java @@ -64,7 +64,7 @@ public class EclipseJavaCompiler extends AbstractCompiler { public EclipseJavaCompiler() { super(CompilerOutputStyle.ONE_OUTPUT_FILE_PER_INPUT_FILE, ".java", ".class", null); - if (Runtime.version().feature() < 17) throw new EcjFailureException("ECJ only works on Java 17+"); + if (!isJdkSupported) return; try { // Do not directly import EclipseJavaCompilerDelegate or any ECJ classes compiled to target 17. // This ensures that the plugin still runs on Java 11 and can report the error above. @@ -82,9 +82,10 @@ public EclipseJavaCompiler() { // ---------------------------------------------------------------------- // Compiler Implementation // ---------------------------------------------------------------------- + static boolean isJdkSupported = Runtime.version().feature() >= 17; boolean errorsAsWarnings = false; - private final MethodHandle getClassLoaderMH; - private final MethodHandle batchCompileMH; + private MethodHandle getClassLoaderMH; + private MethodHandle batchCompileMH; @Override public String getCompilerId() { @@ -93,6 +94,9 @@ public String getCompilerId() { @Override public CompilerResult performCompile(CompilerConfiguration config) throws CompilerException { + // Safeguard before using method handle accessing EclipseJavaCompilerDelegate + if (!isJdkSupported) throw new CompilerException("ECJ needs JRE 17+"); + List args = new ArrayList<>(); args.add("-noExit"); // Make sure ecj does not System.exit on us 8-/ @@ -507,7 +511,9 @@ private static boolean haveSourceOrReleaseArgument(List args) { return false; } - private JavaCompiler getEcj() { + private JavaCompiler getEcj() throws CompilerException { + // Safeguard before using method handle accessing EclipseJavaCompilerDelegate + if (!isJdkSupported) throw new CompilerException("ECJ needs JRE 17+"); ClassLoader classLoader; try { classLoader = (ClassLoader) getClassLoaderMH.invoke(); diff --git a/plexus-compilers/plexus-compiler-eclipse/src/test/java/org/codehaus/plexus/compiler/eclipse/EclipseCompilerUnsupportedJdkTest.java b/plexus-compilers/plexus-compiler-eclipse/src/test/java/org/codehaus/plexus/compiler/eclipse/EclipseCompilerUnsupportedJdkTest.java new file mode 100644 index 00000000..b19a4f8d --- /dev/null +++ b/plexus-compilers/plexus-compiler-eclipse/src/test/java/org/codehaus/plexus/compiler/eclipse/EclipseCompilerUnsupportedJdkTest.java @@ -0,0 +1,63 @@ +package org.codehaus.plexus.compiler.eclipse; + +import javax.inject.Inject; +import javax.inject.Named; + +import java.io.File; +import java.util.Set; +import java.util.stream.Collectors; + +import org.codehaus.plexus.compiler.Compiler; +import org.codehaus.plexus.compiler.CompilerConfiguration; +import org.codehaus.plexus.compiler.CompilerException; +import org.codehaus.plexus.testing.PlexusTest; +import org.codehaus.plexus.util.FileUtils; +import org.hamcrest.MatcherAssert; +import org.hamcrest.Matchers; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.parallel.Isolated; + +import static org.codehaus.plexus.testing.PlexusExtension.getBasedir; +import static org.junit.jupiter.api.Assertions.assertThrows; + +@PlexusTest +@Isolated("changes static variable") +public class EclipseCompilerUnsupportedJdkTest { + static final boolean IS_JDK_SUPPORTED = EclipseJavaCompiler.isJdkSupported; + + @Inject + @Named("eclipse") + Compiler compiler; + + @BeforeAll + public static void setUpClass() { + EclipseJavaCompiler.isJdkSupported = false; + } + + @AfterAll + public static void cleanUpClass() { + EclipseJavaCompiler.isJdkSupported = IS_JDK_SUPPORTED; + } + + @Test + public void testUnsupportedJdk() { + CompilerException error = assertThrows(CompilerException.class, () -> compiler.performCompile(getConfig())); + MatcherAssert.assertThat(error.getMessage(), Matchers.containsString("ECJ needs JRE 17+")); + } + + private CompilerConfiguration getConfig() throws Exception { + String sourceDir = getBasedir() + "/src/test-input/src/main"; + Set sourceFiles = FileUtils.getFileNames(new File(sourceDir), "**/*.java", null, false, true).stream() + .map(File::new) + .collect(Collectors.toSet()); + CompilerConfiguration compilerConfig = new CompilerConfiguration(); + compilerConfig.addSourceLocation(sourceDir); + compilerConfig.setOutputLocation(getBasedir() + "/target/eclipse/classes"); + compilerConfig.setSourceFiles(sourceFiles); + compilerConfig.setTargetVersion("1.8"); + compilerConfig.setSourceVersion("1.8"); + return compilerConfig; + } +}