From c032f7da20f429c31d94bf5fe19db7f61b8d2449 Mon Sep 17 00:00:00 2001 From: Thomas Broyer Date: Sun, 4 Jul 2021 02:18:13 +0200 Subject: [PATCH] Do not reset fork options when using toolchains Fixes #64 --- .../gradle/errorprone/ErrorPronePlugin.kt | 21 ++++++------ .../gradle/errorprone/Java8IntegrationTest.kt | 27 +++++++++++++--- .../errorprone/ToolchainsIntegrationTest.kt | 32 ++++++++++++++++--- 3 files changed, 61 insertions(+), 19 deletions(-) diff --git a/src/main/kotlin/net/ltgt/gradle/errorprone/ErrorPronePlugin.kt b/src/main/kotlin/net/ltgt/gradle/errorprone/ErrorPronePlugin.kt index 82d1422..7a8f47d 100644 --- a/src/main/kotlin/net/ltgt/gradle/errorprone/ErrorPronePlugin.kt +++ b/src/main/kotlin/net/ltgt/gradle/errorprone/ErrorPronePlugin.kt @@ -92,11 +92,7 @@ Add a dependency to com.google.errorprone:javac with the appropriate version cor val noJavacDependencyNotified = AtomicBoolean() fun JavaCompile.configureErrorProneJavac() { - if (!options.isFork) { - options.isFork = true - // reset forkOptions in case they were configured - options.forkOptions = ForkOptions() - } + ensureForking() javacConfiguration.asPath.also { if (it.isNotBlank()) { options.forkOptions.jvmArgs!!.add("-Xbootclasspath/p:$it") @@ -190,13 +186,20 @@ Add a dependency to com.google.errorprone:javac with the appropriate version cor } } - private fun JavaCompile.configureForJava16plus() { - // https://github.com/google/error-prone/issues/1157#issuecomment-769289564 + private fun JavaCompile.ensureForking() { if (!options.isFork) { options.isFork = true - // reset forkOptions in case they were configured - options.forkOptions = ForkOptions() + // See org.gradle.api.internal.tasks.compile.AbstractJavaCompileSpecFactory#isCurrentVmOurToolchain + // reset forkOptions in case they were configured, but only when not using a toolchain + if (!HAS_TOOLCHAINS || !javaCompiler.isPresent) { + options.forkOptions = ForkOptions() + } } + } + + private fun JavaCompile.configureForJava16plus() { + // https://github.com/google/error-prone/issues/1157#issuecomment-769289564 + ensureForking() options.forkOptions.jvmArgs!!.addAll(JVM_ARGS_STRONG_ENCAPSULATION) } } diff --git a/src/test/kotlin/net/ltgt/gradle/errorprone/Java8IntegrationTest.kt b/src/test/kotlin/net/ltgt/gradle/errorprone/Java8IntegrationTest.kt index 8aca1a5..ff4c344 100644 --- a/src/test/kotlin/net/ltgt/gradle/errorprone/Java8IntegrationTest.kt +++ b/src/test/kotlin/net/ltgt/gradle/errorprone/Java8IntegrationTest.kt @@ -52,6 +52,7 @@ class Java8IntegrationTest : AbstractPluginIntegrationTest() { } } compileJava.finalizedBy(displayCompileJavaOptions) + compileJava.options.forkOptions.jvmArgs!!.add("-XshowSettings") """.trimIndent() ) if (JavaVersion.current().isJava16Compatible && GradleVersion.version(testGradleVersion) < GradleVersion.version("7.0")) { @@ -78,7 +79,8 @@ class Java8IntegrationTest : AbstractPluginIntegrationTest() { // then assertThat(result.task(":compileJava")?.outcome).isEqualTo(TaskOutcome.SUCCESS) assertThat(result.output).contains(NOT_FORKED) - assertThat(result.output).doesNotContain(JVM_ARG) + // Check that the configured jvm arg is preserved + assertThat(result.output).contains(jvmArg("-XshowSettings")) } // Test a forked task @@ -96,7 +98,9 @@ class Java8IntegrationTest : AbstractPluginIntegrationTest() { // then assertThat(result.task(":compileJava")?.outcome).isEqualTo(TaskOutcome.SUCCESS) assertThat(result.output).contains(FORKED) - assertThat(result.output).doesNotContain(JVM_ARG) + assertThat(result.output).doesNotContain(JVM_ARG_BOOTCLASSPATH) + // Check that the configured jvm arg is preserved + assertThat(result.output).contains(jvmArg("-XshowSettings")) } } @@ -131,6 +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 it doesn't mess with task avoidance @@ -160,7 +166,9 @@ class Java8IntegrationTest : AbstractPluginIntegrationTest() { // then assertThat(result.task(":compileJava")?.outcome).isEqualTo(TaskOutcome.SUCCESS) assertThat(result.output).contains(NOT_FORKED) - assertThat(result.output).doesNotContain(JVM_ARG) + assertThat(result.output).doesNotContain(JVM_ARG_BOOTCLASSPATH) + // Check that the configured jvm arg is preserved + assertThat(result.output).contains(jvmArg("-XshowSettings")) } @Test @@ -173,7 +181,6 @@ class Java8IntegrationTest : AbstractPluginIntegrationTest() { compileJava.apply { options.isFork = true - options.forkOptions.jvmArgs!!.add("-XshowSettings") } """.trimIndent() ) @@ -219,7 +226,9 @@ class Java8IntegrationTest : AbstractPluginIntegrationTest() { // then assertThat(result.task(":compileJava")?.outcome).isNotNull() assertThat(result.output).contains(FORKED) - assertThat(result.output).doesNotContain(JVM_ARG) + assertThat(result.output).doesNotContain(JVM_ARG_BOOTCLASSPATH) + // Check that the configured jvm arg is preserved + assertThat(result.output).contains(jvmArg("-XshowSettings")) } @Test @@ -258,6 +267,8 @@ class Java8IntegrationTest : AbstractPluginIntegrationTest() { assertThat(result.task(":compileJava")?.outcome).isNotNull() assertThat(result.output).contains(FORKED) assertThat(result.output).doesNotContain(JVM_ARG_BOOTCLASSPATH) + // Check that the configured jvm arg is preserved + assertThat(result.output).contains(jvmArg("-XshowSettings")) } @Test @@ -279,6 +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 adding back the dependency fixes compilation (so it was indeed caused by missing dependency) and silences the warning @@ -300,6 +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")) } } @@ -354,6 +369,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")) } // Move the errorproneJavac dependency: first remove it, then add it back… differently diff --git a/src/test/kotlin/net/ltgt/gradle/errorprone/ToolchainsIntegrationTest.kt b/src/test/kotlin/net/ltgt/gradle/errorprone/ToolchainsIntegrationTest.kt index 1f2e493..c0335c2 100644 --- a/src/test/kotlin/net/ltgt/gradle/errorprone/ToolchainsIntegrationTest.kt +++ b/src/test/kotlin/net/ltgt/gradle/errorprone/ToolchainsIntegrationTest.kt @@ -68,6 +68,7 @@ class ToolchainsIntegrationTest : AbstractPluginIntegrationTest() { } compileJava { finalizedBy(displayCompileJavaOptions) + options.forkOptions.jvmArgs!!.add("-XshowSettings") } } """.trimIndent() @@ -164,7 +165,10 @@ class ToolchainsIntegrationTest : AbstractPluginIntegrationTest() { result.assumeToolchainAvailable() assertThat(result.task(":compileJava")?.outcome).isEqualTo(TaskOutcome.SUCCESS) assertThat(result.output).contains(NOT_FORKED) - assertThat(result.output).doesNotContain(JVM_ARG) + assertThat(result.output).doesNotContain(JVM_ARG_BOOTCLASSPATH) + assertThat(result.output).doesNotContain(JVM_ARGS_STRONG_ENCAPSULATION) + // Check that the configured jvm arg is preserved + assertThat(result.output).contains(jvmArg("-XshowSettings")) } // Test a forked task @@ -183,7 +187,10 @@ class ToolchainsIntegrationTest : AbstractPluginIntegrationTest() { result.assumeToolchainAvailable() assertThat(result.task(":compileJava")?.outcome).isEqualTo(TaskOutcome.SUCCESS) assertThat(result.output).contains(FORKED) - assertThat(result.output).doesNotContain(JVM_ARG) + assertThat(result.output).doesNotContain(JVM_ARG_BOOTCLASSPATH) + assertThat(result.output).doesNotContain(JVM_ARGS_STRONG_ENCAPSULATION) + // Check that the configured jvm arg is preserved + assertThat(result.output).contains(jvmArg("-XshowSettings")) } } @@ -208,6 +215,8 @@ class ToolchainsIntegrationTest : 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 preserved + assertThat(result.output).contains(jvmArg("-XshowSettings")) } // check that it doesn't mess with task avoidance @@ -241,6 +250,8 @@ class ToolchainsIntegrationTest : 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 preserved + assertThat(result.output).contains(jvmArg("-XshowSettings")) } // check that it doesn't mess with task avoidance @@ -276,7 +287,10 @@ class ToolchainsIntegrationTest : AbstractPluginIntegrationTest() { result.assumeToolchainAvailable() assertThat(result.task(":compileJava")?.outcome).isEqualTo(TaskOutcome.SUCCESS) assertThat(result.output).contains(NOT_FORKED) - assertThat(result.output).doesNotContain(JVM_ARG) + assertThat(result.output).doesNotContain(JVM_ARG_BOOTCLASSPATH) + assertThat(result.output).doesNotContain(JVM_ARGS_STRONG_ENCAPSULATION) + // Check that the configured jvm arg is preserved + assertThat(result.output).contains(jvmArg("-XshowSettings")) } @Test @@ -313,7 +327,10 @@ class ToolchainsIntegrationTest : AbstractPluginIntegrationTest() { result.assumeToolchainAvailable() assertThat(result.task(":compileJava")?.outcome).isEqualTo(TaskOutcome.SUCCESS) assertThat(result.output).contains(NOT_FORKED) - assertThat(result.output).doesNotContain(JVM_ARG) + assertThat(result.output).doesNotContain(JVM_ARG_BOOTCLASSPATH) + assertThat(result.output).doesNotContain(JVM_ARGS_STRONG_ENCAPSULATION) + // Check that the configured jvm arg is preserved + assertThat(result.output).contains(jvmArg("-XshowSettings")) } @Test @@ -330,7 +347,6 @@ class ToolchainsIntegrationTest : AbstractPluginIntegrationTest() { tasks.compileJava { options.isFork = true - options.forkOptions.jvmArgs!!.add("-XshowSettings") } """.trimIndent() ) @@ -375,6 +391,8 @@ class ToolchainsIntegrationTest : 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 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 @@ -397,6 +415,8 @@ class ToolchainsIntegrationTest : 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 preserved + assertThat(result.output).contains(jvmArg("-XshowSettings")) } } @@ -427,6 +447,8 @@ class ToolchainsIntegrationTest : AbstractPluginIntegrationTest() { assertThat(result.task(":compileJava")?.outcome).isEqualTo(TaskOutcome.SUCCESS) assertThat(result.output).doesNotContain(ErrorPronePlugin.NO_JAVAC_DEPENDENCY_WARNING_MESSAGE) assertThat(result.output).doesNotContain(JVM_ARG_BOOTCLASSPATH) + // Check that the configured jvm arg is preserved + assertThat(result.output).contains(jvmArg("-XshowSettings")) } } }