Skip to content

Commit

Permalink
Avoid forking if Gradle already has appropriate add-exports/add-opens
Browse files Browse the repository at this point in the history
…passed through org.gradle.jvmargs property.
  • Loading branch information
tbroyer committed Apr 22, 2023
1 parent 3adff47 commit f981cfc
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 6 deletions.
9 changes: 6 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -157,17 +157,20 @@ Starting with JDK 16, due to [JEP 396: Strongly Encapsulate JDK Internals by Def

The plugin will automatically [use a forking compiler][CompileOptions.fork]
and pass the necessary [JVM arguments][BaseForkOptions.getJvmArgs]
whenever it detects such a JDK is being used and ErrorProne is enabled.
whenever it detects such a JDK is being used and ErrorProne is enabled
(unless the Gradle daemon's JVM already was given the appropriate options [through `org.gradle.jvmargs`][org.gradle.jvmargs]).

That detection will only take into account the [toolchain][gradle-toolchains] used by the `JavaCompile` task,
or the JDK used to run Gradle in case no toolchain is being used.
The plugin will ignore any task that forks and defines either [a `javaHome`][ForkOptions.setJavaHome] or [an `executable`][ForkOptions.setExecutable],
and thus won't configure the JVM arguments if you're e.g. running Gradle with an older JDK and forking the compilation tasks to use JDK 17.

Note that the plugin also configures the JVM arguments for any JDK between 9 and 15 to silence related warnings,
but they will then only be used if the task is explicitly configured for forking.
Note that the plugin also configures the JVM arguments for any JDK above version 9 to silence related warnings,
but they will then only be used if the task is explicitly configured for forking
(or if the configured toolchain is incompatible with the JDK used to run Gradle, which will then implicitly fork a compiler daemon).

[jep396]: https://openjdk.java.net/jeps/396
[org.gradle.jvmargs]: https://docs.gradle.org/current/userguide/build_environment.html#sec:configuring_jvm_memory

## Custom Error Prone checks

Expand Down
35 changes: 34 additions & 1 deletion src/main/kotlin/net/ltgt/gradle/errorprone/ErrorPronePlugin.kt
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,39 @@ class ErrorPronePlugin @Inject constructor(
"--add-opens=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED",
"--add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED"
)

private val CURRENT_JVM_NEEDS_FORKING by lazy {
// Needs bootclasspath
JavaVersion.current() == JavaVersion.VERSION_1_8 || (
// Needs --add-exports and --add-opens
JavaVersion.current() >= JavaVersion.VERSION_16 &&
try {
sequenceOf(
"com.sun.tools.javac.api.BasicJavacTask",
"com.sun.tools.javac.api.JavacTrees",
"com.sun.tools.javac.file.JavacFileManager",
"com.sun.tools.javac.main.JavaCompiler",
"com.sun.tools.javac.model.JavacElements",
"com.sun.tools.javac.parser.JavacParser",
"com.sun.tools.javac.processing.JavacProcessingEnvironment",
"com.sun.tools.javac.tree.JCTree",
"com.sun.tools.javac.util.JCDiagnostic"
).any {
val klass = Class.forName(it)
return@any !klass.module.isExported(klass.packageName, this::class.java.classLoader.unnamedModule)
} &&
sequenceOf(
"com.sun.tools.javac.code.Symbol",
"com.sun.tools.javac.comp.Enter"
).any {
val klass = Class.forName(it)
return@any !klass.module.isOpen(klass.packageName, this::class.java.classLoader.unnamedModule)
}
} catch (e: ClassNotFoundException) {
true
}
)
}
}

override fun apply(project: Project) {
Expand Down Expand Up @@ -103,7 +136,7 @@ class ErrorPronePlugin @Inject constructor(
if (!errorproneOptions.isEnabled.getOrElse(false)) return@doFirst
jvmArgumentProvider.compilerVersion?.let {
if (it < JavaVersion.VERSION_1_8) throw UnsupportedOperationException(TOO_OLD_TOOLCHAIN_ERROR_MESSAGE)
if (it == JavaVersion.VERSION_1_8 || it >= JavaVersion.VERSION_16) options.isFork = true
if (it == JavaVersion.VERSION_1_8 || (it == JavaVersion.current() && CURRENT_JVM_NEEDS_FORKING)) options.isFork = true
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package net.ltgt.gradle.errorprone

import com.google.common.truth.Truth.assertThat
import com.google.common.truth.TruthJUnit.assume
import org.gradle.api.JavaVersion
import org.gradle.testkit.runner.BuildResult
import org.gradle.testkit.runner.TaskOutcome
import org.gradle.util.GradleVersion
Expand Down Expand Up @@ -230,7 +231,10 @@ class ToolchainsIntegrationTest : AbstractPluginIntegrationTest() {
}

@Test
fun `configure forking in Java 16+ VM`() {
fun `configure forking in Java 16+ VM (unless implicitly forked by incompatible toolchain)`() {
// https://github.com/gradle/gradle/issues/16857#issuecomment-931610187
assume().that(GradleVersion.version(testGradleVersion)).isAtLeast(GradleVersion.version("7.0"))

// given
buildFile.appendText(
"""
Expand All @@ -248,7 +252,53 @@ class ToolchainsIntegrationTest : AbstractPluginIntegrationTest() {
// then
result.assumeToolchainAvailable()
assertThat(result.task(":compileJava")?.outcome).isEqualTo(TaskOutcome.SUCCESS)
assertThat(result.output).contains(FORKED)
assertThat(result.output).contains(if (JavaVersion.current() == JavaVersion.VERSION_17) FORKED else NOT_FORKED)
assertThat(result.output).contains(JVM_ARGS_STRONG_ENCAPSULATION)
// Check that the configured jvm arg is preserved
assertThat(result.output).contains(jvmArg("-XshowSettings"))
}

// check that it doesn't mess with task avoidance

// when
testProjectDir.buildWithArgsAndFail("compileJava").also { result ->
// then
result.assumeToolchainAvailable()
assertThat(result.task(":compileJava")?.outcome).isEqualTo(TaskOutcome.UP_TO_DATE)
}
}

@Test
fun `does not configure forking in Java 16+ VM if current JVM has appropriate JVM args`() {
// https://github.com/gradle/gradle/issues/16857#issuecomment-931610187
assume().that(GradleVersion.version(testGradleVersion)).isAtLeast(GradleVersion.version("7.0"))
assume().withMessage("isJava16Compatible").that(JavaVersion.current()).isAtLeast(JavaVersion.VERSION_16)

testProjectDir.resolve("gradle.properties").appendText(
"""
org.gradle.jvmargs=-Xmx512m "-XX:MaxMetaspaceSize=384m" ${ErrorPronePlugin.JVM_ARGS_STRONG_ENCAPSULATION.joinToString(" ")}
""".trimIndent()
)

// given
buildFile.appendText(
"""
java {
toolchain {
languageVersion.set(JavaLanguageVersion.of(${JavaVersion.current().majorVersion}))
}
}
""".trimIndent()
)

// when
testProjectDir.buildWithArgsAndFail("compileJava").also { result ->
// then
result.assumeToolchainAvailable()
assertThat(result.task(":compileJava")?.outcome).isEqualTo(TaskOutcome.SUCCESS)
assertThat(result.output).contains(NOT_FORKED)
assertThat(result.output).contains(JVM_ARGS_STRONG_ENCAPSULATION)
// Check that the configured jvm arg is preserved
assertThat(result.output).contains(jvmArg("-XshowSettings"))
Expand Down

0 comments on commit f981cfc

Please sign in to comment.