Skip to content

Commit

Permalink
Merge pull request #24014 Restore an edge case of non-javac toolchain…
Browse files Browse the repository at this point in the history
… tools being used as a Java compiler

Gradle 8 introduced unconditional usage of Java toolchains for Java compilation. However, providing custom tool paths (custom Java home or a compiler executable path) was still supported.

In addition to that, Gradle 8 protect users from doubly-configuring the toolchain via the tool property and a Java home/executable. Validations were added to make sure that toolchain from a custom path agrees with the final resolved toolchain property. In particular, the path to the executable is checked to exactly match the path to `javac` within the toolchain. This, however, made it impossible to use custom Java compilers that rely on being started as a JVM via `java` launcher tool.

This PR relaxes the checks to only validate that a custom executable path leads to a tool within the same toolchain without checking which exact tool it is. In particular, a path to the `java` tool can be specified.

Fixes: #23990

Co-authored-by: Alex Semin <asemin@gradle.com>
  • Loading branch information
bot-gradle and alllex committed Feb 28, 2023
2 parents 5468606 + 7ce1d3d commit e87ed79
Show file tree
Hide file tree
Showing 5 changed files with 136 additions and 39 deletions.
Expand Up @@ -469,6 +469,92 @@ class JavaCompileToolchainIntegrationTest extends AbstractIntegrationSpec implem
JavaVersion.current() | "[deprecation] foo() in Foo has been deprecated"
}

@Issue("https://github.com/gradle/gradle/issues/23990")
def "can compile with a custom compiler executable"() {
def otherJdk = AvailableJavaHomes.getJdk(JavaVersion.current())
def jdk = AvailableJavaHomes.getDifferentVersion {
def v = it.languageVersion.majorVersion.toInteger()
11 <= v && v <= 18 // Java versions supported by ECJ releases used in the test
}

buildFile << """
plugins {
id("java")
}
java {
toolchain {
languageVersion = JavaLanguageVersion.of(${otherJdk.javaVersion.majorVersion})
}
}
configurations {
ecj {
canBeConsumed = false
canBeResolved = true
}
}
${mavenCentralRepository()}
dependencies {
def changed = providers.gradleProperty("changed").isPresent()
ecj(!changed ? "org.eclipse.jdt:ecj:3.31.0" : "org.eclipse.jdt:ecj:3.32.0")
}
// Make sure the provider is up-to-date only if the ECJ classpath does not change
class EcjClasspathProvider implements CommandLineArgumentProvider {
@Classpath
final FileCollection ecjClasspath
EcjClasspathProvider(FileCollection ecjClasspath) {
this.ecjClasspath = ecjClasspath
}
@Override
List<String> asArguments() {
return ["-cp", ecjClasspath.asPath, "org.eclipse.jdt.internal.compiler.batch.Main"]
}
}
compileJava {
def customJavaLauncher = javaToolchains.launcherFor {
languageVersion.set(JavaLanguageVersion.of(${jdk.javaVersion.majorVersion}))
}.get()
// ECJ does not support generating JNI headers
options.headerOutputDirectory.set(provider { null })
options.fork = true
options.forkOptions.executable = customJavaLauncher.executablePath.asFile.absolutePath
options.forkOptions.jvmArgumentProviders.add(new EcjClasspathProvider(configurations.ecj))
}
"""

when:
withInstallations(jdk, otherJdk).run(":compileJava", "--info")
then:
executedAndNotSkipped(":compileJava")
outputContains("Compiling with toolchain '${jdk.javaHome.absolutePath}'")
outputContains("Compiling with Java command line compiler '${jdk.javaExecutable.absolutePath}'")
classJavaVersion(javaClassFile("Foo.class")) == jdk.javaVersion

// Test up-to-date checks
when:
withInstallations(jdk, otherJdk).run(":compileJava")
then:
skipped(":compileJava")

when:
withInstallations(jdk, otherJdk).run(":compileJava", "-Pchanged")
then:
executedAndNotSkipped(":compileJava")

when:
withInstallations(jdk, otherJdk).run(":compileJava", "-Pchanged")
then:
skipped(":compileJava")
}

private TestFile configureForkOptionsExecutable(Jvm jdk) {
buildFile << """
compileJava {
Expand Down
Expand Up @@ -42,17 +42,7 @@ public T create() {
}

if (compileOptions.isFork()) {
File customJavaHome = compileOptions.getForkOptions().getJavaHome();
if (customJavaHome != null) {
return getCommandLineSpec(Jvm.forHome(customJavaHome).getJavacExecutable());
}

String customExecutable = compileOptions.getForkOptions().getExecutable();
if (customExecutable != null) {
return getCommandLineSpec(JavaExecutableUtils.resolveExecutable(customExecutable));
}

return getForkingSpec(Jvm.current().getJavaHome());
return chooseSpecFromCompileOptions(Jvm.current().getJavaHome());
}

return getDefaultSpec();
Expand All @@ -65,13 +55,7 @@ private T chooseSpecForToolchain() {
}

if (compileOptions.isFork()) {
// Presence of the fork options means that the user has explicitly requested a command-line compiler
if (compileOptions.getForkOptions().getJavaHome() != null || compileOptions.getForkOptions().getExecutable() != null) {
// We use the toolchain path because the fork options must agree with the selected toolchain
return getCommandLineSpec(Jvm.forHome(toolchainJavaHome).getJavacExecutable());
}

return getForkingSpec(toolchainJavaHome);
return chooseSpecFromCompileOptions(toolchainJavaHome);
}

if (!toolchain.isCurrentJvm()) {
Expand All @@ -81,6 +65,20 @@ private T chooseSpecForToolchain() {
return getDefaultSpec();
}

private T chooseSpecFromCompileOptions(File fallbackJavaHome) {
File forkJavaHome = compileOptions.getForkOptions().getJavaHome();
if (forkJavaHome != null) {
return getCommandLineSpec(Jvm.forHome(forkJavaHome).getJavacExecutable());
}

String forkExecutable = compileOptions.getForkOptions().getExecutable();
if (forkExecutable != null) {
return getCommandLineSpec(JavaExecutableUtils.resolveExecutable(forkExecutable));
}

return getForkingSpec(fallbackJavaHome);
}

abstract protected T getCommandLineSpec(File executable);

abstract protected T getForkingSpec(File javaHome);
Expand Down
Expand Up @@ -75,8 +75,6 @@
import java.util.List;
import java.util.concurrent.Callable;

import static com.google.common.base.Preconditions.checkState;

/**
* Compiles Java source files.
*
Expand Down Expand Up @@ -258,19 +256,26 @@ private void validateForkOptionsMatchToolchain() {

JavaCompiler javaCompilerTool = getJavaCompiler().get();
File toolchainJavaHome = javaCompilerTool.getMetadata().getInstallationPath().getAsFile();
File toolchainExecutable = javaCompilerTool.getExecutablePath().getAsFile();

ForkOptions forkOptions = getOptions().getForkOptions();
File customJavaHome = forkOptions.getJavaHome();
checkState(
customJavaHome == null || customJavaHome.equals(toolchainJavaHome),
"Toolchain from `javaHome` property on `ForkOptions` does not match toolchain from `javaCompiler` property"
);
if (customJavaHome != null) {
JavaExecutableUtils.validateMatchingFiles(
customJavaHome, "Toolchain from `javaHome` property on `ForkOptions`",
toolchainJavaHome, "toolchain from `javaCompiler` property"
);
}

String customExecutablePath = forkOptions.getExecutable();
JavaExecutableUtils.validateExecutable(
customExecutablePath, "Toolchain from `executable` property on `ForkOptions`",
toolchainExecutable, "toolchain from `javaCompiler` property");
if (customExecutablePath != null) {
// We do not match the custom executable against the compiler executable from the toolchain (javac),
// because the custom executable can be set to the path of another tool in the toolchain such as a launcher (java).
File customExecutableJavaHome = JavaExecutableUtils.resolveJavaHomeOfExecutable(customExecutablePath);
JavaExecutableUtils.validateMatchingFiles(
customExecutableJavaHome, "Toolchain from `executable` property on `ForkOptions`",
toolchainJavaHome, "toolchain from `javaCompiler` property"
);
}
}

private boolean isToolchainCompatibleWithJava8() {
Expand Down
Expand Up @@ -39,32 +39,43 @@ public static File resolveExecutable(String executable) {
.undocumented()
.nagUser();
}
if (!executableFile.getAbsoluteFile().exists()) {
File executableAbsoluteFile = executableFile.getAbsoluteFile();
if (!executableAbsoluteFile.exists()) {
throw new InvalidUserDataException("The configured executable does not exist (" + executableFile.getAbsolutePath() + ")");
}
if (executableFile.getAbsoluteFile().isDirectory()) {
if (executableAbsoluteFile.isDirectory()) {
throw new InvalidUserDataException("The configured executable is a directory (" + executableFile.getAbsolutePath() + ")");
}
return executableFile;
return executableAbsoluteFile;
}

public static File resolveJavaHomeOfExecutable(String executable) {
// Relying on the layout of the toolchain distribution: <JAVA HOME>/bin/<executable>
return resolveExecutable(executable).getParentFile().getParentFile();
}


public static void validateExecutable(@Nullable String executable, String executableDescription, File referenceFile, String referenceDescription) {
if (executable == null) {
return;
}

File executableFile = resolveExecutable(executable);
if (executableFile.equals(referenceFile)) {
validateMatchingFiles(executableFile, executableDescription, referenceFile, referenceDescription);
}

public static void validateMatchingFiles(File customFile, String customDescription, File referenceFile, String referenceDescription) {
if (customFile.equals(referenceFile)) {
return;
}

File canonicalExecutableFile = canonicalFile(executableFile);
File canonicalCustomFile = canonicalFile(customFile);
File canonicalReferenceFile = canonicalFile(referenceFile);
if (canonicalExecutableFile.equals(canonicalReferenceFile)) {
if (canonicalCustomFile.equals(canonicalReferenceFile)) {
return;
}

throw new IllegalStateException(executableDescription + " does not match " + referenceDescription + ".");
throw new IllegalStateException(customDescription + " does not match " + referenceDescription + ".");
}

private static File canonicalFile(File file) {
Expand Down
Expand Up @@ -74,10 +74,7 @@ public static SpecificInstallationToolchainSpec fromJavaHome(ObjectFactory objec
}

public static SpecificInstallationToolchainSpec fromJavaExecutable(ObjectFactory objectFactory, String executable) {
File executableFile = JavaExecutableUtils.resolveExecutable(executable);
// Relying on the layout of the toolchain distribution: <JAVA HOME>/bin/<executable>
final File parentJavaHome = executableFile.getParentFile().getParentFile();
return new SpecificInstallationToolchainSpec(objectFactory, parentJavaHome);
return new SpecificInstallationToolchainSpec(objectFactory, JavaExecutableUtils.resolveJavaHomeOfExecutable(executable));
}

@Override
Expand Down

0 comments on commit e87ed79

Please sign in to comment.