diff --git a/build.gradle.kts b/build.gradle.kts index f035a62d..8872bf77 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -15,11 +15,13 @@ plugins { configure { disallowMultipleDaemons.set(false) - ensureJavaHomeMatches.set(!providers.environmentVariable("CI").forUseAtConfigurationTime().isPresent) GCWarningThreshold.set(0.01f) enableTestCaching.set(false) downloadSpeedWarningThreshold.set(2.0f) daggerThreshold.set(100) + javaHome { + ensureJavaHomeMatches.set(!providers.environmentVariable("CI").forUseAtConfigurationTime().isPresent) + } } tasks.withType(Test::class.java).configureEach { diff --git a/docs/configuration.md b/docs/configuration.md index 09278052..be6d5e41 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -9,14 +9,6 @@ * Throw an exception when multiple Gradle Daemons are running. */ disallowMultipleDaemons = false - /** - * Ensure that we are using JAVA_HOME to build with this Gradle. - */ - ensureJavaHomeMatches = true - /** - * Ensure we have JAVA_HOME set. - */ - ensureJavaHomeIsSet = true /** * Show a message if the download speed is less than this many megabytes / sec. */ @@ -42,6 +34,27 @@ * Do not allow building all apps simultaneously. This is likely not what the user intended. */ allowBuildingAllAndroidAppsSimultaneously = false + + /** Configuration properties relating to JAVA_HOME */ + javaHome { + /** + * Ensure that we are using JAVA_HOME to build with this Gradle. + */ + ensureJavaHomeMatches = true + /** + * Ensure we have JAVA_HOME set. + */ + ensureJavaHomeIsSet = true + /** + * Fail on any `JAVA_HOME` issues. + */ + failOnError.set(true) + /** + * Extra message text, if any, to show with the Gradle Doctor message. This is useful if you have a wiki page or + * other instructions that you want to link for developers on your team if they encounter an issue. + */ + extraMessage.set("Here's an extra message to show.") + } } ``` === "Kotlin" @@ -51,14 +64,6 @@ * Throw an exception when multiple Gradle Daemons are running. */ disallowMultipleDaemons.set(false) - /** - * Ensure that we are using JAVA_HOME to build with this Gradle. - */ - ensureJavaHomeMatches.set(true) - /** - * Ensure we have JAVA_HOME set. - */ - ensureJavaHomeIsSet.set(true) /** * Show a message if the download speed is less than this many megabytes / sec. */ @@ -84,6 +89,27 @@ * Do not allow building all apps simultaneously. This is likely not what the user intended. */ allowBuildingAllAndroidAppsSimultaneously.set(false) + + /** Configuration properties relating to JAVA_HOME */ + javaHome { + /** + * Ensure that we are using JAVA_HOME to build with this Gradle. + */ + ensureJavaHomeMatches.set(true) + /** + * Ensure we have JAVA_HOME set. + */ + ensureJavaHomeIsSet.set(true) + /** + * Fail on any `JAVA_HOME` issues. + */ + failOnError.set(true) + /** + * Extra message text, if any, to show with the Gradle Doctor message. This is useful if you have a wiki page or + * other instructions that you want to link for developers on your team if they encounter an issue. + */ + extraMessage.set("Here's an extra message to show.") + } } ``` diff --git a/doctor-plugin/src/main/java/com/osacky/doctor/DoctorExtension.kt b/doctor-plugin/src/main/java/com/osacky/doctor/DoctorExtension.kt index 8c195f41..5a8f3039 100644 --- a/doctor-plugin/src/main/java/com/osacky/doctor/DoctorExtension.kt +++ b/doctor-plugin/src/main/java/com/osacky/doctor/DoctorExtension.kt @@ -1,21 +1,19 @@ package com.osacky.doctor +import org.gradle.api.Action import org.gradle.api.model.ObjectFactory +import org.gradle.kotlin.dsl.newInstance import org.gradle.kotlin.dsl.property +import javax.inject.Inject open class DoctorExtension(objects: ObjectFactory) { + + internal val javaHomeHandler = objects.newInstance() + /** * Throw an exception when multiple Gradle Daemons are running. */ val disallowMultipleDaemons = objects.property().convention(false) - /** - * Ensure that we are using JAVA_HOME to build with this Gradle. - */ - val ensureJavaHomeMatches = objects.property().convention(true) - /** - * Ensure we have JAVA_HOME set. - */ - val ensureJavaHomeIsSet = objects.property().convention(true) /** * Show a message if the download speed is less than this many megabytes / sec. */ @@ -42,4 +40,34 @@ open class DoctorExtension(objects: ObjectFactory) { * Do not allow building all apps simultaneously. This is likely not what the user intended. */ val allowBuildingAllAndroidAppsSimultaneously = objects.property().convention(false) + + /** + * Configures `JAVA_HOME`-specific behavior. + */ + fun javaHome(action: Action) { + action.execute(javaHomeHandler) + } +} + +abstract class JavaHomeHandler @Inject constructor(objects: ObjectFactory) { + /** + * Ensure that we are using `JAVA_HOME` to build with this Gradle. + */ + val ensureJavaHomeMatches = objects.property().convention(true) + /** + * Ensure we have `JAVA_HOME` set. + */ + val ensureJavaHomeIsSet = objects.property().convention(true) + + /** + * Fail on any `JAVA_HOME` issues. + */ + val failOnError = objects.property().convention(true) + + /** + * Extra message text, if any, to show with the Gradle Doctor message. This is useful if you have a wiki page or + * other instructions that you want to link for developers on your team if they encounter an issue. + */ + @Suppress("CAST_NEVER_SUCCEEDS") // Cast is for overload ambiguity + val extraMessage = objects.property().convention(null as? String) } diff --git a/doctor-plugin/src/main/java/com/osacky/doctor/DoctorPlugin.kt b/doctor-plugin/src/main/java/com/osacky/doctor/DoctorPlugin.kt index 68851bce..d55c0b94 100644 --- a/doctor-plugin/src/main/java/com/osacky/doctor/DoctorPlugin.kt +++ b/doctor-plugin/src/main/java/com/osacky/doctor/DoctorPlugin.kt @@ -32,7 +32,7 @@ class DoctorPlugin : Plugin { val intervalMeasurer = IntervalMeasurer() val pillBoxPrinter = PillBoxPrinter(target.logger) val daemonChecker = BuildDaemonChecker(extension, DaemonCheck(), pillBoxPrinter) - val javaHomeCheck = JavaHomeCheck(extension, pillBoxPrinter) + val javaHomeCheck = JavaHomeCheck(extension, pillBoxPrinter, target.logger) val garbagePrinter = GarbagePrinter(clock, DirtyBeanCollector(), extension) val operations = BuildOperations(target.gradle) val javaAnnotationTime = JavaAnnotationTime(operations, extension, target.buildscript.configurations) diff --git a/doctor-plugin/src/main/java/com/osacky/doctor/JavaHomeCheck.kt b/doctor-plugin/src/main/java/com/osacky/doctor/JavaHomeCheck.kt index 3c4e1c79..dc8d4828 100644 --- a/doctor-plugin/src/main/java/com/osacky/doctor/JavaHomeCheck.kt +++ b/doctor-plugin/src/main/java/com/osacky/doctor/JavaHomeCheck.kt @@ -3,44 +3,68 @@ package com.osacky.doctor import com.osacky.doctor.internal.Finish import com.osacky.doctor.internal.PillBoxPrinter import org.gradle.api.GradleException +import org.gradle.api.logging.Logger import org.gradle.internal.jvm.Jvm import java.io.File +import java.util.Collections class JavaHomeCheck( private val extension: DoctorExtension, - private val pillBoxPrinter: PillBoxPrinter + private val pillBoxPrinter: PillBoxPrinter, + private val logger: Logger ) : BuildStartFinishListener { + + private val recordedErrors = Collections.synchronizedSet(LinkedHashSet()) + override fun onStart() { - if (extension.ensureJavaHomeIsSet.get() && environmentJavaHome == null) { - throw GradleException( - pillBoxPrinter.createPill( - """ - JAVA_HOME is not set. - Please set JAVA_HOME so that switching between Android Studio and the terminal does not trigger a full rebuild. - To set JAVA_HOME: (using bash) - echo "export JAVA_HOME=${'$'}(/usr/libexec/java_home)" >> ~/.bash_profile - or `~/.zshrc` if using zsh. - """.trimIndent() - ) - ) + val extraMessage = extension.javaHomeHandler.extraMessage.orNull + val failOnError = extension.javaHomeHandler.failOnError.get() + + if (extension.javaHomeHandler.ensureJavaHomeIsSet.get() && environmentJavaHome == null) { + val message = buildString { + appendln("JAVA_HOME is not set.") + appendln("Please set JAVA_HOME so that switching between Android Studio and the terminal does not trigger a full rebuild.") + appendln("To set JAVA_HOME: (using bash)") + appendln("echo \"export JAVA_HOME=${'$'}(/usr/libexec/java_home)\" >> ~/.bash_profile") + appendln("or `~/.zshrc` if using zsh.") + extraMessage?.let { + appendln() + appendln(extraMessage) + } + } + val pill = pillBoxPrinter.createPill(message) + if (failOnError) { + throw GradleException(pill) + } else { + recordedErrors.add(pill) + } } - if (extension.ensureJavaHomeMatches.get() && !isGradleUsingJavaHome()) { - throw GradleException( - pillBoxPrinter.createPill( - """ - |Gradle is not using JAVA_HOME. - |JAVA_HOME is ${environmentJavaHome.toFile().toPath().toAbsolutePath()} - |Gradle is using ${gradleJavaHome.toPath().toAbsolutePath()} - |This can slow down your build significantly when switching from Android Studio to the terminal. - |To fix: Project Structure -> JDK Location. - |Set this to your JAVA_HOME. - """.trimMargin() - ) - ) + if (extension.javaHomeHandler.ensureJavaHomeMatches.get() && !isGradleUsingJavaHome()) { + val message = buildString { + appendln("Gradle is not using JAVA_HOME.") + appendln("JAVA_HOME is ${environmentJavaHome.toFile().toPath().toAbsolutePath()}") + appendln("Gradle is using ${gradleJavaHome.toPath().toAbsolutePath()}") + appendln("This can slow down your build significantly when switching from Android Studio to the terminal.") + appendln("To fix: Project Structure -> JDK Location.") + appendln("Set this to your JAVA_HOME.") + extraMessage?.let { + appendln() + appendln(extraMessage) + } + } + val pill = pillBoxPrinter.createPill(message) + if (failOnError) { + throw GradleException(pill) + } else { + recordedErrors.add(pill) + } } } override fun onFinish(): Finish { + if (recordedErrors.isNotEmpty()) { + logger.error(recordedErrors.joinToString("\n\n")) + } return Finish.None } diff --git a/doctor-plugin/src/test/java/com/osacky/doctor/ConfigurationCacheTest.kt b/doctor-plugin/src/test/java/com/osacky/doctor/ConfigurationCacheTest.kt index d398b2f4..f6bc7ee0 100644 --- a/doctor-plugin/src/test/java/com/osacky/doctor/ConfigurationCacheTest.kt +++ b/doctor-plugin/src/test/java/com/osacky/doctor/ConfigurationCacheTest.kt @@ -18,8 +18,10 @@ class ConfigurationCacheTest { | id "com.osacky.doctor" |} |doctor { - | disallowMultipleDaemons = false - | ensureJavaHomeMatches = false + | javaHome { + | disallowMultipleDaemons = false + | ensureJavaHomeMatches = false + | } |} """.trimMargin("|") ) diff --git a/doctor-plugin/src/test/java/com/osacky/doctor/PluginIntegrationTest.kt b/doctor-plugin/src/test/java/com/osacky/doctor/PluginIntegrationTest.kt index e56d5d26..98659759 100644 --- a/doctor-plugin/src/test/java/com/osacky/doctor/PluginIntegrationTest.kt +++ b/doctor-plugin/src/test/java/com/osacky/doctor/PluginIntegrationTest.kt @@ -38,7 +38,9 @@ class PluginIntegrationTest constructor(private val version: String) { |} |doctor { | disallowMultipleDaemons = false - | ensureJavaHomeMatches = false + | javaHome { + | ensureJavaHomeMatches = false + | } |} """.trimMargin("|") ) @@ -58,7 +60,9 @@ class PluginIntegrationTest constructor(private val version: String) { |} |doctor { | disallowMultipleDaemons = false - | ensureJavaHomeMatches = !System.getenv().containsKey("CI") + | javaHome { + | ensureJavaHomeMatches = !System.getenv().containsKey("CI") + | } |} """.trimMargin("|") ) @@ -77,7 +81,9 @@ class PluginIntegrationTest constructor(private val version: String) { |} |doctor { | disallowMultipleDaemons = true - | ensureJavaHomeMatches = !System.getenv().containsKey("CI") + | javaHome { + | ensureJavaHomeMatches = !System.getenv().containsKey("CI") + | } |} """.trimMargin("|") ) @@ -113,8 +119,12 @@ class PluginIntegrationTest constructor(private val version: String) { | id "com.osacky.doctor" |} |doctor { - | disallowMultipleDaemons = false - | ensureJavaHomeMatches = true + | javaHome { + | disallowMultipleDaemons = false + | javaHome { + | ensureJavaHomeMatches = true + | } + | } |} """.trimMargin("|") ) @@ -134,6 +144,70 @@ class PluginIntegrationTest constructor(private val version: String) { ) } + // This is failing, perhaps because it is actually trying to use "foo" as JAVA_HOME. + @Test @Ignore + fun testJavaHomeNotSetWithConsoleError() { + assumeSupportedVersion() + + writeBuildGradle( + """ + |plugins { + | id "com.osacky.doctor" + |} + |doctor { + | javaHome { + | disallowMultipleDaemons = false + | ensureJavaHomeMatches = true + | failOnError = false + | } + |} + """.trimMargin("|") + ) + testProjectRoot.newFile("settings.gradle") + + val result = createRunner() + .withEnvironment(mapOf("JAVA_HOME" to "foo")) + .withArguments("tasks") + .buildAndFail() + // Still prints the error + assertThat(result.output).contains( + """ + |> =============================== Gradle Doctor Prescriptions ============================================ + | | Gradle is not using JAVA_HOME. | + | | JAVA_HOME is foo | + | """ + .trimMargin("|") + ) + } + + // This is failing, perhaps because it is actually trying to use "foo" as JAVA_HOME. + @Test @Ignore + fun testJavaHomeNotSetWithCustomMessage() { + assumeSupportedVersion() + + writeBuildGradle( + """ + |plugins { + | id "com.osacky.doctor" + |} + |doctor { + | javaHome { + | disallowMultipleDaemons = false + | ensureJavaHomeMatches = true + | extraMessage = "Check for more details here!" + | } + |} + """.trimMargin("|") + ) + testProjectRoot.newFile("settings.gradle") + + val result = createRunner() + .withEnvironment(mapOf("JAVA_HOME" to "foo")) + .withArguments("tasks") + .buildAndFail() + assertThat(result.output).contains("Check for more details here!") + } + @Test fun testFailAssembleMultipleProjects() { assumeSupportedVersion() @@ -155,7 +229,9 @@ class PluginIntegrationTest constructor(private val version: String) { } doctor { disallowMultipleDaemons = false - ensureJavaHomeMatches = false + javaHome { + ensureJavaHomeMatches = false + } } """.trimIndent() ) @@ -232,7 +308,9 @@ class PluginIntegrationTest constructor(private val version: String) { } doctor { disallowMultipleDaemons = false - ensureJavaHomeMatches = false + javaHome { + ensureJavaHomeMatches = false + } } """.trimIndent() ) @@ -298,7 +376,9 @@ class PluginIntegrationTest constructor(private val version: String) { |} |doctor { | disallowMultipleDaemons = false - | ensureJavaHomeMatches = false + | javaHome { + | ensureJavaHomeMatches = false + | } | failOnEmptyDirectories = true |} """.trimMargin("|") @@ -325,7 +405,9 @@ class PluginIntegrationTest constructor(private val version: String) { |} |doctor { | disallowMultipleDaemons = false - | ensureJavaHomeMatches = false + | javaHome { + | ensureJavaHomeMatches = false + | } | failOnEmptyDirectories = false |} """.trimMargin("|") diff --git a/doctor-plugin/src/test/java/com/osacky/doctor/TestDaggerTime.kt b/doctor-plugin/src/test/java/com/osacky/doctor/TestDaggerTime.kt index 1d946937..bff7b2d5 100644 --- a/doctor-plugin/src/test/java/com/osacky/doctor/TestDaggerTime.kt +++ b/doctor-plugin/src/test/java/com/osacky/doctor/TestDaggerTime.kt @@ -20,8 +20,10 @@ class TestDaggerTime { } doctor { disallowMultipleDaemons = false - daggerThreshold = 100 - ensureJavaHomeMatches = !System.getenv().containsKey("CI") + javaHome { + daggerThreshold = 100 + ensureJavaHomeMatches = !System.getenv().containsKey("CI") + } } """.trimIndent() ) @@ -75,8 +77,10 @@ class TestDaggerTime { apply plugin: 'com.soundcloud.delect' doctor { disallowMultipleDaemons = false - daggerThreshold = 100 - ensureJavaHomeMatches = false + javaHome { + daggerThreshold = 100 + ensureJavaHomeMatches = false + } } """.trimIndent() )