Implement direct dependency tags for maven external targets

grab · Oct 1, 2024 · 14f324d · 14f324d
1 parent 9d7c55b
commit 14f324d
Show file tree

Hide file tree

Showing 9 changed files with 170 additions and 35 deletions.
diff --git 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
@@ -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<MavenArtifact> {
     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<BazelDependency>
+
+    /**
+     * Collects all transitive maven dependencies for the given [buildGraphType]. Similar to
+     * [collectMavenDeps]
+     */
+    fun collectTransitiveMavenDeps(
+        project: Project,
+        buildGraphType: BuildGraphType
+    ): Set<MavenDependency>
 }
 
 @Singleton
@@ -208,23 +223,7 @@ internal class DefaultDependenciesDataSource @Inject constructor(
         project: Project,
         buildGraphType: BuildGraphType
     ): List<BazelDependency> {
-        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<ExternalDependency>() }
@@ -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<MavenDependency> {
+        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
@@ -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/...le-plugin/src/main/kotlin/com/grab/grazel/migrate/android/AndroidUnitTestDataExtractor.kt b/...le-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/...-gradle-plugin/src/main/kotlin/com/grab/grazel/migrate/dependencies/ClasspathReduction.kt b/...-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<BazelDependency>.calculateDirectDependencyTags(self: String) = asSequence()
-    .filterIsInstance<BazelDependency.ProjectDependency>()
-    .map { "@direct${it}" }
-    .toMutableList()
+fun calculateDirectDependencyTags(
+    self: String,
+    deps: List<BazelDependency>
+) = 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()
diff --git a/...radle-plugin/src/main/kotlin/com/grab/grazel/migrate/kotlin/KotlinProjectDataExtractor.kt b/...radle-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/...adle-plugin/src/main/kotlin/com/grab/grazel/migrate/kotlin/KotlinUnitTestDataExtractor.kt b/...adle-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/...dle-plugin/src/test/kotlin/com/grab/grazel/migrate/dependencies/ClasspathReductionTest.kt b/...dle-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<ProjectDependency> = 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<BazelDependency> = 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<BazelDependency> = listOf(
+            ProjectDependency(dependencyProject = rootProject),
+            MavenDependency(group = "com.example", name = "lib1")
+        )
+        calculateDirectDependencyTags("self", deps).truth {
+            containsExactly(
+                "@direct//root:root",
+                "@maven//:com_example_lib1",
+                "@self//self",
+            )
+        }
+    }
+}
diff --git a/keystore/BUILD.bazel b/keystore/BUILD.bazel
@@ -1,9 +1,10 @@
 filegroup(
-    name = "debug-keystore",
-    srcs = [
-        "debug.keystore",
-    ],
-    visibility = [
-        "//visibility:public",
-    ],
+  name = "debug-keystore",
+  srcs = [
+    "debug.keystore",
+  ],
+  visibility = [
+    "//visibility:public",
+  ]
 )
+