Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Race Condition during afterEvaluate Configuration Validation in 0.4.2 #90

Merged
merged 1 commit into from
Mar 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion dependency-guard/gradle.properties
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
GROUP=com.dropbox.dependency-guard
VERSION_NAME=0.4.2
VERSION_NAME=0.4.3-SNAPSHOT

POM_ARTIFACT_ID=dependency-guard
POM_NAME=Dependency Guard
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ class PluginTest {
args = arrayOf(":lib:dependencyGuard")
)

assertThat(result.output).contains("Error: No configurations provided to Dependency Guard Plugin.")
assertThat(result.output).contains("Error: No configurations provided to Dependency Guard Plugin for project :lib")
}

@ParameterizedPluginTest
Expand All @@ -283,6 +283,6 @@ class PluginTest {
args = arrayOf(":lib:dependencyGuard")
)

assertThat(result.output).contains("Configuration with name releaseCompileClasspath was not found for :lib")
assertThat(result.output).contains("Dependency Guard could not resolve the configurations named [releaseCompileClasspath] for :lib")
}
}
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
package com.dropbox.gradle.plugins.dependencyguard

import com.dropbox.gradle.plugins.dependencyguard.internal.ConfigurationValidators
import com.dropbox.gradle.plugins.dependencyguard.internal.DependencyTreeDiffTaskNames
import com.dropbox.gradle.plugins.dependencyguard.internal.isRootProject
import com.dropbox.gradle.plugins.dependencyguard.internal.list.DependencyGuardListTask
import com.dropbox.gradle.plugins.dependencyguard.internal.projectConfigurations
import com.dropbox.gradle.plugins.dependencyguard.internal.tree.DependencyTreeDiffTask
import org.gradle.api.Plugin
import org.gradle.api.Project
Expand Down Expand Up @@ -46,33 +43,6 @@ public class DependencyGuardPlugin : Plugin<Project> {
target = target,
dependencyGuardTask = dependencyGuardTask
)

validateConfigurationAfterEvaluate(target, extension)
}

/**
* It's recommended to avoid `afterEvaluate` but in this case we are doing a validation of inputs
*
* Without adding this, there were cases when the `target.configurations` were not all available yet.
*/
private fun validateConfigurationAfterEvaluate(target: Project, extension: DependencyGuardPluginExtension) {
target.afterEvaluate {
val availableConfigurationNames = target.configurations.map { it.name }
val monitoredConfigurationNames = extension.configurations.map { it.configurationName }
ConfigurationValidators.requirePluginConfig(
projectPath = target.path,
isForRootProject = target.isRootProject(),
availableConfigurations = availableConfigurationNames,
monitoredConfigurations = monitoredConfigurationNames,
)
ConfigurationValidators.validateConfigurationsAreAvailable(
projectPath = target.path,
isForRootProject = target.isRootProject(),
logger = target.logger,
availableConfigurationNames = availableConfigurationNames,
monitoredConfigurationNames = monitoredConfigurationNames
)
}
}

private fun attachToCheckTask(target: Project, dependencyGuardTask: TaskProvider<DependencyGuardListTask>) {
Expand Down Expand Up @@ -110,8 +80,7 @@ public class DependencyGuardPlugin : Plugin<Project> {
DEPENDENCY_GUARD_TASK_NAME,
DependencyGuardListTask::class.java
) {
val task = this
task.setParams(
setParams(
project = target,
extension = extension,
shouldBaseline = false
Expand Down
Original file line number Diff line number Diff line change
@@ -1,25 +1,47 @@
package com.dropbox.gradle.plugins.dependencyguard.internal

import com.dropbox.gradle.plugins.dependencyguard.DependencyGuardConfiguration
import com.dropbox.gradle.plugins.dependencyguard.DependencyGuardPlugin
import com.dropbox.gradle.plugins.dependencyguard.DependencyGuardPluginExtension
import org.gradle.api.GradleException
import org.gradle.api.Project
import org.gradle.api.artifacts.result.ResolvedComponentResult
import org.gradle.api.initialization.dsl.ScriptHandler
import org.gradle.api.logging.Logger
import org.gradle.api.logging.Logging
import org.gradle.api.provider.Provider

internal object ConfigurationValidators {

private val logger = Logging.getLogger(DependencyGuardPlugin::class.java)

fun requirePluginConfig(
fun validatePluginConfiguration(
project: Project,
extension: DependencyGuardPluginExtension,
resolvedConfigurationsMap: Map<DependencyGuardConfiguration, Provider<ResolvedComponentResult>>
) {
val requestedConfigurations = extension.configurations.map { it.configurationName }
val validRequestedConfigurations = resolvedConfigurationsMap.keys.map { it.configurationName }
val allAvailableConfigurations = project.projectConfigurations.map { it.name }

validatePluginConfiguration(
projectPath = project.path,
isForRootProject = project.isRootProject(),
requestedConfigurations = requestedConfigurations,
validRequestedConfigurations = validRequestedConfigurations,
allAvailableConfigurations = allAvailableConfigurations,
)
}

internal fun validatePluginConfiguration(
projectPath: String,
isForRootProject: Boolean,
availableConfigurations: List<String>,
monitoredConfigurations: List<String>
requestedConfigurations: List<String>,
validRequestedConfigurations: List<String>,
allAvailableConfigurations: List<String>,
) {
if (isForRootProject) {
logger.info("Configured for Root Project")
if (monitoredConfigurations.isNotEmpty() && monitoredConfigurations.first() != ScriptHandler.CLASSPATH_CONFIGURATION) {
logger.error("If you wish to use dependency guard on your root project, use the following config:")
if (requestedConfigurations != listOf(ScriptHandler.CLASSPATH_CONFIGURATION)) {
logger.error("If you wish to use Dependency Guard on your root project, use the following config:")
throw GradleException(
"""
dependencyGuard {
Expand All @@ -29,35 +51,36 @@ internal object ConfigurationValidators {
)
}
} else {
val availableConfigNames = availableConfigurations
.filter { isClasspathConfig(it) }
if (requestedConfigurations.isEmpty()) {
throw GradleException(buildString {
appendLine("Error: No configurations provided to Dependency Guard Plugin for project $projectPath")
appendLine(configurationsYouCouldUseMessage(allAvailableConfigurations))
})
}

require(availableConfigNames.isNotEmpty()) {
buildString {
appendLine("Error: The Dependency Guard Plugin can not find any configurations ending in 'RuntimeClasspath' or 'CompileClasspath'.")
appendLine("Suggestion: Move your usage of Dependency Guard as access to these Classpath configurations are required to configure Dependency Guard correctly.")
appendLine("Why: Either you have applied the plugin too early or your project just doesn't have any matching Classpath configurations.")
appendLine("All available Gradle configurations for project $projectPath at this point of time:")
availableConfigurations.forEach {
appendLine("* $it")
val unresolvedConfigurations = requestedConfigurations.minus(validRequestedConfigurations.toSet())
if (unresolvedConfigurations.isNotEmpty()) {
throw GradleException(
buildString {
appendLine("Dependency Guard could not resolve the configurations named $unresolvedConfigurations for $projectPath")
appendLine(configurationsYouCouldUseMessage(allAvailableConfigurations))
}
}
)
}
}
}

val configurationNames = monitoredConfigurations.map { it }
require(configurationNames.isNotEmpty() && configurationNames[0].isNotBlank()) {
buildString {
appendLine("Error: No configurations provided to Dependency Guard Plugin.")
appendLine("Here are some valid configurations you could use.")
appendLine("")
private fun configurationsYouCouldUseMessage(availableConfigurations: List<String>): String = buildString {
val availableConfigNames = availableConfigurations.filter { isClasspathConfig(it) }
if (availableConfigNames.isNotEmpty()) {
appendLine("Here are some valid configurations you could use.")
appendLine("")

appendLine("dependencyGuard {")
availableConfigNames.forEach {
appendLine(""" configuration("$it")""")
}
appendLine("}")
}
appendLine("dependencyGuard {")
availableConfigNames.forEach {
appendLine(""" configuration("$it")""")
}
appendLine("}")
}
}

Expand All @@ -70,49 +93,4 @@ internal object ConfigurationValidators {
ignoreCase = true
)
}


fun validateConfigurationsAreAvailable(
isForRootProject: Boolean,
projectPath: String,
logger: Logger,
availableConfigurationNames: Collection<String>,
monitoredConfigurationNames: List<String>
) {

if (isForRootProject) {
if (monitoredConfigurationNames != listOf(ScriptHandler.CLASSPATH_CONFIGURATION)) {
throw GradleException(
"""
For the root project, the only allowed configuration is ${ScriptHandler.CLASSPATH_CONFIGURATION}.
Use the following config:
dependencyGuard {
configuration("classpath")
}
""".trimIndent()
)
}
return
}

monitoredConfigurationNames.forEach { monitoredConfigurationName ->
availableConfigurationNames
.firstOrNull { it == monitoredConfigurationName }
?: run {
val availableClasspathConfigs = availableConfigurationNames
.filter {
isClasspathConfig(it)
}
if (availableClasspathConfigs.isNotEmpty()) {
logger.quiet("Configuration with name $monitoredConfigurationName was not found. Available Configurations for $projectPath are:")
availableClasspathConfigs.forEach {
logger.quiet("* $it")
}
}
throw GradleException(
"Configuration with name $monitoredConfigurationName was not found for $projectPath"
)
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ internal val Project.projectConfigurations: ConfigurationContainer
configurations
}

internal fun ConfigurationContainer.getResolvedComponentResult(name: String): Provider<ResolvedComponentResult> = this
.getByName(name)
.incoming
.resolutionResult
.rootComponent
internal fun ConfigurationContainer.getResolvedComponentResult(name: String): Provider<ResolvedComponentResult>? = this
.findByName(name)
?.incoming
?.resolutionResult
?.rootComponent
Original file line number Diff line number Diff line change
Expand Up @@ -158,24 +158,43 @@ internal abstract class DependencyGuardListTask : DefaultTask() {
extension: DependencyGuardPluginExtension,
shouldBaseline: Boolean
) {
val resolvedConfigurationsMap = resolveMonitoredConfigurationsMap(
project = project,
monitoredConfigurations = extension.configurations
)

ConfigurationValidators.validatePluginConfiguration(
project = project,
extension = extension,
resolvedConfigurationsMap = resolvedConfigurationsMap,
)

this.forRootProject.set(project.isRootProject())
this.projectPath.set(project.path)
this.monitoredConfigurationsMap.set(resolveMonitoredConfigurationsMap(project, extension.configurations))
this.monitoredConfigurationsMap.set(resolvedConfigurationsMap)
this.shouldBaseline.set(shouldBaseline)
val projectDirDependenciesDir = OutputFileUtils.projectDirDependenciesDir(project)
this.projectDirectoryDependenciesDir.set(projectDirDependenciesDir)

declareCompatibilities()
}

/**
* Attempts to resolve configurations, and returns only the resolved ones
*/
private fun resolveMonitoredConfigurationsMap(
project: Project,
monitoredConfigurations: Collection<DependencyGuardConfiguration>,
): Map<DependencyGuardConfiguration, Provider<ResolvedComponentResult>> {
val configurationContainer = project.projectConfigurations

return monitoredConfigurations.associateWith {
configurationContainer.getResolvedComponentResult(it.configurationName)
val resolvedConfigurationsMap = mutableMapOf<DependencyGuardConfiguration, Provider<ResolvedComponentResult>>()
monitoredConfigurations.forEach { monitoredConfiguration ->
val resolvedComponentResultForConfiguration =
project.projectConfigurations.getResolvedComponentResult(monitoredConfiguration.configurationName)
if (resolvedComponentResultForConfiguration != null) {
resolvedConfigurationsMap[monitoredConfiguration] = resolvedComponentResultForConfiguration
}
}
return resolvedConfigurationsMap
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import com.dropbox.gradle.plugins.dependencyguard.internal.utils.OutputFileUtils
import com.dropbox.gradle.plugins.dependencyguard.internal.utils.Tasks.declareCompatibilities
import java.io.File
import org.gradle.api.DefaultTask
import org.gradle.api.GradleException
import org.gradle.api.Project
import org.gradle.api.artifacts.result.ResolvedComponentResult
import org.gradle.api.provider.Property
Expand Down Expand Up @@ -46,10 +47,22 @@ internal abstract class DependencyTreeDiffTask : DefaultTask() {
shouldBaseline: Boolean,
) {
val projectDependenciesDir = OutputFileUtils.projectDirDependenciesDir(project)
val projectDirOutputFile: File = DependencyGuardTreeDiffer.projectDirOutputFile(projectDependenciesDir, configurationName)
val buildDirOutputFile: File = DependencyGuardTreeDiffer.buildDirOutputFile(project.layout.buildDirectory.get(), configurationName)
val projectDirOutputFile: File = DependencyGuardTreeDiffer.projectDirOutputFile(
projectDirectory = projectDependenciesDir,
configurationName = configurationName
)
val buildDirOutputFile: File = DependencyGuardTreeDiffer.buildDirOutputFile(
buildDirectory = project.layout.buildDirectory.get(),
configurationName = configurationName
)
val projectPath = project.path
val resolvedComponentResult = project.projectConfigurations.getResolvedComponentResult(configurationName)

val resolvedComponentResult: Provider<ResolvedComponentResult> =
project.projectConfigurations.getResolvedComponentResult(configurationName)
?: throw GradleException(buildString {
appendLine("Dependency Guard could not resolve configuration $configurationName for $projectPath.")
appendLine("Please update your configuration and try again.")
})

this.resolvedComponentResult.set(resolvedComponentResult)
this.projectDirOutputFile.set(projectDirOutputFile)
Expand Down