From 5af0a11c48460990029058ab57ceac0c568ada9c Mon Sep 17 00:00:00 2001 From: Konrad Windszus Date: Thu, 1 Feb 2024 19:33:04 +0100 Subject: [PATCH 1/4] Correctly determine the version of the underlying javac tool Ignore CompilerConfiguration values as they are unreliable (and don't represent the used javac version) Make test execution more resilient by interpolating settings.xml correctly and make sure commons-lang 2.0 is resolved prior to unit testing This closes #356 --- plexus-compiler-test/pom.xml | 2 +- .../plexus/compiler/AbstractCompilerTest.java | 16 +- .../plexus/compiler/javac/JavacCompiler.java | 249 +++++++++--------- .../javac/AbstractJavacCompilerTest.java | 63 ++--- .../compiler/javac/JavacCompilerTest.java | 23 ++ plexus-compilers/pom.xml | 7 + 6 files changed, 189 insertions(+), 171 deletions(-) diff --git a/plexus-compiler-test/pom.xml b/plexus-compiler-test/pom.xml index 3dcab953..d3f45562 100644 --- a/plexus-compiler-test/pom.xml +++ b/plexus-compiler-test/pom.xml @@ -50,7 +50,7 @@ org.apache.maven - maven-settings + maven-settings-builder ${mavenVersion} diff --git a/plexus-compiler-test/src/main/java/org/codehaus/plexus/compiler/AbstractCompilerTest.java b/plexus-compiler-test/src/main/java/org/codehaus/plexus/compiler/AbstractCompilerTest.java index 977ffbd1..85374c54 100644 --- a/plexus-compiler-test/src/main/java/org/codehaus/plexus/compiler/AbstractCompilerTest.java +++ b/plexus-compiler-test/src/main/java/org/codehaus/plexus/compiler/AbstractCompilerTest.java @@ -41,11 +41,13 @@ import org.apache.maven.artifact.repository.DefaultArtifactRepository; import org.apache.maven.artifact.repository.layout.ArtifactRepositoryLayout; import org.apache.maven.artifact.versioning.VersionRange; +import org.apache.maven.properties.internal.SystemProperties; import org.apache.maven.settings.Settings; -import org.apache.maven.settings.io.xpp3.SettingsXpp3Reader; +import org.apache.maven.settings.building.DefaultSettingsBuilderFactory; +import org.apache.maven.settings.building.DefaultSettingsBuildingRequest; +import org.apache.maven.settings.building.SettingsBuildingRequest; import org.codehaus.plexus.testing.PlexusTest; import org.codehaus.plexus.util.FileUtils; -import org.codehaus.plexus.util.ReaderFactory; import org.codehaus.plexus.util.StringUtils; import org.hamcrest.io.FileMatchers; import org.junit.jupiter.api.BeforeEach; @@ -83,7 +85,13 @@ final void setUpLocalRepo() throws Exception { if (localRepo == null) { File settingsFile = new File(System.getProperty("user.home"), ".m2/settings.xml"); if (settingsFile.exists()) { - Settings settings = new SettingsXpp3Reader().read(ReaderFactory.newXmlReader(settingsFile)); + SettingsBuildingRequest request = new DefaultSettingsBuildingRequest(); + request.setUserSettingsFile(settingsFile); + request.setSystemProperties(SystemProperties.getSystemProperties()); + Settings settings = new DefaultSettingsBuilderFactory() + .newInstance() + .build(request) + .getEffectiveSettings(); localRepo = settings.getLocalRepository(); } } @@ -116,7 +124,7 @@ protected List getClasspath() throws Exception { File file = getLocalArtifactPath("commons-lang", "commons-lang", "2.0", "jar"); assertThat( - "test prerequisite: commons-lang library must be available in local repository, expected ", + "test prerequisite: commons-lang library must be available in local repository at " + file, file, FileMatchers.aReadableFile()); diff --git a/plexus-compilers/plexus-compiler-javac/src/main/java/org/codehaus/plexus/compiler/javac/JavacCompiler.java b/plexus-compilers/plexus-compiler-javac/src/main/java/org/codehaus/plexus/compiler/javac/JavacCompiler.java index 0255c2a7..68c623ef 100644 --- a/plexus-compilers/plexus-compiler-javac/src/main/java/org/codehaus/plexus/compiler/javac/JavacCompiler.java +++ b/plexus-compilers/plexus-compiler-javac/src/main/java/org/codehaus/plexus/compiler/javac/JavacCompiler.java @@ -59,12 +59,15 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Deque; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.NoSuchElementException; import java.util.Properties; +import java.util.Set; import java.util.StringTokenizer; import java.util.concurrent.ConcurrentLinkedDeque; +import java.util.regex.Matcher; import java.util.regex.Pattern; import org.codehaus.plexus.compiler.AbstractCompiler; @@ -108,6 +111,8 @@ public class JavacCompiler extends AbstractCompiler { private final Deque> javacClasses = new ConcurrentLinkedDeque<>(); + private static final Pattern JAVA_MAJOR_AND_MINOR_VERSION_PATTERN = Pattern.compile("\\d+(\\.\\d+)?"); + @Inject private InProcessCompiler inProcessCompiler; @@ -128,6 +133,39 @@ public String getCompilerId() { return "javac"; } + private String getInProcessJavacVersion() throws CompilerException { + return System.getProperty("java.version"); + } + + private String getOutOfProcessJavacVersion(String executable) throws CompilerException { + Commandline cli = new Commandline(); + cli.setExecutable(executable); + /* + * The option "-version" should be supported by javac since 1.6 (https://docs.oracle.com/javase/6/docs/technotes/tools/solaris/javac.html) + * up to 21 (https://docs.oracle.com/en/java/javase/21/docs/specs/man/javac.html#standard-options) + */ + cli.addArguments(new String[] {"-version"}); // + CommandLineUtils.StringStreamConsumer out = new CommandLineUtils.StringStreamConsumer(); + try { + int exitCode = CommandLineUtils.executeCommandLine(cli, out, out); + if (exitCode != 0) { + throw new CompilerException("Could not retrieve version from " + executable + ". Exit code " + exitCode + + ", Output: " + out.getOutput()); + } + } catch (CommandLineException e) { + throw new CompilerException("Error while executing the external compiler " + executable, e); + } + return extractMajorAndMinorVersion(out.getOutput()); + } + + static String extractMajorAndMinorVersion(String text) { + Matcher matcher = JAVA_MAJOR_AND_MINOR_VERSION_PATTERN.matcher(text); + if (!matcher.find()) { + throw new IllegalArgumentException("Could not extract version from \"" + text + "\""); + } + return matcher.group(); + } + @Override public CompilerResult performCompile(CompilerConfiguration config) throws CompilerException { File destinationDir = new File(config.getOutputLocation()); @@ -144,27 +182,25 @@ public CompilerResult performCompile(CompilerConfiguration config) throws Compil logCompiling(sourceFiles, config); - String[] args = buildCompilerArguments(config, sourceFiles); + final String javacVersion; + final String executable; + if (config.isFork()) { + executable = getJavacExecutable(config); + javacVersion = getOutOfProcessJavacVersion(executable); + } else { + javacVersion = getInProcessJavacVersion(); + executable = null; + } + + String[] args = buildCompilerArguments(config, sourceFiles, javacVersion); CompilerResult result; if (config.isFork()) { - String executable = config.getExecutable(); - - if (StringUtils.isEmpty(executable)) { - try { - executable = getJavacExecutable(); - } catch (IOException e) { - if (getLog().isWarnEnabled()) { - getLog().warn("Unable to autodetect 'javac' path, using 'javac' from the environment."); - } - executable = "javac"; - } - } result = compileOutOfProcess(config, executable, args); } else { - if (isJava16() && !config.isForceJavacCompilerUse()) { + if (hasJavaxToolProvider() && !config.isForceJavacCompilerUse()) { // use fqcn to prevent loading of the class on 1.5 environment ! result = inProcessCompiler().compileInProcess(args, config, sourceFiles); } else { @@ -179,7 +215,11 @@ protected InProcessCompiler inProcessCompiler() { return inProcessCompiler; } - protected static boolean isJava16() { + /** + * + * @return {@code true} if the current context class loader has access to {@code javax.tools.ToolProvider} + */ + protected static boolean hasJavaxToolProvider() { try { Thread.currentThread().getContextClassLoader().loadClass("javax.tools.ToolProvider"); return true; @@ -189,10 +229,18 @@ protected static boolean isJava16() { } public String[] createCommandLine(CompilerConfiguration config) throws CompilerException { - return buildCompilerArguments(config, getSourceFiles(config)); + final String javacVersion; + if (config.isFork()) { + String executable = getJavacExecutable(config); + javacVersion = getOutOfProcessJavacVersion(executable); + } else { + javacVersion = getInProcessJavacVersion(); + } + return buildCompilerArguments(config, getSourceFiles(config), javacVersion); } - public static String[] buildCompilerArguments(CompilerConfiguration config, String[] sourceFiles) { + public static String[] buildCompilerArguments( + CompilerConfiguration config, String[] sourceFiles, String javacVersion) { List args = new ArrayList<>(); // ---------------------------------------------------------------------- @@ -231,11 +279,11 @@ public static String[] buildCompilerArguments(CompilerConfiguration config, Stri args.add(getPathString(sourceLocations)); } - if (!isJava16() || config.isForceJavacCompilerUse() || config.isFork()) { + if (!hasJavaxToolProvider() || config.isForceJavacCompilerUse() || config.isFork()) { args.addAll(Arrays.asList(sourceFiles)); } - if (!isPreJava16(config)) { + if (JavaVersion.JAVA_1_6.isOlderOrEqualTo(javacVersion)) { // now add jdk 1.6 annotation processing related parameters if (config.getGeneratedSourcesDirectory() != null) { @@ -288,7 +336,7 @@ public static String[] buildCompilerArguments(CompilerConfiguration config, Stri args.add("-verbose"); } - if (!isPreJava18(config) && config.isParameters()) { + if (JavaVersion.JAVA_1_8.isOlderOrEqualTo(javacVersion) && config.isParameters()) { args.add("-parameters"); } @@ -338,17 +386,17 @@ public static String[] buildCompilerArguments(CompilerConfiguration config, Stri args.add(config.getTargetVersion()); } - if (!suppressSource(config) && StringUtils.isEmpty(config.getSourceVersion())) { + if (JavaVersion.JAVA_1_4.isOlderOrEqualTo(javacVersion) && StringUtils.isEmpty(config.getSourceVersion())) { // If omitted, later JDKs complain about a 1.1 target args.add("-source"); args.add("1.3"); - } else if (!suppressSource(config)) { + } else if (JavaVersion.JAVA_1_4.isOlderOrEqualTo(javacVersion)) { args.add("-source"); args.add(config.getSourceVersion()); } } - if (!suppressEncoding(config) && !StringUtils.isEmpty(config.getSourceEncoding())) { + if (JavaVersion.JAVA_1_4.isOlderOrEqualTo(javacVersion) && !StringUtils.isEmpty(config.getSourceEncoding())) { args.add("-encoding"); args.add(config.getSourceEncoding()); } @@ -384,115 +432,38 @@ public static String[] buildCompilerArguments(CompilerConfiguration config, Stri } /** - * Determine if the compiler is a version prior to 1.4. - * This is needed as 1.3 and earlier did not support -source or -encoding parameters - * - * @param config The compiler configuration to test. - * @return true if the compiler configuration represents a Java 1.4 compiler or later, false otherwise - */ - private static boolean isPreJava14(CompilerConfiguration config) { - String v = config.getCompilerVersion(); - - if (v == null) { - return false; - } - - return v.startsWith("1.3") || v.startsWith("1.2") || v.startsWith("1.1") || v.startsWith("1.0"); - } - - /** - * Determine if the compiler is a version prior to 1.6. - * This is needed for annotation processing parameters. - * - * @param config The compiler configuration to test. - * @return true if the compiler configuration represents a Java 1.6 compiler or later, false otherwise + * Represents a particular Java version (through their according version prefixes) */ - private static boolean isPreJava16(CompilerConfiguration config) { - String v = config.getReleaseVersion(); - - if (v == null) { - v = config.getCompilerVersion(); + enum JavaVersion { + JAVA_1_3("1.3", "1.2", "1.1", "1.0"), + JAVA_1_4("1.4"), + JAVA_1_5("1.5"), + JAVA_1_6("1.6"), + JAVA_1_7("1.7"), + JAVA_1_8("1.8"), + JAVA_9("9"); // since Java 9 a different versioning scheme was used (https://openjdk.org/jeps/223) + final Set versionPrefixes; + + JavaVersion(String... versionPrefixes) { + this.versionPrefixes = new HashSet<>(Arrays.asList(versionPrefixes)); } - if (v == null) { - v = config.getSourceVersion(); - } - - if (v == null) { - return true; - } - - return v.startsWith("5") - || v.startsWith("1.5") - || v.startsWith("1.4") - || v.startsWith("1.3") - || v.startsWith("1.2") - || v.startsWith("1.1") - || v.startsWith("1.0"); - } - - private static boolean isPreJava18(CompilerConfiguration config) { - String v = config.getReleaseVersion(); - - if (v == null) { - v = config.getCompilerVersion(); - } - - if (v == null) { - v = config.getSourceVersion(); - } - - if (v == null) { - return true; - } - - return v.startsWith("7") - || v.startsWith("1.7") - || v.startsWith("6") - || v.startsWith("1.6") - || v.startsWith("1.5") - || v.startsWith("1.4") - || v.startsWith("1.3") - || v.startsWith("1.2") - || v.startsWith("1.1") - || v.startsWith("1.0"); - } - - private static boolean isPreJava9(CompilerConfiguration config) { - - String v = config.getReleaseVersion(); - - if (v == null) { - v = config.getCompilerVersion(); - } - - if (v == null) { - v = config.getSourceVersion(); - } - - if (v == null) { + /** + * The internal logic checks if the given version starts with the prefix of one of the enums preceding the current one. + * + * @param version the version to check + * @return {@code true} if the version represented by this enum is older than or equal (in its minor and major version) to a given version + */ + boolean isOlderOrEqualTo(String version) { + // go through all previous enums + JavaVersion[] allJavaVersionPrefixes = JavaVersion.values(); + for (int n = ordinal() - 1; n > -1; n--) { + if (allJavaVersionPrefixes[n].versionPrefixes.stream().anyMatch(version::startsWith)) { + return false; + } + } return true; } - - return v.startsWith("8") - || v.startsWith("1.8") - || v.startsWith("7") - || v.startsWith("1.7") - || v.startsWith("1.6") - || v.startsWith("1.5") - || v.startsWith("1.4") - || v.startsWith("1.3") - || v.startsWith("1.2") - || v.startsWith("1.1") - || v.startsWith("1.0"); - } - - private static boolean suppressSource(CompilerConfiguration config) { - return isPreJava14(config); - } - - private static boolean suppressEncoding(CompilerConfiguration config) { - return isPreJava14(config); } /** @@ -925,11 +896,33 @@ private File createFileWithArguments(String[] args, String outputDirectory) thro } } + /** + * Get the path of the javac tool executable to use. + * Either given through explicit configuration or via {@link #getJavacExecutable()}. + * @param config the configuration + * @return the path of the javac tool + */ + protected String getJavacExecutable(CompilerConfiguration config) { + String executable = config.getExecutable(); + + if (StringUtils.isEmpty(executable)) { + try { + executable = getJavacExecutable(); + } catch (IOException e) { + if (getLog().isWarnEnabled()) { + getLog().warn("Unable to autodetect 'javac' path, using 'javac' from the environment."); + } + executable = "javac"; + } + } + return executable; + } + /** * Get the path of the javac tool executable: try to find it depending the OS or the java.home * system property or the JAVA_HOME environment variable. * - * @return the path of the Javadoc tool + * @return the path of the javac tool * @throws IOException if not found */ private static String getJavacExecutable() throws IOException { diff --git a/plexus-compilers/plexus-compiler-javac/src/test/java/org/codehaus/plexus/compiler/javac/AbstractJavacCompilerTest.java b/plexus-compilers/plexus-compiler-javac/src/test/java/org/codehaus/plexus/compiler/javac/AbstractJavacCompilerTest.java index 9cb656a5..2dfd7a5d 100644 --- a/plexus-compilers/plexus-compiler-javac/src/test/java/org/codehaus/plexus/compiler/javac/AbstractJavacCompilerTest.java +++ b/plexus-compilers/plexus-compiler-javac/src/test/java/org/codehaus/plexus/compiler/javac/AbstractJavacCompilerTest.java @@ -34,12 +34,10 @@ import org.codehaus.plexus.compiler.AbstractCompilerTest; import org.codehaus.plexus.compiler.CompilerConfiguration; import org.codehaus.plexus.util.StringUtils; -import org.hamcrest.Matchers; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.is; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; /** * @author Jason van Zyl @@ -207,22 +205,19 @@ protected Collection expectedOutputFiles() { "org/codehaus/foo/ReservedWord.class"); } - protected void internalTest(CompilerConfiguration compilerConfiguration, List expectedArguments) { - internalTest(compilerConfiguration, expectedArguments, new String[0]); + protected void internalTest( + CompilerConfiguration compilerConfiguration, List expectedArguments, String javacVersion) { + internalTest(compilerConfiguration, expectedArguments, new String[0], javacVersion); } public void internalTest( - CompilerConfiguration compilerConfiguration, List expectedArguments, String[] sources) { - String[] actualArguments = JavacCompiler.buildCompilerArguments(compilerConfiguration, sources); - - assertThat( - "The expected and actual argument list sizes differ.", - actualArguments, - Matchers.arrayWithSize(expectedArguments.size())); + CompilerConfiguration compilerConfiguration, + List expectedArguments, + String[] sources, + String javacVersion) { + String[] actualArguments = JavacCompiler.buildCompilerArguments(compilerConfiguration, sources, javacVersion); - for (int i = 0; i < actualArguments.length; i++) { - assertThat("Unexpected argument", actualArguments[i], is(expectedArguments.get(i))); - } + assertArrayEquals(actualArguments, expectedArguments.toArray(new String[0])); } @Test @@ -231,11 +226,9 @@ public void testBuildCompilerArgs13() { CompilerConfiguration compilerConfiguration = new CompilerConfiguration(); - compilerConfiguration.setCompilerVersion("1.3"); - populateArguments(compilerConfiguration, expectedArguments, true, true, false); - internalTest(compilerConfiguration, expectedArguments); + internalTest(compilerConfiguration, expectedArguments, "1.3"); } @Test @@ -244,11 +237,9 @@ public void testBuildCompilerArgs14() { CompilerConfiguration compilerConfiguration = new CompilerConfiguration(); - compilerConfiguration.setCompilerVersion("1.4"); - populateArguments(compilerConfiguration, expectedArguments, false, false, false); - internalTest(compilerConfiguration, expectedArguments); + internalTest(compilerConfiguration, expectedArguments, "1.4"); } @Test @@ -257,11 +248,9 @@ public void testBuildCompilerArgs15() { CompilerConfiguration compilerConfiguration = new CompilerConfiguration(); - compilerConfiguration.setCompilerVersion("1.5"); - populateArguments(compilerConfiguration, expectedArguments, false, false, false); - internalTest(compilerConfiguration, expectedArguments); + internalTest(compilerConfiguration, expectedArguments, "1.5"); } @Test @@ -270,11 +259,9 @@ public void testBuildCompilerArgs18() { CompilerConfiguration compilerConfiguration = new CompilerConfiguration(); - compilerConfiguration.setCompilerVersion("1.8"); - populateArguments(compilerConfiguration, expectedArguments, false, false, true); - internalTest(compilerConfiguration, expectedArguments); + internalTest(compilerConfiguration, expectedArguments, "1.8"); } @Test @@ -283,9 +270,9 @@ public void testBuildCompilerArgsUnspecifiedVersion() { CompilerConfiguration compilerConfiguration = new CompilerConfiguration(); - populateArguments(compilerConfiguration, expectedArguments, false, false, false); + populateArguments(compilerConfiguration, expectedArguments, false, false, true); - internalTest(compilerConfiguration, expectedArguments); + internalTest(compilerConfiguration, expectedArguments, "unknown"); } @Test @@ -298,9 +285,9 @@ public void testBuildCompilerDebugLevel() { compilerConfiguration.setDebugLevel("none"); - populateArguments(compilerConfiguration, expectedArguments, false, false, false); + populateArguments(compilerConfiguration, expectedArguments, false, false, true); - internalTest(compilerConfiguration, expectedArguments); + internalTest(compilerConfiguration, expectedArguments, "1.8"); } // PLXCOMP-190 @@ -334,7 +321,7 @@ public void testJRuntimeArguments() { compilerConfiguration.setCustomCompilerArgumentsAsMap(customCompilerArguments); // don't expect this argument!! - internalTest(compilerConfiguration, expectedArguments); + internalTest(compilerConfiguration, expectedArguments, "1.8"); } @Test @@ -370,7 +357,7 @@ public void testModulePathAnnotations() throws Exception { // unshared table expectedArguments.add("-XDuseUnsharedTable=true"); - internalTest(compilerConfiguration, expectedArguments, source); + internalTest(compilerConfiguration, expectedArguments, source, "11.0.1"); } @Test @@ -399,7 +386,7 @@ public void testModulePath() throws Exception { // unshared table expectedArguments.add("-XDuseUnsharedTable=true"); - internalTest(compilerConfiguration, expectedArguments); + internalTest(compilerConfiguration, expectedArguments, "11.0.1"); } @Test @@ -427,7 +414,7 @@ public void testModuleVersion() { // unshared table expectedArguments.add("-XDuseUnsharedTable=true"); - internalTest(compilerConfiguration, expectedArguments); + internalTest(compilerConfiguration, expectedArguments, "11.0.1"); } @Test @@ -449,7 +436,7 @@ public void testReleaseVersion() { // unshared table expectedArguments.add("-XDuseUnsharedTable=true"); - internalTest(compilerConfiguration, expectedArguments); + internalTest(compilerConfiguration, expectedArguments, "11.0.1"); } @Test @@ -476,7 +463,7 @@ public void testFailOnWarning() { // unshared table expectedArguments.add("-XDuseUnsharedTable=true"); - internalTest(compilerConfiguration, expectedArguments); + internalTest(compilerConfiguration, expectedArguments, "1.8"); } @Test @@ -507,7 +494,7 @@ public void testMultipleAddExports() { // unshared table expectedArguments.add("-XDuseUnsharedTable=true"); - internalTest(compilerConfiguration, expectedArguments); + internalTest(compilerConfiguration, expectedArguments, "1.8"); } /* This test fails on Java 1.4. The multiple parameters of the same source file cause an error, as it is interpreted as a DuplicateClass diff --git a/plexus-compilers/plexus-compiler-javac/src/test/java/org/codehaus/plexus/compiler/javac/JavacCompilerTest.java b/plexus-compilers/plexus-compiler-javac/src/test/java/org/codehaus/plexus/compiler/javac/JavacCompilerTest.java index 8a182fcf..3cbc4f12 100644 --- a/plexus-compilers/plexus-compiler-javac/src/test/java/org/codehaus/plexus/compiler/javac/JavacCompilerTest.java +++ b/plexus-compilers/plexus-compiler-javac/src/test/java/org/codehaus/plexus/compiler/javac/JavacCompilerTest.java @@ -7,7 +7,9 @@ import java.util.stream.Stream; import org.codehaus.plexus.compiler.CompilerMessage; +import org.codehaus.plexus.compiler.javac.JavacCompiler.JavaVersion; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; @@ -18,6 +20,9 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.notNullValue; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; /* * Licensed to the Apache Software Foundation (ASF) under one @@ -103,4 +108,22 @@ private static Stream testParseModernStream_withAnnotationProcessingE "JDK 21 German", "\n\nEin Annotationsprozessor hat eine nicht abgefangene Ausnahme ausgelöst.\nDetails finden Sie im folgenden Stacktrace.\n\n")); } + + @Test + void testJavaVersionPrefixes() { + assertFalse(JavaVersion.JAVA_1_4.isOlderOrEqualTo("1.3")); + assertTrue(JavaVersion.JAVA_1_4.isOlderOrEqualTo("1.4")); + assertTrue(JavaVersion.JAVA_1_4.isOlderOrEqualTo("1.4.0_something")); + assertFalse(JavaVersion.JAVA_1_5.isOlderOrEqualTo("1.4")); + assertTrue(JavaVersion.JAVA_1_8.isOlderOrEqualTo("1.8")); + assertTrue(JavaVersion.JAVA_1_8.isOlderOrEqualTo("22.0.2-something")); + assertTrue(JavaVersion.JAVA_1_8.isOlderOrEqualTo("unknown")); + } + + @Test + void testExtractMajorAndMinorVersion() { + assertEquals("11.0", JavacCompiler.extractMajorAndMinorVersion("javac 11.0.22")); + assertEquals("11.0", JavacCompiler.extractMajorAndMinorVersion("11.0.22")); + assertEquals("21", JavacCompiler.extractMajorAndMinorVersion("javac 21")); + } } diff --git a/plexus-compilers/pom.xml b/plexus-compilers/pom.xml index fcea9983..12f78ce9 100644 --- a/plexus-compilers/pom.xml +++ b/plexus-compilers/pom.xml @@ -36,6 +36,13 @@ plexus-compiler-test test + + + commons-lang + commons-lang + 2.0 + test + From 37195795d24d0e6a09e493b65d1901233329ba98 Mon Sep 17 00:00:00 2001 From: Konrad Windszus Date: Fri, 2 Feb 2024 11:07:23 +0100 Subject: [PATCH 2/4] Cache versions per executable --- .../plexus/compiler/javac/JavacCompiler.java | 43 +++++++++++-------- 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/plexus-compilers/plexus-compiler-javac/src/main/java/org/codehaus/plexus/compiler/javac/JavacCompiler.java b/plexus-compilers/plexus-compiler-javac/src/main/java/org/codehaus/plexus/compiler/javac/JavacCompiler.java index 68c623ef..f7154566 100644 --- a/plexus-compilers/plexus-compiler-javac/src/main/java/org/codehaus/plexus/compiler/javac/JavacCompiler.java +++ b/plexus-compilers/plexus-compiler-javac/src/main/java/org/codehaus/plexus/compiler/javac/JavacCompiler.java @@ -66,6 +66,7 @@ import java.util.Properties; import java.util.Set; import java.util.StringTokenizer; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentLinkedDeque; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -113,6 +114,9 @@ public class JavacCompiler extends AbstractCompiler { private static final Pattern JAVA_MAJOR_AND_MINOR_VERSION_PATTERN = Pattern.compile("\\d+(\\.\\d+)?"); + /** Cache of javac version per executable (never invalidated) */ + private static final Map VERSION_PER_EXECUTABLE = new ConcurrentHashMap<>(); + @Inject private InProcessCompiler inProcessCompiler; @@ -138,24 +142,29 @@ private String getInProcessJavacVersion() throws CompilerException { } private String getOutOfProcessJavacVersion(String executable) throws CompilerException { - Commandline cli = new Commandline(); - cli.setExecutable(executable); - /* - * The option "-version" should be supported by javac since 1.6 (https://docs.oracle.com/javase/6/docs/technotes/tools/solaris/javac.html) - * up to 21 (https://docs.oracle.com/en/java/javase/21/docs/specs/man/javac.html#standard-options) - */ - cli.addArguments(new String[] {"-version"}); // - CommandLineUtils.StringStreamConsumer out = new CommandLineUtils.StringStreamConsumer(); - try { - int exitCode = CommandLineUtils.executeCommandLine(cli, out, out); - if (exitCode != 0) { - throw new CompilerException("Could not retrieve version from " + executable + ". Exit code " + exitCode - + ", Output: " + out.getOutput()); + String version = VERSION_PER_EXECUTABLE.get(executable); + if (version == null) { + Commandline cli = new Commandline(); + cli.setExecutable(executable); + /* + * The option "-version" should be supported by javac since 1.6 (https://docs.oracle.com/javase/6/docs/technotes/tools/solaris/javac.html) + * up to 21 (https://docs.oracle.com/en/java/javase/21/docs/specs/man/javac.html#standard-options) + */ + cli.addArguments(new String[] {"-version"}); // + CommandLineUtils.StringStreamConsumer out = new CommandLineUtils.StringStreamConsumer(); + try { + int exitCode = CommandLineUtils.executeCommandLine(cli, out, out); + if (exitCode != 0) { + throw new CompilerException("Could not retrieve version from " + executable + ". Exit code " + + exitCode + ", Output: " + out.getOutput()); + } + } catch (CommandLineException e) { + throw new CompilerException("Error while executing the external compiler " + executable, e); } - } catch (CommandLineException e) { - throw new CompilerException("Error while executing the external compiler " + executable, e); + version = extractMajorAndMinorVersion(out.getOutput()); + VERSION_PER_EXECUTABLE.put(executable, version); } - return extractMajorAndMinorVersion(out.getOutput()); + return version; } static String extractMajorAndMinorVersion(String text) { @@ -435,7 +444,7 @@ public static String[] buildCompilerArguments( * Represents a particular Java version (through their according version prefixes) */ enum JavaVersion { - JAVA_1_3("1.3", "1.2", "1.1", "1.0"), + JAVA_1_3_OR_OLDER("1.3", "1.2", "1.1", "1.0"), JAVA_1_4("1.4"), JAVA_1_5("1.5"), JAVA_1_6("1.6"), From 13f1aecca291934e144347cc3ccaa5bd89412163 Mon Sep 17 00:00:00 2001 From: Konrad Windszus Date: Fri, 2 Feb 2024 11:19:18 +0100 Subject: [PATCH 3/4] Deprecate compilerVersion in CompilerConfiguration --- .../plexus/compiler/CompilerConfiguration.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/plexus-compiler-api/src/main/java/org/codehaus/plexus/compiler/CompilerConfiguration.java b/plexus-compiler-api/src/main/java/org/codehaus/plexus/compiler/CompilerConfiguration.java index 2907988e..1ab43ea1 100644 --- a/plexus-compiler-api/src/main/java/org/codehaus/plexus/compiler/CompilerConfiguration.java +++ b/plexus-compiler-api/src/main/java/org/codehaus/plexus/compiler/CompilerConfiguration.java @@ -500,10 +500,20 @@ public void setOptimize(boolean optimize) { this.optimize = optimize; } + /** + * @deprecated Don't use any longer because this is just the configured version which does not necessarily match the version + * of the actually executed compiler binary + */ + @Deprecated public String getCompilerVersion() { return compilerVersion; } + /** + * @deprecated Don't use any longer because this is just the configured version which does not necessarily match the version + * of the actually executed compiler binary + */ + @Deprecated public void setCompilerVersion(String compilerVersion) { this.compilerVersion = compilerVersion; } From d8187847a69b18cd3a6cdf8cf48225ba419051c6 Mon Sep 17 00:00:00 2001 From: Konrad Windszus Date: Fri, 2 Feb 2024 11:26:01 +0100 Subject: [PATCH 4/4] reformat --- .../org/codehaus/plexus/compiler/CompilerConfiguration.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plexus-compiler-api/src/main/java/org/codehaus/plexus/compiler/CompilerConfiguration.java b/plexus-compiler-api/src/main/java/org/codehaus/plexus/compiler/CompilerConfiguration.java index 1ab43ea1..a1680c2e 100644 --- a/plexus-compiler-api/src/main/java/org/codehaus/plexus/compiler/CompilerConfiguration.java +++ b/plexus-compiler-api/src/main/java/org/codehaus/plexus/compiler/CompilerConfiguration.java @@ -501,7 +501,7 @@ public void setOptimize(boolean optimize) { } /** - * @deprecated Don't use any longer because this is just the configured version which does not necessarily match the version + * @deprecated Don't use any longer because this is just the configured version which does not necessarily match the version * of the actually executed compiler binary */ @Deprecated @@ -510,7 +510,7 @@ public String getCompilerVersion() { } /** - * @deprecated Don't use any longer because this is just the configured version which does not necessarily match the version + * @deprecated Don't use any longer because this is just the configured version which does not necessarily match the version * of the actually executed compiler binary */ @Deprecated