diff --git a/affectedmoduledetector/src/main/kotlin/com/dropbox/affectedmoduledetector/AffectedModuleDetector.kt b/affectedmoduledetector/src/main/kotlin/com/dropbox/affectedmoduledetector/AffectedModuleDetector.kt index 5f891ed..f5ca3fa 100644 --- a/affectedmoduledetector/src/main/kotlin/com/dropbox/affectedmoduledetector/AffectedModuleDetector.kt +++ b/affectedmoduledetector/src/main/kotlin/com/dropbox/affectedmoduledetector/AffectedModuleDetector.kt @@ -32,7 +32,10 @@ import org.gradle.api.GradleException import org.gradle.api.Project import org.gradle.api.Task import org.gradle.api.file.DirectoryProperty +import org.gradle.api.file.RegularFileProperty import org.gradle.api.logging.Logger +import org.gradle.api.provider.ListProperty +import org.gradle.api.provider.Property import org.gradle.api.provider.Provider import org.gradle.api.services.BuildService import org.gradle.api.services.BuildServiceParameters @@ -155,28 +158,20 @@ abstract class AffectedModuleDetector(protected val logger: Logger?) { "extension added." } - val distDir = if (config.logFolder != null) { - val distDir = File(config.logFolder!!) - if (!distDir.exists()) { - distDir.mkdirs() - } - distDir + val logFile = if (config.logFolder != null) { + File(config.logFolder!!).resolve(config.logFilename) } else { - rootProject.rootDir - } - - val outputFile = distDir.resolve(config.logFilename).also { - it.writeText("") + rootProject.rootDir.resolve(config.logFilename) } - val logger = FileLogger(outputFile) val enabled = isProjectEnabled(rootProject) if (!enabled) { val provider = setupWithParams(rootProject) { spec -> val params = spec.parameters - params.acceptAll = true - params.log = logger + params.acceptAll.set(true) + params.logFile.set(logFile) + params.workingDir.set(rootProject.projectDir) } instance.wrapped = provider return @@ -195,14 +190,11 @@ abstract class AffectedModuleDetector(protected val logger: Logger?) { } } - val modules = - getModulesProperty( - rootProject - ) + val modules = getModulesProperty(rootProject) val gitClient = GitClientImpl( rootProject.projectDir, - logger, + null, commitShaProviderConfiguration = CommitShaProviderConfiguration( type = config.compareFrom, specifiedBranch = config.specifiedBranch, @@ -213,23 +205,22 @@ abstract class AffectedModuleDetector(protected val logger: Logger?) { ignoredFiles = config.ignoredFiles ) - logger.lifecycle("projects evaluated") - val projectGraph = ProjectGraph(rootProject, logger.toLogger()) - val dependencyTracker = DependencyTracker(rootProject, logger.toLogger()) + val fileLogger = FileLogger(logFile) + val projectGraph = ProjectGraph(rootProject, fileLogger.toLogger()) + val dependencyTracker = DependencyTracker(rootProject, fileLogger.toLogger()) val provider = setupWithParams(rootProject) { spec -> val parameters = spec.parameters - parameters.acceptAll = false - parameters.projectGraph = projectGraph - parameters.dependencyTracker = dependencyTracker - parameters.log = logger - parameters.ignoreUnknownProjects = true - parameters.projectSubset = subset - parameters.modules = modules - parameters.config = config - parameters.gitChangedFilesProvider = gitClient.findChangedFiles(rootProject) - parameters.gitRoot.set(gitClient.getGitRoot()) + parameters.acceptAll.set(false) + parameters.projectGraph.set(projectGraph) + parameters.dependencyTracker.set(dependencyTracker) + parameters.logFile.set(logFile) + parameters.ignoreUnknownProjects.set(true) + parameters.projectSubset.set(subset) + if (modules != null) parameters.modules.set(modules.toList()) + parameters.config.set(config) + parameters.gitChangedFilesProvider.set(gitClient.findChangedFiles(rootProject)) + parameters.workingDir.set(rootProject.projectDir) } - logger.info("Using real detector with $subset") instance.wrapped = provider } @@ -288,10 +279,13 @@ abstract class AffectedModuleDetector(protected val logger: Logger?) { @Throws(GradleException::class) @JvmStatic fun configureTaskGuard(task: Task) { + // Capture only serializable values (String path) rather than live Task/Project + // references, which are not configuration-cache compatible. + val projectPath = task.project.path + val detectorExtension = task.project.rootProject.extensions.getByName(ROOT_PROP_NAME) + as AffectedModuleDetectorWrapper task.onlyIf { - getOrThrow( - task.project - ).shouldInclude(task.project.projectPath) + detectorExtension.shouldInclude(ProjectPath(projectPath)) } } @@ -402,36 +396,57 @@ class AffectedModuleDetectorWrapper : AffectedModuleDetector(logger = null) { abstract class AffectedModuleDetectorLoader : BuildService { interface Parameters : BuildServiceParameters { - var acceptAll: Boolean - var projectGraph: ProjectGraph - var dependencyTracker: DependencyTracker - var log: FileLogger - var ignoreUnknownProjects: Boolean - var projectSubset: ProjectSubset - var modules: Set? - var gitChangedFilesProvider: Provider> - var config: AffectedModuleConfiguration - val gitRoot: DirectoryProperty + val acceptAll: Property + val projectGraph: Property + val dependencyTracker: Property + val logFile: RegularFileProperty + val ignoreUnknownProjects: Property + val projectSubset: Property + val modules: ListProperty + val gitChangedFilesProvider: Property>> + val config: Property + // workingDir is used to locate the git root inside the service, avoiding a + // filesystem walk at configuration time. + val workingDir: DirectoryProperty } val detector: AffectedModuleDetector by lazy { - val logger = parameters.log.toLogger() - if (parameters.acceptAll) { + val logFile = parameters.logFile.orNull?.asFile + if (logFile != null) { + logFile.parentFile?.mkdirs() + if (!logFile.exists()) logFile.createNewFile() + } + val logger = logFile?.let { FileLogger(it).toLogger() } + + if (parameters.acceptAll.get()) { AcceptAll(logger) } else { + val workingDir = parameters.workingDir.get().asFile + val gitRoot = findGitDirInParentFilepath(workingDir) ?: workingDir AffectedModuleDetectorImpl( - projectGraph = parameters.projectGraph, - dependencyTracker = parameters.dependencyTracker, + projectGraph = parameters.projectGraph.get(), + dependencyTracker = parameters.dependencyTracker.get(), logger = logger, - ignoreUnknownProjects = parameters.ignoreUnknownProjects, - projectSubset = parameters.projectSubset, - modules = parameters.modules, - config = parameters.config, - changedFilesProvider = parameters.gitChangedFilesProvider, - gitRoot = parameters.gitRoot.get().asFile + ignoreUnknownProjects = parameters.ignoreUnknownProjects.get(), + projectSubset = parameters.projectSubset.get(), + modules = parameters.modules.orNull?.toSet()?.takeIf { it.isNotEmpty() }, + config = parameters.config.get(), + changedFilesProvider = parameters.gitChangedFilesProvider.get(), + gitRoot = gitRoot ) } } + + private fun findGitDirInParentFilepath(filepath: File): File? { + var curDirectory: File = filepath + while (curDirectory.path != "/") { + if (File("$curDirectory/.git").exists()) { + return curDirectory + } + curDirectory = curDirectory.parentFile + } + return null + } } /** diff --git a/affectedmoduledetector/src/main/kotlin/com/dropbox/affectedmoduledetector/AffectedModuleDetectorPlugin.kt b/affectedmoduledetector/src/main/kotlin/com/dropbox/affectedmoduledetector/AffectedModuleDetectorPlugin.kt index 9f4edc1..27e3f30 100644 --- a/affectedmoduledetector/src/main/kotlin/com/dropbox/affectedmoduledetector/AffectedModuleDetectorPlugin.kt +++ b/affectedmoduledetector/src/main/kotlin/com/dropbox/affectedmoduledetector/AffectedModuleDetectorPlugin.kt @@ -8,7 +8,6 @@ import org.gradle.api.Plugin import org.gradle.api.Project import org.gradle.api.Task import org.gradle.api.tasks.testing.Test -import org.gradle.util.GradleVersion import org.jetbrains.annotations.VisibleForTesting /** @@ -90,7 +89,6 @@ class AffectedModuleDetectorPlugin : Plugin { rootProject.tasks.register(taskType.commandByImpact) { task -> task.group = CUSTOM_TASK_GROUP_NAME task.description = taskType.taskDescription - disableConfigCache(task) rootProject.subprojects { project -> pluginIds.forEach { pluginId -> @@ -131,7 +129,6 @@ class AffectedModuleDetectorPlugin : Plugin { rootProject.tasks.register(taskType.commandByImpact) { task -> task.group = groupName task.description = taskType.taskDescription - disableConfigCache(task) rootProject.subprojects { project -> pluginIds.forEach { pluginId -> @@ -147,13 +144,6 @@ class AffectedModuleDetectorPlugin : Plugin { } } - @Suppress("UnstableApiUsage") - private fun disableConfigCache(task: Task) { - if (GradleVersion.current() >= GradleVersion.version("7.4")) { - task.notCompatibleWithConfigurationCache("AMD requires knowledge of what has changed in the file system so we can not cache those values (https://github.com/dropbox/AffectedModuleDetector/issues/150)") - } - } - private fun withPlugin( pluginId: String, task: Task, @@ -169,16 +159,15 @@ class AffectedModuleDetectorPlugin : Plugin { project.pluginManager.withPlugin(pluginId) { getAffectedPath(testType, project)?.let { path -> val pathOrNull = project.tasks.findByPath(path) - val onlyIf = when { - pathOrNull == null -> false - !AffectedModuleDetector.isProjectEnabled(pathOrNull.project) -> true - else -> AffectedModuleDetector.isProjectAffected(pathOrNull.project) - } + if (pathOrNull == null) return@withPlugin - if (onlyIf && AffectedModuleDetector.isProjectProvided(project) && !isExcludedModule(config, path)) { - task.dependsOn(path) - } - pathOrNull?.onlyIf { onlyIf } + val isProvided = AffectedModuleDetector.isProjectProvided(project) + if (!isProvided || isExcludedModule(config, path)) return@withPlugin + + task.dependsOn(path) + // Defer the affected/not-affected decision to execution time via configureTaskGuard + // so neither the result nor any Project/Task reference is baked into the cache. + AffectedModuleDetector.configureTaskGuard(pathOrNull) } } } diff --git a/affectedmoduledetector/src/main/kotlin/com/dropbox/affectedmoduledetector/DependencyTracker.kt b/affectedmoduledetector/src/main/kotlin/com/dropbox/affectedmoduledetector/DependencyTracker.kt index 924b3e0..2695f62 100644 --- a/affectedmoduledetector/src/main/kotlin/com/dropbox/affectedmoduledetector/DependencyTracker.kt +++ b/affectedmoduledetector/src/main/kotlin/com/dropbox/affectedmoduledetector/DependencyTracker.kt @@ -30,26 +30,35 @@ import java.io.Serializable */ class DependencyTracker(rootProject: Project, logger: Logger?) : Serializable { - private val dependentList: Map> by lazy { - val result = mutableMapOf>() - rootProject.subprojects.forEach { project -> - logger?.info("checking ${project.path} for dependencies") - project.configurations.forEach { config -> - logger?.info("checking config ${project.path}/$config for dependencies") - config - .dependencies - .filterIsInstance() - .forEach { - logger?.info( - "there is a dependency from ${project.projectPath} to " + - it.path, - ) - result.getOrPut(ProjectPath(it.path)) { mutableSetOf() } - .add(project.projectPath) - } + // Eagerly computed at construction so no live Project reference is retained in a + // serializable field (required for Gradle configuration cache compatibility). + private val dependentList: Map> = buildDependentList(rootProject, logger) + + companion object { + private fun buildDependentList( + rootProject: Project, + logger: Logger? + ): Map> { + val result = mutableMapOf>() + rootProject.subprojects.forEach { project -> + logger?.info("checking ${project.path} for dependencies") + project.configurations.forEach { config -> + logger?.info("checking config ${project.path}/$config for dependencies") + config + .dependencies + .filterIsInstance() + .forEach { + logger?.info( + "there is a dependency from ${project.projectPath} to " + + it.path, + ) + result.getOrPut(ProjectPath(it.path)) { mutableSetOf() } + .add(project.projectPath) + } + } } + return result } - result } fun findAllDependents(projectPath: ProjectPath, logger: Logger? = null): Set { diff --git a/affectedmoduledetector/src/main/kotlin/com/dropbox/affectedmoduledetector/GitClient.kt b/affectedmoduledetector/src/main/kotlin/com/dropbox/affectedmoduledetector/GitClient.kt index 1d8a7ef..558ab68 100644 --- a/affectedmoduledetector/src/main/kotlin/com/dropbox/affectedmoduledetector/GitClient.kt +++ b/affectedmoduledetector/src/main/kotlin/com/dropbox/affectedmoduledetector/GitClient.kt @@ -31,7 +31,9 @@ import com.dropbox.affectedmoduledetector.util.toOsSpecificLineEnding import com.dropbox.affectedmoduledetector.util.toOsSpecificPath import org.gradle.api.Project import org.gradle.api.file.DirectoryProperty +import org.gradle.api.file.RegularFileProperty import org.gradle.api.logging.Logger +import org.gradle.api.provider.Property import org.gradle.api.provider.Provider import org.gradle.api.provider.SetProperty import org.gradle.api.provider.ValueSource @@ -91,9 +93,9 @@ internal class GitClientImpl( project: Project, ): Provider> { return project.providers.of(GitChangedFilesSource::class.java) { - it.parameters.commitShaProvider = commitShaProviderConfiguration + it.parameters.commitShaProvider.set(commitShaProviderConfiguration) it.parameters.workingDir.set(workingDir) - it.parameters.logger = logger + it.parameters.logFile.set(logger?.file) it.parameters.ignoredFiles.set(ignoredFiles) } } @@ -176,9 +178,9 @@ private class RealCommandRunner( internal abstract class GitChangedFilesSource : ValueSource, GitChangedFilesSource.Parameters> { interface Parameters : ValueSourceParameters { - var commitShaProvider: CommitShaProviderConfiguration + val commitShaProvider: Property val workingDir: DirectoryProperty - var logger: FileLogger? + val logFile: RegularFileProperty val ignoredFiles: SetProperty } @@ -189,17 +191,18 @@ internal abstract class GitChangedFilesSource : private val commandRunner: GitClient.CommandRunner by lazy { RealCommandRunner( workingDir = gitRoot ?: parameters.workingDir.get().asFile, - logger = null + logger = parameters.logFile.orNull?.asFile?.let { FileLogger(it).toLogger() } ) } override fun obtain(): List { - val top = parameters.commitShaProvider.top + val commitShaProviderConfig = parameters.commitShaProvider.get() + val top = commitShaProviderConfig.top val sha = getSha() // use this if we don't want local changes val changedFiles = commandRunner.executeAndParse( - if (parameters.commitShaProvider.includeUncommitted) { + if (commitShaProviderConfig.includeUncommitted) { "$CHANGED_FILES_CMD_PREFIX $sha" } else { "$CHANGED_FILES_CMD_PREFIX $top..$sha" @@ -227,9 +230,10 @@ internal abstract class GitChangedFilesSource : } private fun getSha(): Sha { - val specifiedBranch = parameters.commitShaProvider.specifiedBranch - val specifiedSha = parameters.commitShaProvider.specifiedSha - val type = when (parameters.commitShaProvider.type) { + val config = parameters.commitShaProvider.get() + val specifiedBranch = config.specifiedBranch + val specifiedSha = config.specifiedSha + val type = when (config.type) { "PreviousCommit" -> PreviousCommit() "ForkCommit" -> ForkCommit() "SpecifiedBranchCommit" -> { diff --git a/affectedmoduledetector/src/test/kotlin/com/dropbox/affectedmoduledetector/ConfigurationCacheIntegrationTest.kt b/affectedmoduledetector/src/test/kotlin/com/dropbox/affectedmoduledetector/ConfigurationCacheIntegrationTest.kt new file mode 100644 index 0000000..5a3ea6a --- /dev/null +++ b/affectedmoduledetector/src/test/kotlin/com/dropbox/affectedmoduledetector/ConfigurationCacheIntegrationTest.kt @@ -0,0 +1,71 @@ +package com.dropbox.affectedmoduledetector + +import com.dropbox.affectedmoduledetector.rules.SetupAndroidProject +import com.google.common.truth.Truth.assertThat +import org.gradle.testkit.runner.GradleRunner +import org.junit.Rule +import org.junit.Test + +class ConfigurationCacheIntegrationTest { + + @Rule + @JvmField + val tmpFolder = SetupAndroidProject() + + private fun setupProject() { + tmpFolder.newFile("settings.gradle").writeText("") + tmpFolder.newFile("build.gradle").writeText( + """plugins { + | id "health.flo.affectedmoduledetector" + |}""".trimMargin() + ) + // Init a git repo so the plugin can locate the git root without erroring + fun git(vararg args: String) = ProcessBuilder("git", *args) + .directory(tmpFolder.root) + .also { it.environment()["GIT_CONFIG_NOSYSTEM"] = "1" } + .start().waitFor() + git("init") + git("config", "user.email", "test@test.com") + git("config", "user.name", "Test") + git("commit", "--allow-empty", "-m", "init") + } + + @Test + fun `GIVEN configuration cache enabled WHEN plugin is applied THEN build is configuration cache compatible`() { + // GIVEN + setupProject() + + // WHEN — runAffectedUnitTests puts an AMD task in the execution plan, which is where + // the notCompatibleWithConfigurationCache() call takes effect. Using `tasks` avoids AMD + // tasks entirely and would give a false-positive pass. + val result = GradleRunner.create() + .withProjectDir(tmpFolder.root) + .withPluginClasspath() + .withArguments("runAffectedUnitTests", "--configuration-cache", "-Paffected_module_detector.enable") + .build() + + // THEN — the cache must be stored, not discarded due to an incompatible task + assertThat(result.output).contains("Configuration cache entry stored.") + assertThat(result.output).doesNotContain("Configuration cache entry discarded") + } + + @Test + fun `GIVEN configuration cache enabled WHEN build runs twice THEN configuration cache is reused on second run`() { + // GIVEN + setupProject() + + val runner = GradleRunner.create() + .withProjectDir(tmpFolder.root) + .withPluginClasspath() + .withArguments("runAffectedUnitTests", "--configuration-cache", "-Paffected_module_detector.enable") + + // First run — should store the configuration cache (currently does not due to incompatibility) + runner.build() + + // WHEN — second run + val result = runner.build() + + // THEN + assertThat(result.output).contains("Reusing configuration cache.") + } +}