From 6b9f03929d6b4225ae78cbf888b3d582e953e4c1 Mon Sep 17 00:00:00 2001 From: Alex Semin Date: Thu, 23 Feb 2023 13:37:16 +0100 Subject: [PATCH 1/6] Support non-javac toolchain tools to be used as a Java compiler --- .../fixtures/jvm/JavaToolchainFixture.groovy | 2 +- ...JavaCompileToolchainIntegrationTest.groovy | 49 +++++++++++++++++++ .../AbstractJavaCompileSpecFactory.java | 34 ++++++------- .../gradle/api/tasks/compile/JavaCompile.java | 15 ++++-- .../internal/JavaExecutableUtils.java | 6 +++ .../SpecificInstallationToolchainSpec.java | 5 +- 6 files changed, 84 insertions(+), 27 deletions(-) diff --git a/subprojects/internal-integ-testing/src/main/groovy/org/gradle/integtests/fixtures/jvm/JavaToolchainFixture.groovy b/subprojects/internal-integ-testing/src/main/groovy/org/gradle/integtests/fixtures/jvm/JavaToolchainFixture.groovy index a04e2546b5e4..ff9788a1e1c4 100644 --- a/subprojects/internal-integ-testing/src/main/groovy/org/gradle/integtests/fixtures/jvm/JavaToolchainFixture.groovy +++ b/subprojects/internal-integ-testing/src/main/groovy/org/gradle/integtests/fixtures/jvm/JavaToolchainFixture.groovy @@ -97,7 +97,7 @@ trait JavaToolchainFixture { * Returns the Java version from the compiled class bytecode. */ JavaVersion classJavaVersion(File classFile) { - assert classFile.exists() + assert classFile.exists(), "Class file ${classFile} does not exist" return JavaVersion.forClass(classFile.bytes) } } diff --git a/subprojects/language-java/src/integTest/groovy/org/gradle/api/tasks/compile/JavaCompileToolchainIntegrationTest.groovy b/subprojects/language-java/src/integTest/groovy/org/gradle/api/tasks/compile/JavaCompileToolchainIntegrationTest.groovy index 35ed63dbb288..3cf6ab830201 100644 --- a/subprojects/language-java/src/integTest/groovy/org/gradle/api/tasks/compile/JavaCompileToolchainIntegrationTest.groovy +++ b/subprojects/language-java/src/integTest/groovy/org/gradle/api/tasks/compile/JavaCompileToolchainIntegrationTest.groovy @@ -469,6 +469,55 @@ class JavaCompileToolchainIntegrationTest extends AbstractIntegrationSpec implem JavaVersion.current() | "[deprecation] foo() in Foo has been deprecated" } + @Issue("https://github.com/gradle/gradle/issues/23990") + @Requires(TestPrecondition.JDK11_OR_LATER) + def "can compile with a custom compiler executable"() { + def jdk = AvailableJavaHomes.getJdk(JavaVersion.current()) + + buildFile << """ + plugins { + id("java") + } + + configurations { + ecj { + canBeConsumed = false + canBeResolved = true + } + } + + ${mavenCentralRepository()} + + dependencies { + ecj("org.eclipse.jdt:ecj:3.32.0") + } + + 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({ + ["-cp", configurations.ecj.asPath, "org.eclipse.jdt.internal.compiler.batch.Main"] + } as CommandLineArgumentProvider) + } + """ + + when: + withInstallations(jdk).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 + } + private TestFile configureForkOptionsExecutable(Jvm jdk) { buildFile << """ compileJava { diff --git a/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/AbstractJavaCompileSpecFactory.java b/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/AbstractJavaCompileSpecFactory.java index 27f408f62e1e..b55778a1573e 100644 --- a/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/AbstractJavaCompileSpecFactory.java +++ b/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/AbstractJavaCompileSpecFactory.java @@ -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(); @@ -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()) { @@ -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); diff --git a/subprojects/language-java/src/main/java/org/gradle/api/tasks/compile/JavaCompile.java b/subprojects/language-java/src/main/java/org/gradle/api/tasks/compile/JavaCompile.java index e6749cb5a522..b05556bec2d1 100644 --- a/subprojects/language-java/src/main/java/org/gradle/api/tasks/compile/JavaCompile.java +++ b/subprojects/language-java/src/main/java/org/gradle/api/tasks/compile/JavaCompile.java @@ -258,7 +258,6 @@ 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(); @@ -268,9 +267,17 @@ private void validateForkOptionsMatchToolchain() { ); String customExecutablePath = forkOptions.getExecutable(); - JavaExecutableUtils.validateExecutable( - customExecutablePath, "Toolchain from `executable` property on `ForkOptions`", - toolchainExecutable, "toolchain from `javaCompiler` property"); + if (customExecutablePath == null) { + return; + } + + // 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); + checkState( + customExecutableJavaHome.equals(toolchainJavaHome), + "Toolchain from `executable` property on `ForkOptions` does not match toolchain from `javaCompiler` property" + ); } private boolean isToolchainCompatibleWithJava8() { diff --git a/subprojects/platform-jvm/src/main/java/org/gradle/jvm/toolchain/internal/JavaExecutableUtils.java b/subprojects/platform-jvm/src/main/java/org/gradle/jvm/toolchain/internal/JavaExecutableUtils.java index a0ca762531b2..a25c6d19cf4e 100644 --- a/subprojects/platform-jvm/src/main/java/org/gradle/jvm/toolchain/internal/JavaExecutableUtils.java +++ b/subprojects/platform-jvm/src/main/java/org/gradle/jvm/toolchain/internal/JavaExecutableUtils.java @@ -48,6 +48,12 @@ public static File resolveExecutable(String executable) { return executableFile; } + public static File resolveJavaHomeOfExecutable(String executable) { + // Relying on the layout of the toolchain distribution: /bin/ + return resolveExecutable(executable).getParentFile().getParentFile(); + } + + public static void validateExecutable(@Nullable String executable, String executableDescription, File referenceFile, String referenceDescription) { if (executable == null) { return; diff --git a/subprojects/platform-jvm/src/main/java/org/gradle/jvm/toolchain/internal/SpecificInstallationToolchainSpec.java b/subprojects/platform-jvm/src/main/java/org/gradle/jvm/toolchain/internal/SpecificInstallationToolchainSpec.java index 9472b25c9903..6bd436759f48 100644 --- a/subprojects/platform-jvm/src/main/java/org/gradle/jvm/toolchain/internal/SpecificInstallationToolchainSpec.java +++ b/subprojects/platform-jvm/src/main/java/org/gradle/jvm/toolchain/internal/SpecificInstallationToolchainSpec.java @@ -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: /bin/ - final File parentJavaHome = executableFile.getParentFile().getParentFile(); - return new SpecificInstallationToolchainSpec(objectFactory, parentJavaHome); + return new SpecificInstallationToolchainSpec(objectFactory, JavaExecutableUtils.resolveJavaHomeOfExecutable(executable)); } @Override From ba332d7fd9035940a7b4c6bd4a710d63904399ba Mon Sep 17 00:00:00 2001 From: Alex Semin Date: Thu, 23 Feb 2023 14:28:43 +0100 Subject: [PATCH 2/6] Resolve java executable to absolute files --- .../gradle/jvm/toolchain/internal/JavaExecutableUtils.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/subprojects/platform-jvm/src/main/java/org/gradle/jvm/toolchain/internal/JavaExecutableUtils.java b/subprojects/platform-jvm/src/main/java/org/gradle/jvm/toolchain/internal/JavaExecutableUtils.java index a25c6d19cf4e..0fdff895e068 100644 --- a/subprojects/platform-jvm/src/main/java/org/gradle/jvm/toolchain/internal/JavaExecutableUtils.java +++ b/subprojects/platform-jvm/src/main/java/org/gradle/jvm/toolchain/internal/JavaExecutableUtils.java @@ -39,13 +39,14 @@ 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) { From c11d14a66cf819d659e242760f430288f47ff028 Mon Sep 17 00:00:00 2001 From: Alex Semin Date: Thu, 23 Feb 2023 14:29:13 +0100 Subject: [PATCH 3/6] Match Java homes canonically --- .../gradle/api/tasks/compile/JavaCompile.java | 30 +++++++++---------- .../internal/JavaExecutableUtils.java | 12 +++++--- 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/subprojects/language-java/src/main/java/org/gradle/api/tasks/compile/JavaCompile.java b/subprojects/language-java/src/main/java/org/gradle/api/tasks/compile/JavaCompile.java index b05556bec2d1..6193838b824e 100644 --- a/subprojects/language-java/src/main/java/org/gradle/api/tasks/compile/JavaCompile.java +++ b/subprojects/language-java/src/main/java/org/gradle/api/tasks/compile/JavaCompile.java @@ -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. * @@ -261,23 +259,23 @@ private void validateForkOptionsMatchToolchain() { 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(); - if (customExecutablePath == null) { - return; + 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" + ); } - - // 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); - checkState( - customExecutableJavaHome.equals(toolchainJavaHome), - "Toolchain from `executable` property on `ForkOptions` does not match toolchain from `javaCompiler` property" - ); } private boolean isToolchainCompatibleWithJava8() { diff --git a/subprojects/platform-jvm/src/main/java/org/gradle/jvm/toolchain/internal/JavaExecutableUtils.java b/subprojects/platform-jvm/src/main/java/org/gradle/jvm/toolchain/internal/JavaExecutableUtils.java index 0fdff895e068..a3a72b7826d7 100644 --- a/subprojects/platform-jvm/src/main/java/org/gradle/jvm/toolchain/internal/JavaExecutableUtils.java +++ b/subprojects/platform-jvm/src/main/java/org/gradle/jvm/toolchain/internal/JavaExecutableUtils.java @@ -61,17 +61,21 @@ public static void validateExecutable(@Nullable String executable, String execut } 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) { From 74bdc8a77b04534ad7868f81ee06f3e6e6de4caf Mon Sep 17 00:00:00 2001 From: Alex Semin Date: Mon, 27 Feb 2023 12:38:37 +0100 Subject: [PATCH 4/6] Add up-to-date checks for the custom-compiler test --- ...JavaCompileToolchainIntegrationTest.groovy | 49 ++++++++++++++++--- 1 file changed, 42 insertions(+), 7 deletions(-) diff --git a/subprojects/language-java/src/integTest/groovy/org/gradle/api/tasks/compile/JavaCompileToolchainIntegrationTest.groovy b/subprojects/language-java/src/integTest/groovy/org/gradle/api/tasks/compile/JavaCompileToolchainIntegrationTest.groovy index 3cf6ab830201..61ccc8ccf077 100644 --- a/subprojects/language-java/src/integTest/groovy/org/gradle/api/tasks/compile/JavaCompileToolchainIntegrationTest.groovy +++ b/subprojects/language-java/src/integTest/groovy/org/gradle/api/tasks/compile/JavaCompileToolchainIntegrationTest.groovy @@ -473,12 +473,19 @@ class JavaCompileToolchainIntegrationTest extends AbstractIntegrationSpec implem @Requires(TestPrecondition.JDK11_OR_LATER) def "can compile with a custom compiler executable"() { def jdk = AvailableJavaHomes.getJdk(JavaVersion.current()) + def otherJdk = AvailableJavaHomes.differentVersion buildFile << """ plugins { id("java") } + java { + toolchain { + languageVersion = JavaLanguageVersion.of(${otherJdk.javaVersion.majorVersion}) + } + } + configurations { ecj { canBeConsumed = false @@ -489,7 +496,23 @@ class JavaCompileToolchainIntegrationTest extends AbstractIntegrationSpec implem ${mavenCentralRepository()} dependencies { - ecj("org.eclipse.jdt:ecj:3.32.0") + 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 asArguments() { + return ["-cp", ecjClasspath.asPath, "org.eclipse.jdt.internal.compiler.batch.Main"] + } } compileJava { @@ -501,21 +524,33 @@ class JavaCompileToolchainIntegrationTest extends AbstractIntegrationSpec implem options.headerOutputDirectory.set(provider { null }) options.fork = true options.forkOptions.executable = customJavaLauncher.executablePath.asFile.absolutePath - - options.forkOptions.jvmArgumentProviders.add({ - ["-cp", configurations.ecj.asPath, "org.eclipse.jdt.internal.compiler.batch.Main"] - } as CommandLineArgumentProvider) + options.forkOptions.jvmArgumentProviders.add(new EcjClasspathProvider(configurations.ecj)) } """ when: - withInstallations(jdk).run(":compileJava", "--info") - + 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) { From 277e222727cc1c7b82584cf2b3ebfff2cbe17373 Mon Sep 17 00:00:00 2001 From: Alex Semin Date: Mon, 27 Feb 2023 18:55:16 +0100 Subject: [PATCH 5/6] Fix ECJ test by constraining Java versions --- .../compile/JavaCompileToolchainIntegrationTest.groovy | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/subprojects/language-java/src/integTest/groovy/org/gradle/api/tasks/compile/JavaCompileToolchainIntegrationTest.groovy b/subprojects/language-java/src/integTest/groovy/org/gradle/api/tasks/compile/JavaCompileToolchainIntegrationTest.groovy index 61ccc8ccf077..9d1ab6196686 100644 --- a/subprojects/language-java/src/integTest/groovy/org/gradle/api/tasks/compile/JavaCompileToolchainIntegrationTest.groovy +++ b/subprojects/language-java/src/integTest/groovy/org/gradle/api/tasks/compile/JavaCompileToolchainIntegrationTest.groovy @@ -470,10 +470,12 @@ class JavaCompileToolchainIntegrationTest extends AbstractIntegrationSpec implem } @Issue("https://github.com/gradle/gradle/issues/23990") - @Requires(TestPrecondition.JDK11_OR_LATER) def "can compile with a custom compiler executable"() { - def jdk = AvailableJavaHomes.getJdk(JavaVersion.current()) - def otherJdk = AvailableJavaHomes.differentVersion + 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 { From 7ce1d3d57943a97d2db9084af600819acfcc2918 Mon Sep 17 00:00:00 2001 From: Alex Semin Date: Tue, 28 Feb 2023 09:48:39 +0100 Subject: [PATCH 6/6] Remove unnecessary assert message --- .../gradle/integtests/fixtures/jvm/JavaToolchainFixture.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/subprojects/internal-integ-testing/src/main/groovy/org/gradle/integtests/fixtures/jvm/JavaToolchainFixture.groovy b/subprojects/internal-integ-testing/src/main/groovy/org/gradle/integtests/fixtures/jvm/JavaToolchainFixture.groovy index ff9788a1e1c4..a04e2546b5e4 100644 --- a/subprojects/internal-integ-testing/src/main/groovy/org/gradle/integtests/fixtures/jvm/JavaToolchainFixture.groovy +++ b/subprojects/internal-integ-testing/src/main/groovy/org/gradle/integtests/fixtures/jvm/JavaToolchainFixture.groovy @@ -97,7 +97,7 @@ trait JavaToolchainFixture { * Returns the Java version from the compiled class bytecode. */ JavaVersion classJavaVersion(File classFile) { - assert classFile.exists(), "Class file ${classFile} does not exist" + assert classFile.exists() return JavaVersion.forClass(classFile.bytes) } }