Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#420: fix detection of java version when JAVA_TOOL_OPTIONS is set #426

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,16 @@ protected static class Messages {
private final Deque<Class<?>> javacClasses = new ConcurrentLinkedDeque<>();

private static final Pattern JAVA_MAJOR_AND_MINOR_VERSION_PATTERN = Pattern.compile("\\d+(\\.\\d+)?");
private static final String JAVAC_PREFIX = "javac";

/**
* List of variable that may produce an output "Picked up %s: %s\n".
* <p>
* There may be more, but for Jenkins withMaven, this is enough.
*
* @see <a href="https://github.com/openjdk/jdk/blob/81e43114eca5199a0d816c02f50ecb6bc370135b/src/hotspot/share/runtime/arguments.cpp#L3074">arguments.cpp</a>
*/
private static final String[] PICKED_ENV_VARS = {"_JAVA_OPTIONS", "JAVA_TOOL_OPTIONS"};

/** Cache of javac version per executable (never invalidated) */
private static final Map<String, String> VERSION_PER_EXECUTABLE = new ConcurrentHashMap<>();
Expand Down Expand Up @@ -258,6 +268,7 @@ private String getOutOfProcessJavacVersion(String executable) throws CompilerExc
* 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);
Expand All @@ -268,20 +279,39 @@ private String getOutOfProcessJavacVersion(String executable) throws CompilerExc
} catch (CommandLineException e) {
throw new CompilerException("Error while executing the external compiler " + executable, e);
}
version = extractMajorAndMinorVersion(out.getOutput());

/*
* javac will output the content of JAVA_TOOL_OPTIONS which may (sic) contains javac: we are forced to remove it.
*/
String cleanedOutput = cleanPickedUp(out.getOutput(), CommandLineUtils.getSystemEnvVars());
version = extractMajorAndMinorVersion(cleanedOutput);
VERSION_PER_EXECUTABLE.put(executable, version);
}
return version;
}

static String extractMajorAndMinorVersion(String text) {
int javacIndex = text.indexOf("javac");
Matcher matcher = JAVA_MAJOR_AND_MINOR_VERSION_PATTERN.matcher(text);
if (!matcher.find()) {
int start = javacIndex == -1 ? 0 : javacIndex + JAVAC_PREFIX.length();
if (!matcher.find(start)) {
throw new IllegalArgumentException("Could not extract version from \"" + text + "\"");
}
return matcher.group();
}

static String cleanPickedUp(String text, Properties envvars) {
String ls = "\n";
String ntext = StringUtils.unifyLineSeparators(text, ls);
for (String env : PICKED_ENV_VARS) {
String value = StringUtils.unifyLineSeparators(envvars.getProperty(env), ls);
if (value != null) {
ntext = ntext.replace(String.format("Picked up %s: %s\n", env, value), "");
}
}
return ntext;
}

@Override
public CompilerResult performCompile(CompilerConfiguration config) throws CompilerException {
File destinationDir = new File(config.getOutputLocation());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,12 @@

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.io.StringReader;
import java.nio.charset.StandardCharsets;
import java.util.List;
import java.util.Properties;
import java.util.stream.Stream;

import org.codehaus.plexus.compiler.CompilerMessage;
Expand All @@ -13,6 +17,7 @@
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
import org.opentest4j.AssertionFailedError;

import static org.codehaus.plexus.compiler.javac.JavacCompiler.Messages.*;
import static org.hamcrest.CoreMatchers.endsWith;
Expand Down Expand Up @@ -121,5 +126,51 @@ 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"));
assertEquals("1.8", JavacCompiler.extractMajorAndMinorVersion("1.3.4\njavac 1.8.0_432"));
}

@Test
void testCleanPickedUp() {
assertEquals(
"This text contains CRLF\n",
JavacCompiler.cleanPickedUp("This text contains CRLF\r\n", new Properties()));

// files were generated by using:
// declare -x JAVA_TOOL_OPTIONS=$'-Daaa\n\n-Dxxx'
// declare -x _JAVA_OPTIONS="-Dccc"
// "${JAVA_8_HOME}/bin/javac" -version >
// plexus-compilers/plexus-compiler-javac/src/test/resources/org/codehaus/plexus/compiler/javac/java8_pickedUp.txt 2>&1
// "${JAVA_17_HOME}/bin/javac" -version >
// plexus-compilers/plexus-compiler-javac/src/test/resources/org/codehaus/plexus/compiler/javac/java17_pickedUp.txt 2>&1
// "${JAVA_21_HOME}/bin/javac" -version >
// plexus-compilers/plexus-compiler-javac/src/test/resources/org/codehaus/plexus/compiler/javac/java21_pickedUp.txt 2>&1
Properties envvars = new Properties();
envvars.setProperty("JAVA_TOOL_OPTIONS", "-Daaa\r\n\r\n-Dxxx");
envvars.setProperty("_JAVA_OPTIONS", "-Dccc");

assertEquals(
"javac 1.8.0_432\n", JavacCompiler.cleanPickedUp(readAllLines("java8_pickedUp.txt", "\n"), envvars));
assertEquals(
"javac 17.0.13\n", JavacCompiler.cleanPickedUp(readAllLines("java17_pickedUp.txt", "\r\n"), envvars));
assertEquals("javac 21.0.5\n", JavacCompiler.cleanPickedUp(readAllLines("java21_pickedUp.txt", "\r"), envvars));
}

private String readAllLines(String resource, String ls) {
try (InputStream is = this.getClass().getResourceAsStream(resource)) {
if (is == null) {
throw new AssertionFailedError("No such resource: " + resource + " in class " + this.getClass());
}
try (InputStreamReader isr = new InputStreamReader(is, StandardCharsets.UTF_8);
BufferedReader br = new BufferedReader(isr)) {
StringBuilder sb = new StringBuilder();
for (String line = null; (line = br.readLine()) != null; ) {
sb.append(line).append(ls);
}
return sb.toString();
}
} catch (IOException e) {
throw new AssertionFailedError(
"Could not fetch lines of resource: " + resource + " in class " + this.getClass(), e);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Picked up JAVA_TOOL_OPTIONS: -Daaa

-Dxxx
Picked up _JAVA_OPTIONS: -Dccc
javac 17.0.13
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Picked up JAVA_TOOL_OPTIONS: -Daaa

-Dxxx
Picked up _JAVA_OPTIONS: -Dccc
javac 21.0.5
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Picked up JAVA_TOOL_OPTIONS: -Daaa

-Dxxx
Picked up _JAVA_OPTIONS: -Dccc
javac 1.8.0_432
Loading