Skip to content

Commit c032f7d

Browse files
committed
Do not reset fork options when using toolchains
Fixes #64
1 parent 966dfb3 commit c032f7d

File tree

3 files changed

+61
-19
lines changed

3 files changed

+61
-19
lines changed

src/main/kotlin/net/ltgt/gradle/errorprone/ErrorPronePlugin.kt

+12-9
Original file line numberDiff line numberDiff line change
@@ -92,11 +92,7 @@ Add a dependency to com.google.errorprone:javac with the appropriate version cor
9292

9393
val noJavacDependencyNotified = AtomicBoolean()
9494
fun JavaCompile.configureErrorProneJavac() {
95-
if (!options.isFork) {
96-
options.isFork = true
97-
// reset forkOptions in case they were configured
98-
options.forkOptions = ForkOptions()
99-
}
95+
ensureForking()
10096
javacConfiguration.asPath.also {
10197
if (it.isNotBlank()) {
10298
options.forkOptions.jvmArgs!!.add("-Xbootclasspath/p:$it")
@@ -190,13 +186,20 @@ Add a dependency to com.google.errorprone:javac with the appropriate version cor
190186
}
191187
}
192188

193-
private fun JavaCompile.configureForJava16plus() {
194-
// https://github.com/google/error-prone/issues/1157#issuecomment-769289564
189+
private fun JavaCompile.ensureForking() {
195190
if (!options.isFork) {
196191
options.isFork = true
197-
// reset forkOptions in case they were configured
198-
options.forkOptions = ForkOptions()
192+
// See org.gradle.api.internal.tasks.compile.AbstractJavaCompileSpecFactory#isCurrentVmOurToolchain
193+
// reset forkOptions in case they were configured, but only when not using a toolchain
194+
if (!HAS_TOOLCHAINS || !javaCompiler.isPresent) {
195+
options.forkOptions = ForkOptions()
196+
}
199197
}
198+
}
199+
200+
private fun JavaCompile.configureForJava16plus() {
201+
// https://github.com/google/error-prone/issues/1157#issuecomment-769289564
202+
ensureForking()
200203
options.forkOptions.jvmArgs!!.addAll(JVM_ARGS_STRONG_ENCAPSULATION)
201204
}
202205
}

src/test/kotlin/net/ltgt/gradle/errorprone/Java8IntegrationTest.kt

+22-5
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ class Java8IntegrationTest : AbstractPluginIntegrationTest() {
5252
}
5353
}
5454
compileJava.finalizedBy(displayCompileJavaOptions)
55+
compileJava.options.forkOptions.jvmArgs!!.add("-XshowSettings")
5556
""".trimIndent()
5657
)
5758
if (JavaVersion.current().isJava16Compatible && GradleVersion.version(testGradleVersion) < GradleVersion.version("7.0")) {
@@ -78,7 +79,8 @@ class Java8IntegrationTest : AbstractPluginIntegrationTest() {
7879
// then
7980
assertThat(result.task(":compileJava")?.outcome).isEqualTo(TaskOutcome.SUCCESS)
8081
assertThat(result.output).contains(NOT_FORKED)
81-
assertThat(result.output).doesNotContain(JVM_ARG)
82+
// Check that the configured jvm arg is preserved
83+
assertThat(result.output).contains(jvmArg("-XshowSettings"))
8284
}
8385

8486
// Test a forked task
@@ -96,7 +98,9 @@ class Java8IntegrationTest : AbstractPluginIntegrationTest() {
9698
// then
9799
assertThat(result.task(":compileJava")?.outcome).isEqualTo(TaskOutcome.SUCCESS)
98100
assertThat(result.output).contains(FORKED)
99-
assertThat(result.output).doesNotContain(JVM_ARG)
101+
assertThat(result.output).doesNotContain(JVM_ARG_BOOTCLASSPATH)
102+
// Check that the configured jvm arg is preserved
103+
assertThat(result.output).contains(jvmArg("-XshowSettings"))
100104
}
101105
}
102106

@@ -131,6 +135,8 @@ class Java8IntegrationTest : AbstractPluginIntegrationTest() {
131135
assertThat(result.task(":compileJava")?.outcome).isEqualTo(TaskOutcome.SUCCESS)
132136
assertThat(result.output).contains(FORKED)
133137
assertThat(result.output).contains(JVM_ARGS_STRONG_ENCAPSULATION)
138+
// Check that the configured jvm arg is removed
139+
assertThat(result.output).doesNotContain(jvmArg("-XshowSettings"))
134140
}
135141

136142
// check that it doesn't mess with task avoidance
@@ -160,7 +166,9 @@ class Java8IntegrationTest : AbstractPluginIntegrationTest() {
160166
// then
161167
assertThat(result.task(":compileJava")?.outcome).isEqualTo(TaskOutcome.SUCCESS)
162168
assertThat(result.output).contains(NOT_FORKED)
163-
assertThat(result.output).doesNotContain(JVM_ARG)
169+
assertThat(result.output).doesNotContain(JVM_ARG_BOOTCLASSPATH)
170+
// Check that the configured jvm arg is preserved
171+
assertThat(result.output).contains(jvmArg("-XshowSettings"))
164172
}
165173

166174
@Test
@@ -173,7 +181,6 @@ class Java8IntegrationTest : AbstractPluginIntegrationTest() {
173181
174182
compileJava.apply {
175183
options.isFork = true
176-
options.forkOptions.jvmArgs!!.add("-XshowSettings")
177184
}
178185
""".trimIndent()
179186
)
@@ -219,7 +226,9 @@ class Java8IntegrationTest : AbstractPluginIntegrationTest() {
219226
// then
220227
assertThat(result.task(":compileJava")?.outcome).isNotNull()
221228
assertThat(result.output).contains(FORKED)
222-
assertThat(result.output).doesNotContain(JVM_ARG)
229+
assertThat(result.output).doesNotContain(JVM_ARG_BOOTCLASSPATH)
230+
// Check that the configured jvm arg is preserved
231+
assertThat(result.output).contains(jvmArg("-XshowSettings"))
223232
}
224233

225234
@Test
@@ -258,6 +267,8 @@ class Java8IntegrationTest : AbstractPluginIntegrationTest() {
258267
assertThat(result.task(":compileJava")?.outcome).isNotNull()
259268
assertThat(result.output).contains(FORKED)
260269
assertThat(result.output).doesNotContain(JVM_ARG_BOOTCLASSPATH)
270+
// Check that the configured jvm arg is preserved
271+
assertThat(result.output).contains(jvmArg("-XshowSettings"))
261272
}
262273

263274
@Test
@@ -279,6 +290,8 @@ class Java8IntegrationTest : AbstractPluginIntegrationTest() {
279290
assertThat(result.output).contains(ErrorPronePlugin.NO_JAVAC_DEPENDENCY_WARNING_MESSAGE)
280291
assertThat(result.output).contains(FORKED)
281292
assertThat(result.output).doesNotContain(JVM_ARG_BOOTCLASSPATH)
293+
// Check that the configured jvm arg is removed
294+
assertThat(result.output).doesNotContain(jvmArg("-XshowSettings"))
282295
}
283296

284297
// 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() {
300313
assertThat(result.output).doesNotContain(ErrorPronePlugin.NO_JAVAC_DEPENDENCY_WARNING_MESSAGE)
301314
assertThat(result.output).contains(FORKED)
302315
assertThat(result.output).containsMatch(JVM_ARG_BOOTCLASSPATH_ERRORPRONE_JAVAC)
316+
// Check that the configured jvm arg is removed
317+
assertThat(result.output).doesNotContain(jvmArg("-XshowSettings"))
303318
}
304319
}
305320

@@ -354,6 +369,8 @@ class Java8IntegrationTest : AbstractPluginIntegrationTest() {
354369
assertThat(result.task(":compileJava")?.outcome).isEqualTo(TaskOutcome.SUCCESS)
355370
assertThat(result.output).contains(FORKED)
356371
assertThat(result.output).containsMatch(JVM_ARG_BOOTCLASSPATH_ERRORPRONE_JAVAC)
372+
// Check that the configured jvm arg is removed
373+
assertThat(result.output).doesNotContain(jvmArg("-XshowSettings"))
357374
}
358375

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

src/test/kotlin/net/ltgt/gradle/errorprone/ToolchainsIntegrationTest.kt

+27-5
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ class ToolchainsIntegrationTest : AbstractPluginIntegrationTest() {
6868
}
6969
compileJava {
7070
finalizedBy(displayCompileJavaOptions)
71+
options.forkOptions.jvmArgs!!.add("-XshowSettings")
7172
}
7273
}
7374
""".trimIndent()
@@ -164,7 +165,10 @@ class ToolchainsIntegrationTest : AbstractPluginIntegrationTest() {
164165
result.assumeToolchainAvailable()
165166
assertThat(result.task(":compileJava")?.outcome).isEqualTo(TaskOutcome.SUCCESS)
166167
assertThat(result.output).contains(NOT_FORKED)
167-
assertThat(result.output).doesNotContain(JVM_ARG)
168+
assertThat(result.output).doesNotContain(JVM_ARG_BOOTCLASSPATH)
169+
assertThat(result.output).doesNotContain(JVM_ARGS_STRONG_ENCAPSULATION)
170+
// Check that the configured jvm arg is preserved
171+
assertThat(result.output).contains(jvmArg("-XshowSettings"))
168172
}
169173

170174
// Test a forked task
@@ -183,7 +187,10 @@ class ToolchainsIntegrationTest : AbstractPluginIntegrationTest() {
183187
result.assumeToolchainAvailable()
184188
assertThat(result.task(":compileJava")?.outcome).isEqualTo(TaskOutcome.SUCCESS)
185189
assertThat(result.output).contains(FORKED)
186-
assertThat(result.output).doesNotContain(JVM_ARG)
190+
assertThat(result.output).doesNotContain(JVM_ARG_BOOTCLASSPATH)
191+
assertThat(result.output).doesNotContain(JVM_ARGS_STRONG_ENCAPSULATION)
192+
// Check that the configured jvm arg is preserved
193+
assertThat(result.output).contains(jvmArg("-XshowSettings"))
187194
}
188195
}
189196

@@ -208,6 +215,8 @@ class ToolchainsIntegrationTest : AbstractPluginIntegrationTest() {
208215
assertThat(result.task(":compileJava")?.outcome).isEqualTo(TaskOutcome.SUCCESS)
209216
assertThat(result.output).contains(FORKED)
210217
assertThat(result.output).containsMatch(JVM_ARG_BOOTCLASSPATH_ERRORPRONE_JAVAC)
218+
// Check that the configured jvm arg is preserved
219+
assertThat(result.output).contains(jvmArg("-XshowSettings"))
211220
}
212221

213222
// check that it doesn't mess with task avoidance
@@ -241,6 +250,8 @@ class ToolchainsIntegrationTest : AbstractPluginIntegrationTest() {
241250
assertThat(result.task(":compileJava")?.outcome).isEqualTo(TaskOutcome.SUCCESS)
242251
assertThat(result.output).contains(FORKED)
243252
assertThat(result.output).contains(JVM_ARGS_STRONG_ENCAPSULATION)
253+
// Check that the configured jvm arg is preserved
254+
assertThat(result.output).contains(jvmArg("-XshowSettings"))
244255
}
245256

246257
// check that it doesn't mess with task avoidance
@@ -276,7 +287,10 @@ class ToolchainsIntegrationTest : AbstractPluginIntegrationTest() {
276287
result.assumeToolchainAvailable()
277288
assertThat(result.task(":compileJava")?.outcome).isEqualTo(TaskOutcome.SUCCESS)
278289
assertThat(result.output).contains(NOT_FORKED)
279-
assertThat(result.output).doesNotContain(JVM_ARG)
290+
assertThat(result.output).doesNotContain(JVM_ARG_BOOTCLASSPATH)
291+
assertThat(result.output).doesNotContain(JVM_ARGS_STRONG_ENCAPSULATION)
292+
// Check that the configured jvm arg is preserved
293+
assertThat(result.output).contains(jvmArg("-XshowSettings"))
280294
}
281295

282296
@Test
@@ -313,7 +327,10 @@ class ToolchainsIntegrationTest : AbstractPluginIntegrationTest() {
313327
result.assumeToolchainAvailable()
314328
assertThat(result.task(":compileJava")?.outcome).isEqualTo(TaskOutcome.SUCCESS)
315329
assertThat(result.output).contains(NOT_FORKED)
316-
assertThat(result.output).doesNotContain(JVM_ARG)
330+
assertThat(result.output).doesNotContain(JVM_ARG_BOOTCLASSPATH)
331+
assertThat(result.output).doesNotContain(JVM_ARGS_STRONG_ENCAPSULATION)
332+
// Check that the configured jvm arg is preserved
333+
assertThat(result.output).contains(jvmArg("-XshowSettings"))
317334
}
318335

319336
@Test
@@ -330,7 +347,6 @@ class ToolchainsIntegrationTest : AbstractPluginIntegrationTest() {
330347
331348
tasks.compileJava {
332349
options.isFork = true
333-
options.forkOptions.jvmArgs!!.add("-XshowSettings")
334350
}
335351
""".trimIndent()
336352
)
@@ -375,6 +391,8 @@ class ToolchainsIntegrationTest : AbstractPluginIntegrationTest() {
375391
assertThat(result.output).contains(ErrorPronePlugin.NO_JAVAC_DEPENDENCY_WARNING_MESSAGE)
376392
assertThat(result.output).contains(FORKED)
377393
assertThat(result.output).doesNotContain(JVM_ARG_BOOTCLASSPATH)
394+
// Check that the configured jvm arg is preserved
395+
assertThat(result.output).contains(jvmArg("-XshowSettings"))
378396
}
379397

380398
// 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() {
397415
assertThat(result.output).doesNotContain(ErrorPronePlugin.NO_JAVAC_DEPENDENCY_WARNING_MESSAGE)
398416
assertThat(result.output).contains(FORKED)
399417
assertThat(result.output).containsMatch(JVM_ARG_BOOTCLASSPATH_ERRORPRONE_JAVAC)
418+
// Check that the configured jvm arg is preserved
419+
assertThat(result.output).contains(jvmArg("-XshowSettings"))
400420
}
401421
}
402422

@@ -427,6 +447,8 @@ class ToolchainsIntegrationTest : AbstractPluginIntegrationTest() {
427447
assertThat(result.task(":compileJava")?.outcome).isEqualTo(TaskOutcome.SUCCESS)
428448
assertThat(result.output).doesNotContain(ErrorPronePlugin.NO_JAVAC_DEPENDENCY_WARNING_MESSAGE)
429449
assertThat(result.output).doesNotContain(JVM_ARG_BOOTCLASSPATH)
450+
// Check that the configured jvm arg is preserved
451+
assertThat(result.output).contains(jvmArg("-XshowSettings"))
430452
}
431453
}
432454
}

0 commit comments

Comments
 (0)