Skip to content

Commit

Permalink
fix(DependencyGraphBuilder): Skip deep comparison for Gradle projects
Browse files Browse the repository at this point in the history
This can be avoided as it leads to a long runtime (and possible an
infinite loop / recursion) for large dependency graphs. In order
to be able to skip the deep comparison, the selected variants
need to be tracked in the Identifier. In the DependencyGraphBuilder
all 'scopes' (runtimeClasspath, compileClasspath, ...) are thrown
together. Gradle may have selected different variants of the components
in different 'scopes'.

Signed-off-by: Jendrik Johannes <[email protected]>
  • Loading branch information
jjohannes committed Feb 4, 2025
1 parent 8af12fb commit 7229394
Show file tree
Hide file tree
Showing 8 changed files with 35 additions and 4 deletions.
7 changes: 6 additions & 1 deletion model/src/main/kotlin/Identifier.kt
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,12 @@ data class Identifier(
/**
* The version of the component.
*/
val version: String
val version: String,

/**
* The selected variants of the component.
*/
val variants: Set<String> = emptySet()
) : Comparable<Identifier> {
companion object {
/**
Expand Down
11 changes: 9 additions & 2 deletions model/src/main/kotlin/utils/DependencyGraphBuilder.kt
Original file line number Diff line number Diff line change
Expand Up @@ -326,9 +326,16 @@ class DependencyGraphBuilder<D>(
val dependencies = dependencyHandler.dependenciesFor(dependency)
if (ref.dependencies.size != dependencies.size) return false

val dependencies1 = ref.dependencies.map { dependencyIds[it.pkg] }
val dependencies1 = ref.dependencies.mapTo(mutableSetOf()) { dependencyIds[it.pkg] }
val dependencies2 = dependencies.associateBy { dependencyHandler.identifierFor(it) }
if (!dependencies2.keys.containsAll(dependencies1)) return false

if (dependencies1 == dependencies2.keys) {
if (!dependencyHandler.requiresDeepDependencyTreeComparison()) {
return true

Check warning on line 334 in model/src/main/kotlin/utils/DependencyGraphBuilder.kt

View check run for this annotation

Codecov / codecov/patch

model/src/main/kotlin/utils/DependencyGraphBuilder.kt#L334

Added line #L334 was not covered by tests
}
} else {
return false

Check warning on line 337 in model/src/main/kotlin/utils/DependencyGraphBuilder.kt

View check run for this annotation

Codecov / codecov/patch

model/src/main/kotlin/utils/DependencyGraphBuilder.kt#L337

Added line #L337 was not covered by tests
}

return ref.dependencies.all { refDep ->
dependencies2[dependencyIds[refDep.pkg]]?.let { dependencyTreeEquals(refDep, it) } == true
Expand Down
7 changes: 7 additions & 0 deletions model/src/main/kotlin/utils/DependencyHandler.kt
Original file line number Diff line number Diff line change
Expand Up @@ -65,4 +65,11 @@ interface DependencyHandler<D> {
* implementation returns an empty collection.
*/
fun issuesForDependency(dependency: D): List<Issue> = emptyList()

/**
* Does node comparison require a deep comparison of the whole dependency subtree or not? If the underlying
* dependency management system, gives the guarantee for the latter, a costly comparison can be avoided and
* speed up the analysis process.
*/
fun requiresDeepDependencyTreeComparison() = true
}
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,12 @@ internal class GradleDependencyHandler(
isMetadataOnly = hasNoArtifacts
)
}

/*
* In case of Gradle, the costly deep comparison can be skipped, because if the direct dependencies are the same,
* their children are also the same.
*/
override fun requiresDeepDependencyTreeComparison() = false
}

// See http://maven.apache.org/pom.html#SCM.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ interface OrtDependency {
val groupId: String
val artifactId: String
val version: String
val variants: Set<String>
val classifier: String
val extension: String
val dependencies: List<OrtDependency>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ internal class OrtModelBuilder : ToolingModelBuilder {
groupId = id.group,
artifactId = id.module,
version = id.version,
variants = selectedComponent.variants.map { it.displayName }.toSet(),
classifier = "",
extension = modelBuildingResult?.effectiveModel?.packaging.orEmpty(),
dependencies = dependencies,
Expand Down Expand Up @@ -252,6 +253,7 @@ internal class OrtModelBuilder : ToolingModelBuilder {
groupId = moduleId.group,
artifactId = moduleId.name,
version = moduleId.version.takeUnless { it == "unspecified" }.orEmpty(),
variants = selectedComponent.variants.map { it.displayName }.toSet(),
classifier = "",
extension = "",
dependencies = dependencies,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ internal class OrtDependencyImpl(
override val artifactId: String,
override val version: String,
override val classifier: String,
override val variants: Set<String>,
override val extension: String,
override val dependencies: List<OrtDependency>,
override val error: String?,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ interface OrtDependency {
String getArtifactId()
String getVersion()
String getClassifier()
Set<String> getVariants()
String getExtension()
List<OrtDependency> getDependencies()
String getError()
Expand Down Expand Up @@ -104,6 +105,7 @@ class OrtDependencyImpl implements OrtDependency, Serializable {
String groupId = ''
String artifactId = ''
String version = ''
Set<String> variants = []
String classifier = ''
String extension = ''
List<OrtDependency> dependencies = []
Expand Down Expand Up @@ -369,7 +371,7 @@ class AbstractOrtDependencyTreePlugin<T> implements Plugin<T> {
def classifier = artifact?.classifier ?: ''
def extension = artifact?.extension ?: ''

return new OrtDependencyImpl(id.group, id.module, id.version, classifier, extension, dependencies,
return new OrtDependencyImpl(id.group, id.module, id.version, [] as Set, classifier, extension, dependencies,
error, warning, pomFile, null)
} else if (id instanceof ProjectComponentIdentifier) {
if (id.build.isCurrentBuild() || !project.gradle.gradleVersion.isAtLeastVersion(7, 2)) {
Expand Down

0 comments on commit 7229394

Please sign in to comment.