From 3c9ad3fa9e5a859060b09134527734de0f415847 Mon Sep 17 00:00:00 2001 From: arunkumar9t2 Date: Tue, 1 Oct 2024 15:56:08 +0800 Subject: [PATCH] Implement direct dependency tags for maven external targets --- build.gradle | 3 + .../gradle/dependencies/Dependencies.kt | 86 ++++++++++++++----- .../migrate/android/AndroidExtractor.kt | 9 +- .../android/AndroidUnitTestDataExtractor.kt | 6 +- .../dependencies/ClasspathReduction.kt | 16 +++- .../kotlin/KotlinProjectDataExtractor.kt | 6 +- .../kotlin/KotlinUnitTestDataExtractor.kt | 6 +- .../dependencies/ClasspathReductionTest.kt | 58 +++++++++++++ 8 files changed, 162 insertions(+), 28 deletions(-) create mode 100644 grazel-gradle-plugin/src/test/kotlin/com/grab/grazel/migrate/dependencies/ClasspathReductionTest.kt diff --git a/build.gradle b/build.gradle index 6ffbc213..45330e34 100644 --- a/build.gradle +++ b/build.gradle @@ -137,6 +137,9 @@ grazel { languageVersion = "1.7" jvmTarget = "17" } + // Enable to add tags in generated tags that can be used to do classpath reduction + // for build performance. + enabledTransitiveReduction = false } dagger { tag = "2.47" diff --git a/grazel-gradle-plugin/src/main/kotlin/com/grab/grazel/gradle/dependencies/Dependencies.kt b/grazel-gradle-plugin/src/main/kotlin/com/grab/grazel/gradle/dependencies/Dependencies.kt index fb0c0728..15b79017 100644 --- a/grazel-gradle-plugin/src/main/kotlin/com/grab/grazel/gradle/dependencies/Dependencies.kt +++ b/grazel-gradle-plugin/src/main/kotlin/com/grab/grazel/gradle/dependencies/Dependencies.kt @@ -22,6 +22,7 @@ import com.grab.grazel.bazel.rules.ANNOTATION_ARTIFACT import com.grab.grazel.bazel.rules.DAGGER_GROUP import com.grab.grazel.bazel.rules.DATABINDING_GROUP import com.grab.grazel.bazel.starlark.BazelDependency +import com.grab.grazel.bazel.starlark.BazelDependency.MavenDependency import com.grab.grazel.bazel.starlark.BazelDependency.StringDependency import com.grab.grazel.gradle.ConfigurationDataSource import com.grab.grazel.gradle.ConfigurationScope @@ -66,8 +67,13 @@ internal data class MavenArtifact( val group: String?, val name: String?, val version: String? = null, -) { +) : Comparable { val id get() = "$group:$name" + + override fun compareTo(other: MavenArtifact): Int { + return toString().compareTo(other.toString()) + } + override fun toString() = "$group:$name:$version" } @@ -121,6 +127,15 @@ internal interface DependenciesDataSource { project: Project, buildGraphType: BuildGraphType ): List + + /** + * Collects all transitive maven dependencies for the given [buildGraphType]. Similar to + * [collectMavenDeps] + */ + fun collectTransitiveMavenDeps( + project: Project, + buildGraphType: BuildGraphType + ): Set } @Singleton @@ -208,23 +223,7 @@ internal class DefaultDependenciesDataSource @Inject constructor( project: Project, buildGraphType: BuildGraphType ): List { - val variants = variantBuilder.build(project).groupBy(Variant<*>::name) - val inputVariant = buildGraphType.variant - // From input variant map to Grazel variant - val grazelVariant: Variant<*> = when { - inputVariant != null -> variants[inputVariant.name]!!.first { - it.variantType.isConfigScope(project, buildGraphType.configurationScope) - } - // Input variant is null, probably legacy code path assumes non android project has no - // variants. To compensate, map it to `default` or `test` based on - // BuildGraphType.configurationScope - // This will no longer needed after migrating legacy code path to use variants - else -> when (buildGraphType.configurationScope) { - TEST -> variants[TEST_VARIANT]!!.first() - else -> variants[DEFAULT_VARIANT]!!.first() - } - } - + val grazelVariant: Variant<*> = findGrazelVariant(project, buildGraphType) return grazelVariant.migratableConfigurations .asSequence() .flatMap { it.allDependencies.filterIsInstance() } @@ -245,8 +244,8 @@ internal class DefaultDependenciesDataSource @Inject constructor( DAGGER_GROUP -> StringDependency("//:dagger") else -> dependencyResolutionService.get().get( variants = variantHierarchy, - group = dependency.group, - name = dependency.name + group = dependency.group!!, + name = dependency.name!! ) ?: run { error("$dependency cant be found for migrating ${project.name}") } @@ -255,6 +254,53 @@ internal class DefaultDependenciesDataSource @Inject constructor( .toList() } + override fun collectTransitiveMavenDeps( + project: Project, + buildGraphType: BuildGraphType + ): Set { + val grazelVariant: Variant<*> = findGrazelVariant(project, buildGraphType) + return grazelVariant.migratableConfigurations + .asSequence() + .map { it.incoming.resolutionResult.root } + .flatMapTo(TreeSet()) { rootNode -> + // Using ResolvedComponentsVisitor is redundant here since the work would + // already be done by the time we reach here by resolveVariantDependenciesTask. + // But for simplicity we visit components again. + // TODO(arun) Optimize this and read resolved components as Task input. + ResolvedComponentsVisitor().visit(rootNode) { (component, _, _, _) -> + val version = component.moduleVersion!! + MavenArtifact( + group = version.group, + name = version.name, + ) + } + }.map { MavenDependency(group = it.group!!, name = it.name!!) } + .toSortedSet() + } + + private fun findGrazelVariant( + project: Project, + buildGraphType: BuildGraphType + ): Variant<*> { + val variants = variantBuilder.build(project).groupBy(Variant<*>::name) + val inputVariant = buildGraphType.variant + // From input variant map to Grazel variant + val grazelVariant: Variant<*> = when { + inputVariant != null -> variants[inputVariant.name]!!.first { + it.variantType.isConfigScope(project, buildGraphType.configurationScope) + } + // Input variant is null, probably legacy code path assumes non android project has no + // variants. To compensate, map it to `default` or `test` based on + // BuildGraphType.configurationScope + // This will no longer needed after migrating legacy code path to use variants + else -> when (buildGraphType.configurationScope) { + TEST -> variants[TEST_VARIANT]!!.first() + else -> variants[DEFAULT_VARIANT]!!.first() + } + } + return grazelVariant + } + /** * Collects first level module dependencies from their resolved configuration. Additionally, excludes any artifacts * that are not meant to be used in Bazel as defined by [IGNORED_ARTIFACT_GROUPS] diff --git a/grazel-gradle-plugin/src/main/kotlin/com/grab/grazel/migrate/android/AndroidExtractor.kt b/grazel-gradle-plugin/src/main/kotlin/com/grab/grazel/migrate/android/AndroidExtractor.kt index 56bbd6dc..33bc4d1f 100644 --- a/grazel-gradle-plugin/src/main/kotlin/com/grab/grazel/migrate/android/AndroidExtractor.kt +++ b/grazel-gradle-plugin/src/main/kotlin/com/grab/grazel/migrate/android/AndroidExtractor.kt @@ -123,7 +123,14 @@ constructor( ?.let(::relativePath) val tags = if (grazelExtension.rules.kotlin.enabledTransitiveReduction) { - deps.calculateDirectDependencyTags(name) + val transitiveMavenDeps = dependenciesDataSource.collectTransitiveMavenDeps( + project = project, + buildGraphType = BuildGraphType(BUILD, matchedVariant.variant) + ) + calculateDirectDependencyTags( + self = name, + deps = deps + transitiveMavenDeps + ) } else emptyList() val lintConfigs = lintConfigs(extension.lintOptions, project) diff --git a/grazel-gradle-plugin/src/main/kotlin/com/grab/grazel/migrate/android/AndroidUnitTestDataExtractor.kt b/grazel-gradle-plugin/src/main/kotlin/com/grab/grazel/migrate/android/AndroidUnitTestDataExtractor.kt index 7a4c8b6b..c9840cc4 100644 --- a/grazel-gradle-plugin/src/main/kotlin/com/grab/grazel/migrate/android/AndroidUnitTestDataExtractor.kt +++ b/grazel-gradle-plugin/src/main/kotlin/com/grab/grazel/migrate/android/AndroidUnitTestDataExtractor.kt @@ -91,7 +91,11 @@ internal class DefaultAndroidUnitTestDataExtractor @Inject constructor( ) val tags = if (kotlinExtension.enabledTransitiveReduction) { - deps.calculateDirectDependencyTags(name) + val transitiveMavenDeps = dependenciesDataSource.collectTransitiveMavenDeps( + project = project, + buildGraphType = BuildGraphType(ConfigurationScope.TEST, matchedVariant.variant) + ) + calculateDirectDependencyTags(name, deps + transitiveMavenDeps) } else emptyList() return AndroidUnitTestData( diff --git a/grazel-gradle-plugin/src/main/kotlin/com/grab/grazel/migrate/dependencies/ClasspathReduction.kt b/grazel-gradle-plugin/src/main/kotlin/com/grab/grazel/migrate/dependencies/ClasspathReduction.kt index 67cf9636..a861ce93 100644 --- a/grazel-gradle-plugin/src/main/kotlin/com/grab/grazel/migrate/dependencies/ClasspathReduction.kt +++ b/grazel-gradle-plugin/src/main/kotlin/com/grab/grazel/migrate/dependencies/ClasspathReduction.kt @@ -17,10 +17,18 @@ package com.grab.grazel.migrate.dependencies import com.grab.grazel.bazel.starlark.BazelDependency +import com.grab.grazel.bazel.starlark.BazelDependency.MavenDependency +import com.grab.grazel.bazel.starlark.BazelDependency.ProjectDependency -fun List.calculateDirectDependencyTags(self: String) = asSequence() - .filterIsInstance() - .map { "@direct${it}" } - .toMutableList() +fun calculateDirectDependencyTags( + self: String, + deps: List +) = deps.asSequence().mapNotNull { + when (it) { + is ProjectDependency -> "@direct${it}" + is MavenDependency -> it.copy(repo = "maven").toString() + else -> null + } +}.toMutableList() .also { it.add("@self//$self") } .sorted() \ No newline at end of file diff --git a/grazel-gradle-plugin/src/main/kotlin/com/grab/grazel/migrate/kotlin/KotlinProjectDataExtractor.kt b/grazel-gradle-plugin/src/main/kotlin/com/grab/grazel/migrate/kotlin/KotlinProjectDataExtractor.kt index 34126cf9..43a4ecfc 100644 --- a/grazel-gradle-plugin/src/main/kotlin/com/grab/grazel/migrate/kotlin/KotlinProjectDataExtractor.kt +++ b/grazel-gradle-plugin/src/main/kotlin/com/grab/grazel/migrate/kotlin/KotlinProjectDataExtractor.kt @@ -72,7 +72,11 @@ internal class DefaultKotlinProjectDataExtractor ) + project.androidJarDeps() + project.kotlinParcelizeDeps() val tags = if (kotlinExtension.enabledTransitiveReduction) { - deps.calculateDirectDependencyTags(self = name) + val transitiveMavenDeps = dependenciesDataSource.collectTransitiveMavenDeps( + project = project, + buildGraphType = BuildGraphType(ConfigurationScope.BUILD) + ) + calculateDirectDependencyTags(self = name, deps = deps + transitiveMavenDeps) } else emptyList() return KotlinProjectData( diff --git a/grazel-gradle-plugin/src/main/kotlin/com/grab/grazel/migrate/kotlin/KotlinUnitTestDataExtractor.kt b/grazel-gradle-plugin/src/main/kotlin/com/grab/grazel/migrate/kotlin/KotlinUnitTestDataExtractor.kt index a072eb2d..6e711728 100644 --- a/grazel-gradle-plugin/src/main/kotlin/com/grab/grazel/migrate/kotlin/KotlinUnitTestDataExtractor.kt +++ b/grazel-gradle-plugin/src/main/kotlin/com/grab/grazel/migrate/kotlin/KotlinUnitTestDataExtractor.kt @@ -88,7 +88,11 @@ internal class DefaultKotlinUnitTestDataExtractor @Inject constructor( } val tags = if (kotlinExtension.enabledTransitiveReduction) { - deps.calculateDirectDependencyTags(name) + val transitiveMavenDeps = dependenciesDataSource.collectTransitiveMavenDeps( + project = project, + buildGraphType = BuildGraphType(ConfigurationScope.TEST) + ) + calculateDirectDependencyTags(name, deps + transitiveMavenDeps) } else emptyList() return UnitTestData( diff --git a/grazel-gradle-plugin/src/test/kotlin/com/grab/grazel/migrate/dependencies/ClasspathReductionTest.kt b/grazel-gradle-plugin/src/test/kotlin/com/grab/grazel/migrate/dependencies/ClasspathReductionTest.kt new file mode 100644 index 00000000..f3cc5cdf --- /dev/null +++ b/grazel-gradle-plugin/src/test/kotlin/com/grab/grazel/migrate/dependencies/ClasspathReductionTest.kt @@ -0,0 +1,58 @@ +package com.grab.grazel.migrate.dependencies + +import com.grab.grazel.bazel.starlark.BazelDependency +import com.grab.grazel.bazel.starlark.BazelDependency.MavenDependency +import com.grab.grazel.bazel.starlark.BazelDependency.ProjectDependency +import com.grab.grazel.buildProject +import com.grab.grazel.util.truth +import org.junit.Test + +class ClasspathReductionTest { + @Test + fun `test with project dependencies`() { + val rootProject = buildProject("root") + val subproject = buildProject("sub", parent = rootProject) + val deps: List = listOf( + ProjectDependency(dependencyProject = rootProject), + ProjectDependency(dependencyProject = subproject), + ) + calculateDirectDependencyTags("self", deps).truth { + containsExactly( + "@direct//root:root", + "@direct//sub:sub", + "@self//self" + ) + } + } + + @Test + fun `test with maven dependencies`() { + val deps: List = listOf( + MavenDependency(group = "com.example", name = "lib1"), + MavenDependency(group = "org.test", name = "lib2", repo = "custom_repo") + ) + calculateDirectDependencyTags("self", deps).truth { + containsExactly( + "@maven//:com_example_lib1", + "@maven//:org_test_lib2", + "@self//self" + ) + } + } + + @Test + fun `test with mixed dependencies`() { + val rootProject = buildProject("root") + val deps: List = listOf( + ProjectDependency(dependencyProject = rootProject), + MavenDependency(group = "com.example", name = "lib1") + ) + calculateDirectDependencyTags("self", deps).truth { + containsExactly( + "@direct//root:root", + "@maven//:com_example_lib1", + "@self//self", + ) + } + } +} \ No newline at end of file