[Discussion] Make plugin project isolation compatible

.gitignore

@@ -4,6 +4,7 @@
 /.idea
 .DS_Store
 build/
+*/.kotlin/*
 /captures
 .externalNativeBuild
 

build.gradle.kts

@@ -1,4 +1,3 @@
-import com.osacky.doctor.DoctorExtension
 import org.gradle.api.tasks.testing.logging.TestLogEvent
 
 // Upgrade transitive dependencies in plugin classpath
@@ -18,19 +17,6 @@ buildscript {
 plugins {
   alias(libs.plugins.kgp)
   alias(libs.plugins.versions)
-  id("com.osacky.doctor")
-}
-
-configure<DoctorExtension> {
-  disallowMultipleDaemons.set(false)
-  GCWarningThreshold.set(0.01f)
-  enableTestCaching.set(false)
-  downloadSpeedWarningThreshold.set(2.0f)
-  daggerThreshold.set(100)
-  javaHome {
-      ensureJavaHomeMatches.set(true)
-      ensureJavaHomeIsSet.set(true)
-  }
 }
 
 tasks.withType(Test::class.java).configureEach {

doctor-plugin/build.gradle.kts

@@ -46,7 +46,7 @@ gradlePlugin {
             displayName = "Doctor Plugin"
             description = "The right prescription for your gradle build."
             tags.addAll(listOf("doctor", "android"))
-            implementationClass = "com.osacky.doctor.DoctorPlugin"
+            implementationClass = "com.osacky.doctor.DoctorSettingsPlugin"
         }
     }
 }

doctor-plugin/src/integrationTest/java/com/osacky/doctor/KotlinDaemonFallbackIntegrationTest.kt

@@ -6,8 +6,8 @@ import org.junit.Test
 class KotlinDaemonFallbackIntegrationTest : AbstractIntegrationTest() {
     @Test
     fun testDisallowKotlinCompileDaemonFallback() {
-        writeKotlinBuildGradle(true)
-        writeSettingsFile()
+        writeKotlinBuildGradle()
+        writeSettingsFile(true)
         testProjectRoot.newFolder("src/main/java/foo")
         testProjectRoot.newFolder("src/test/java/foo")
         testProjectRoot.writeFileToName(
@@ -41,8 +41,8 @@ class KotlinDaemonFallbackIntegrationTest : AbstractIntegrationTest() {
 
     @Test
     fun allowKotlinCompileFallback() {
-        writeKotlinBuildGradle(false)
-        writeSettingsFile()
+        writeKotlinBuildGradle()
+        writeSettingsFile(false)
         testProjectRoot.newFolder("src/main/java/foo")
         testProjectRoot.writeFileToName(
             "src/main/java/foo/Foo.kt",
@@ -62,7 +62,7 @@ class KotlinDaemonFallbackIntegrationTest : AbstractIntegrationTest() {
         assertThat(result.output).contains("SUCCESS")
     }
 
-    private fun writeSettingsFile() {
+    private fun writeSettingsFile(allowDaemonFallback: Boolean) {
         testProjectRoot.writeFileToName(
             "settings.gradle",
             """
@@ -72,6 +72,17 @@ class KotlinDaemonFallbackIntegrationTest : AbstractIntegrationTest() {
                 gradlePluginPortal()
               }
             }
+
+            plugins {
+              id "com.osacky.doctor"
+            }
+
+            doctor {
+              warnIfKotlinCompileDaemonFallback = $allowDaemonFallback
+              javaHome {
+                ensureJavaHomeMatches = false
+              }
+            }
             """.trimIndent(),
         )
     }
@@ -80,22 +91,15 @@ class KotlinDaemonFallbackIntegrationTest : AbstractIntegrationTest() {
         createRunner()
             .withArguments("check", "-Dkotlin.daemon.jvm.options=invalid_jvm_argument_to_fail_process_startup")
 
-    private fun writeKotlinBuildGradle(allowDaemonFallback: Boolean) {
+    private fun writeKotlinBuildGradle() {
         testProjectRoot.writeBuildGradle(
             """
             plugins {
-              id "com.osacky.doctor"
               id "org.jetbrains.kotlin.jvm" version "1.6.10"
             }
             repositories {
               mavenCentral()
             }
-            doctor {
-              warnIfKotlinCompileDaemonFallback = $allowDaemonFallback
-              javaHome {
-                ensureJavaHomeMatches = false
-              }
-            }
             """.trimIndent(),
         )
     }

doctor-plugin/src/integrationTest/java/com/osacky/doctor/TestIntegrationTest.kt

@@ -12,7 +12,9 @@ class TestIntegrationTest {
 
     @Test
     fun testIgnoreOnEmptyDirectories() {
-        testProjectRoot.writeBuildGradle(
+        testProjectRoot.writeBuildGradle("")
+        val fixtureName = "java-fixture"
+        testProjectRoot.writeSettingsGradle(
             """
                     |plugins {
                     |  id "com.osacky.doctor"
@@ -25,10 +27,9 @@ class TestIntegrationTest {
                     |  failOnEmptyDirectories = true
                     |  warnWhenNotUsingParallelGC = false
                     |}
-                """.trimMargin("|"),
+                    |include '$fixtureName'
+                """.trimMargin("|")
         )
-        val fixtureName = "java-fixture"
-        testProjectRoot.newFile("settings.gradle").writeText("include '$fixtureName'")
         testProjectRoot.setupFixture(fixtureName)
         testProjectRoot.newFolder("java-fixture", "src", "main", "java", "com", "foo")
 
@@ -46,7 +47,9 @@ class TestIntegrationTest {
 
     @Test
     fun testDirectoriesIgnoredIn6dot8() {
-        testProjectRoot.writeBuildGradle(
+        testProjectRoot.writeBuildGradle("")
+        val fixtureName = "java-fixture"
+        testProjectRoot.writeSettingsGradle(
             """
                     |plugins {
                     |  id "com.osacky.doctor"
@@ -59,10 +62,9 @@ class TestIntegrationTest {
                     |  failOnEmptyDirectories = true
                     |  warnWhenNotUsingParallelGC = false
                     |}
-                """.trimMargin("|"),
+                    |include '$fixtureName'
+                """.trimMargin("|")
         )
-        val fixtureName = "java-fixture"
-        testProjectRoot.newFile("settings.gradle").writeText("include '$fixtureName'")
         testProjectRoot.setupFixture(fixtureName)
         testProjectRoot.newFolder("java-fixture", "src", "main", "java", "com", "foo")
 
@@ -135,18 +137,8 @@ class TestIntegrationTest {
         testProjectRoot.writeBuildGradle(
             """
             plugins {
-              id "com.osacky.doctor"
               id 'java-library'
             }
-            doctor {
-              disallowMultipleDaemons = false
-              javaHome {
-                ensureJavaHomeMatches = false
-              }
-              warnWhenNotUsingParallelGC = false
-              disallowCleanTaskDependencies = $disallowCleanTaskDependencies
-            }
-
             tasks.register('foo') {
               doFirst {
                 println 'foo'
@@ -158,5 +150,21 @@ class TestIntegrationTest {
             }
             """.trimIndent(),
         )
+
+        testProjectRoot.writeSettingsGradle(
+            """
+            plugins {
+              id "com.osacky.doctor"
+            }
+            doctor {
+              disallowMultipleDaemons = false
+              javaHome {
+                ensureJavaHomeMatches = false
+              }
+              warnWhenNotUsingParallelGC = false
+              disallowCleanTaskDependencies = $disallowCleanTaskDependencies
+            }
+            """.trimIndent()
+        )
     }
 }

doctor-plugin/src/main/java/com/osacky/doctor/AppProjectCollectorBuildService.kt

@@ -0,0 +1,25 @@
+package com.osacky.doctor
+
+import org.gradle.api.Project
+import org.gradle.api.services.BuildService
+import org.gradle.api.services.BuildServiceParameters
+
+abstract class AppProjectCollectorBuildService : BuildService<BuildServiceParameters.None> {
+    private val appProjects = mutableSetOf<Project>()
+
+    fun addProject(project: Project) {
+        appProjects.add(project)
+    }
+
+    fun getProjects(): Set<Project> {
+        return appProjects
+    }
+}
+
+fun Project.getAppProjectCollectorBuildService(): AppProjectCollectorBuildService {
+    return project.gradle.sharedServices.registerIfAbsent(
+        "appProjectCollector",
+        AppProjectCollectorBuildService::class.java,
+        {}
+    ).get()
+}

doctor-plugin/src/main/java/com/osacky/doctor/DoctorChildModulePlugin.kt

@@ -0,0 +1,48 @@
+package com.osacky.doctor
+
+import com.osacky.doctor.internal.farthestEmptyParent
+import org.gradle.api.Plugin
+import org.gradle.api.Project
+import org.gradle.api.tasks.SourceTask
+import org.gradle.api.tasks.testing.Test
+import org.gradle.util.GradleVersion
+
+class DoctorChildModulePlugin : Plugin<Project> {
+    override fun apply(target: Project) {
+        val extension = target.getDoctorExtension()
+        target.tasks.withType(SourceTask::class.java).configureEach {
+            if (!gradleIgnoresEmptyDirectories() && extension.failOnEmptyDirectories.get()) {
+                // Fail build if empty directories are found. These cause build cache misses and should be ignored by Gradle.
+                doFirst {
+                    source.visit {
+                        if (file.isDirectory && file.listFiles().isEmpty()) {
+                            val farthestEmptyParent = file.farthestEmptyParent()
+                            throw IllegalStateException(
+                                "Empty src dir(s) found. This causes build cache misses. Run the following command to fix it.\n" +
+                                        "rmdir ${farthestEmptyParent.absolutePath}",
+                            )
+                        }
+                    }
+                }
+            }
+        }
+        // Ensure we are not caching any test tasks. Tests may not declare all inputs properly or depend on things like the date and caching them can lead to dangerous false positives.
+        target.tasks.withType(Test::class.java).configureEach {
+            if (!extension.enableTestCaching.get()) {
+                outputs.upToDateWhen { false }
+            }
+        }
+        target.plugins.whenPluginAdded plugin@{
+            if (this.javaClass.name == "com.android.build.gradle.AppPlugin") {
+                target.getAppProjectCollectorBuildService().addProject(target)
+            }
+        }
+    }
+
+    /**
+     * Gradle now ignores empty directories starting in 6.8
+     * https://docs.gradle.org/6.8-rc-1/release-notes.html#performance-improvements
+     **/
+    private fun gradleIgnoresEmptyDirectories(): Boolean = GradleVersion.current() >= GradleVersion.version("6.8-rc-1")
+
+}

doctor-plugin/src/main/java/com/osacky/doctor/DoctorExtension.kt

@@ -2,7 +2,10 @@ package com.osacky.doctor
 
 import com.osacky.doctor.AppleRosettaTranslationCheckMode.ERROR
 import org.gradle.api.Action
+import org.gradle.api.GradleException
+import org.gradle.api.Project
 import org.gradle.api.model.ObjectFactory
+import org.gradle.api.plugins.ExtraPropertiesExtension
 import org.gradle.kotlin.dsl.newInstance
 import org.gradle.kotlin.dsl.property
 import javax.inject.Inject
@@ -100,6 +103,16 @@ open class DoctorExtension(
     fun javaHome(action: Action<JavaHomeHandler>) {
         action.execute(javaHomeHandler)
     }
+
+    companion object {
+        const val EXTRAS_KEY = "_doctorExtension_settings"
+    }
+}
+
+fun Project.getDoctorExtension(): DoctorExtension {
+    val defaults = extensions.getByType(ExtraPropertiesExtension::class.java).get(DoctorExtension.EXTRAS_KEY)
+            as? DoctorExtension ?: throw GradleException("Settings extension type mismatch")
+    return defaults
 }
 
 abstract class JavaHomeHandler

doctor-plugin/src/main/java/com/osacky/doctor/DoctorPlugin.kt

@@ -11,7 +11,6 @@ import com.osacky.doctor.internal.PillBoxPrinter
 import com.osacky.doctor.internal.SystemClock
 import com.osacky.doctor.internal.UnixDaemonChecker
 import com.osacky.doctor.internal.UnsupportedOsDaemonChecker
-import com.osacky.doctor.internal.farthestEmptyParent
 import com.osacky.doctor.internal.shouldUseCoCaClasses
 import org.gradle.api.Action
 import org.gradle.api.GradleException
@@ -20,13 +19,10 @@ import org.gradle.api.Project
 import org.gradle.api.internal.GradleInternal
 import org.gradle.api.invocation.Gradle
 import org.gradle.api.tasks.Delete
-import org.gradle.api.tasks.SourceTask
-import org.gradle.api.tasks.testing.Test
 import org.gradle.internal.build.event.BuildEventListenerRegistryInternal
 import org.gradle.internal.jvm.Jvm
 import org.gradle.internal.operations.BuildOperationListener
 import org.gradle.internal.operations.BuildOperationListenerManager
-import org.gradle.kotlin.dsl.create
 import org.gradle.kotlin.dsl.support.serviceOf
 import org.gradle.kotlin.dsl.withType
 import org.gradle.launcher.daemon.server.scaninfo.DaemonScanInfo
@@ -39,7 +35,7 @@ class DoctorPlugin : Plugin<Project> {
         ensureMinimumSupportedGradleVersion()
         ensureAppliedInProjectRoot(target)
 
-        val extension = target.extensions.create<DoctorExtension>("doctor")
+        val extension = target.getDoctorExtension()
 
         val os: OperatingSystem = DefaultNativePlatform.getCurrentOperatingSystem()
         val cliCommandExecutor = CliCommandExecutor(target)
@@ -99,41 +95,11 @@ class DoctorPlugin : Plugin<Project> {
 
         tagFreshDaemon(target, buildScanApi)
 
-        val appPluginProjects = mutableSetOf<Project>()
-
         ensureNoCleanTaskDependenciesIfNeeded(target, extension, pillBoxPrinter)
 
-        target.subprojects project@{
-            tasks.withType(SourceTask::class.java).configureEach {
-                if (!gradleIgnoresEmptyDirectories() && extension.failOnEmptyDirectories.get()) {
-                    // Fail build if empty directories are found. These cause build cache misses and should be ignored by Gradle.
-                    doFirst {
-                        source.visit {
-                            if (file.isDirectory && file.listFiles().isEmpty()) {
-                                val farthestEmptyParent = file.farthestEmptyParent()
-                                throw IllegalStateException(
-                                    "Empty src dir(s) found. This causes build cache misses. Run the following command to fix it.\n" +
-                                        "rmdir ${farthestEmptyParent.absolutePath}",
-                                )
-                            }
-                        }
-                    }
-                }
-            }
-            // Ensure we are not caching any test tasks. Tests may not declare all inputs properly or depend on things like the date and caching them can lead to dangerous false positives.
-            tasks.withType(Test::class.java).configureEach {
-                if (!extension.enableTestCaching.get()) {
-                    outputs.upToDateWhen { false }
-                }
-            }
-            plugins.whenPluginAdded plugin@{
-                if (this.javaClass.name == "com.android.build.gradle.AppPlugin") {
-                    appPluginProjects.add(this@project)
-                }
-            }
-        }
-
         target.gradle.taskGraph.whenReady {
+            val service = target.getAppProjectCollectorBuildService()
+            val appPluginProjects = service.getProjects()
             // If there is only one application plugin, we don't need to check that we're assembling all the applications.
             if (appPluginProjects.size <= 1 || extension.allowBuildingAllAndroidAppsSimultaneously.get()) {
                 return@whenReady
@@ -274,12 +240,6 @@ class DoctorPlugin : Plugin<Project> {
             ops
         }
 
-    /**
-     * Gradle now ignores empty directories starting in 6.8
-     * https://docs.gradle.org/6.8-rc-1/release-notes.html#performance-improvements
-     **/
-    private fun gradleIgnoresEmptyDirectories(): Boolean = GradleVersion.current() >= GradleVersion.version("6.8-rc-1")
-
     private val Gradle.buildOperationListenerManager get() = (this as GradleInternal).services[BuildOperationListenerManager::class.java]
 
     class TheActionThing(