Skip to content

Commit

Permalink
No longer reset fork options when configuring forking compiler
Browse files Browse the repository at this point in the history
  • Loading branch information
tbroyer committed Oct 2, 2022
1 parent 7410cc0 commit 8958d29
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 22 deletions.
16 changes: 2 additions & 14 deletions src/main/kotlin/net/ltgt/gradle/errorprone/ErrorPronePlugin.kt
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import org.gradle.api.tasks.Nested
import org.gradle.api.tasks.Optional
import org.gradle.api.tasks.SourceSetContainer
import org.gradle.api.tasks.compile.CompileOptions
import org.gradle.api.tasks.compile.ForkOptions
import org.gradle.api.tasks.compile.JavaCompile
import org.gradle.kotlin.dsl.* // ktlint-disable no-wildcard-imports
import org.gradle.process.CommandLineArgumentProvider
Expand Down Expand Up @@ -91,7 +90,7 @@ Add a dependency to com.google.errorprone:javac with the appropriate version cor

val noJavacDependencyNotified = AtomicBoolean()
fun JavaCompile.configureErrorProneJavac() {
ensureForking()
options.isFork = true
javacConfiguration.asPath.also {
if (it.isNotBlank()) {
options.forkOptions.jvmArgs!!.add("-Xbootclasspath/p:$it")
Expand Down Expand Up @@ -179,20 +178,9 @@ Add a dependency to com.google.errorprone:javac with the appropriate version cor
}
}

private fun JavaCompile.ensureForking() {
if (!options.isFork) {
options.isFork = true
// See org.gradle.api.internal.tasks.compile.AbstractJavaCompileSpecFactory#isCurrentVmOurToolchain
// reset forkOptions in case they were configured, but only when not using a toolchain
if (!javaCompiler.isPresent) {
options.forkOptions = ForkOptions()
}
}
}

private fun JavaCompile.configureForJava16plus() {
// https://github.com/google/error-prone/issues/1157#issuecomment-769289564
ensureForking()
options.isFork = true
options.forkOptions.jvmArgs!!.addAll(JVM_ARGS_STRONG_ENCAPSULATION)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,8 @@ class Java8IntegrationTest : AbstractPluginIntegrationTest() {
assertThat(result.task(":compileJava")?.outcome).isEqualTo(TaskOutcome.SUCCESS)
assertThat(result.output).contains(FORKED)
assertThat(result.output).contains(JVM_ARGS_STRONG_ENCAPSULATION)
// Check that the configured jvm arg is removed
assertThat(result.output).doesNotContain(jvmArg("-XshowSettings"))
// Check that the configured jvm arg is preserved
assertThat(result.output).contains(jvmArg("-XshowSettings"))
}

// check that it doesn't mess with task avoidance
Expand Down Expand Up @@ -290,8 +290,8 @@ class Java8IntegrationTest : AbstractPluginIntegrationTest() {
assertThat(result.output).contains(ErrorPronePlugin.NO_JAVAC_DEPENDENCY_WARNING_MESSAGE)
assertThat(result.output).contains(FORKED)
assertThat(result.output).doesNotContain(JVM_ARG_BOOTCLASSPATH)
// Check that the configured jvm arg is removed
assertThat(result.output).doesNotContain(jvmArg("-XshowSettings"))
// Check that the configured jvm arg is preserved
assertThat(result.output).contains(jvmArg("-XshowSettings"))
}

// check that adding back the dependency fixes compilation (so it was indeed caused by missing dependency) and silences the warning
Expand All @@ -313,8 +313,8 @@ class Java8IntegrationTest : AbstractPluginIntegrationTest() {
assertThat(result.output).doesNotContain(ErrorPronePlugin.NO_JAVAC_DEPENDENCY_WARNING_MESSAGE)
assertThat(result.output).contains(FORKED)
assertThat(result.output).containsMatch(JVM_ARG_BOOTCLASSPATH_ERRORPRONE_JAVAC)
// Check that the configured jvm arg is removed
assertThat(result.output).doesNotContain(jvmArg("-XshowSettings"))
// Check that the configured jvm arg is preserved
assertThat(result.output).contains(jvmArg("-XshowSettings"))
}
}

Expand Down Expand Up @@ -363,8 +363,8 @@ class Java8IntegrationTest : AbstractPluginIntegrationTest() {
assertThat(result.task(":compileJava")?.outcome).isEqualTo(TaskOutcome.SUCCESS)
assertThat(result.output).contains(FORKED)
assertThat(result.output).containsMatch(JVM_ARG_BOOTCLASSPATH_ERRORPRONE_JAVAC)
// Check that the configured jvm arg is removed
assertThat(result.output).doesNotContain(jvmArg("-XshowSettings"))
// Check that the configured jvm arg is preserved
assertThat(result.output).contains(jvmArg("-XshowSettings"))
}

// Move the errorproneJavac dependency: first remove it, then add it back… differently
Expand Down

0 comments on commit 8958d29

Please sign in to comment.