Fix resource sets not handling manifests correctly (#142)

Fixes #142
grab · Nov 20, 2024 · 0f9124e · 0f9124e
1 parent 8d14323
commit 0f9124e
Show file tree

Hide file tree

Showing 12 changed files with 143 additions and 42 deletions.
diff --git a/WORKSPACE b/WORKSPACE
@@ -27,7 +27,7 @@ load("@bazel_tools//tools/build_defs/repo:git.bzl", "git_repository")
 
 git_repository(
     name = "grab_bazel_common",
-    commit = "93a0875d12a7e223221390505b04deff76ea3845",
+    commit = "9b0439127d50741c2f19d8df9932d6338f763dab",
     remote = "https://github.com/grab/grab-bazel-common.git",
 )
 
@@ -400,6 +400,7 @@ maven_install(
         "android.arch.lifecycle:livedata-core",
         "android.arch.lifecycle:runtime",
         "android.arch.lifecycle:viewmodel",
+        "androidx.fragment:fragment",
         "com.android.databinding:baseLibrary",
         "com.android.support:animated-vector-drawable",
         "com.android.support:cardview-v7",
@@ -413,7 +414,6 @@ maven_install(
         "com.android.support:support-compat",
         "com.android.support:support-core-ui",
         "com.android.support:support-core-utils",
-        "com.android.support:support-fragment",
         "com.android.support:support-vector-drawable",
         "com.android.support:versionedparcelable",
         "com.android.support:viewpager",

diff --git a/build.gradle b/build.gradle
@@ -68,7 +68,7 @@ grazel {
     rules {
         bazelCommon {
             gitRepository {
-                commit = "93a0875d12a7e223221390505b04deff76ea3845"
+                commit = "9b0439127d50741c2f19d8df9932d6338f763dab"
                 remote = "https://github.com/grab/grab-bazel-common.git"
             }
             toolchains {

diff --git a/constants.gradle b/constants.gradle
@@ -15,7 +15,7 @@
  */
 ext {
     groupId = "com.grab.grazel"
-    versionName = project.hasProperty("versionName") ? versionName : "0.4.2-alpha.02"
+    versionName = project.hasProperty("versionName") ? versionName : "0.4.2-alpha.03"
 
     website = "https://grab.github.io/Grazel/"
 }
diff --git a/flavors/sample-android-flavor/BUILD.bazel b/flavors/sample-android-flavor/BUILD.bazel
@@ -28,7 +28,6 @@ android_library(
         },
         "main": {
             "res": "src/main/res",
-            "assets": "src/main/assets",
             "manifest": "src/main/AndroidManifest.xml",
         },
     },
@@ -71,7 +70,6 @@ android_library(
         },
         "main": {
             "res": "src/main/res",
-            "assets": "src/main/assets",
             "manifest": "src/main/AndroidManifest.xml",
         },
     },
@@ -117,7 +115,6 @@ android_library(
         },
         "main": {
             "res": "src/main/res",
-            "assets": "src/main/assets",
             "manifest": "src/main/AndroidManifest.xml",
         },
     },
@@ -160,7 +157,6 @@ android_library(
         },
         "main": {
             "res": "src/main/res",
-            "assets": "src/main/assets",
             "manifest": "src/main/AndroidManifest.xml",
         },
     },

diff --git a/grazel-gradle-plugin/src/main/kotlin/com/grab/grazel/migrate/android/AndroidData.kt b/grazel-gradle-plugin/src/main/kotlin/com/grab/grazel/migrate/android/AndroidData.kt
@@ -24,7 +24,10 @@ internal data class BazelSourceSet(
     val res: String?,
     val assets: String?,
     val manifest: String?,
-)
+) {
+    val isEmpty: Boolean = res == null && assets == null && manifest == null
+    val hasResources: Boolean = res != null || assets != null
+}
 
 internal interface AndroidData {
     val name: String

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
@@ -112,8 +112,14 @@ constructor(
         val srcs = androidSources(migratableSourceSets, JAVA_KOTLIN).toList()
 
         val resourceSets = migratableSourceSets.flatMap { it.toResourceSet(project) }
+            .filterNot(BazelSourceSet::isEmpty)
             .reversed()
             .toSet()
+            .let { resourcesSets ->
+                if (resourcesSets.size == 1 && resourcesSets.all { !it.hasResources }) {
+                    emptySet()
+                } else resourcesSets
+            }
 
         val manifestFile = androidManifestParser
             .androidManifestFile(migratableSourceSets)

diff --git a/grazel-gradle-plugin/src/main/kotlin/com/grab/grazel/migrate/android/SourceSet.kt b/grazel-gradle-plugin/src/main/kotlin/com/grab/grazel/migrate/android/SourceSet.kt
@@ -61,19 +61,18 @@ enum class PathResolveMode {
 internal fun AndroidSourceSet.toResourceSet(
     project: Project
 ): Set<BazelSourceSet> {
-    val manifestPath = manifest.srcFile.takeIf { it.exists() }?.let(project::relativePath)
-
     fun File.isValid() = exists() && walk().drop(1).any()
+    val manifestPath = manifest.srcFile.takeIf { it.exists() }?.let(project::relativePath)
     val resources = res.srcDirs.filter(File::isValid)
     val assets = assets.srcDirs.filter(File::isValid)
 
-    return if (resources.size == 1 && assets.size == 1) {
+    return if (resources.size <= 1 && assets.size <= 1) {
         // Happy path, most modules would be like this with one single res and assets dir.
         setOf(
             BazelSourceSet(
                 name = name,
-                res = project.relativePath(resources.first()),
-                assets = project.relativePath(assets.first()),
+                res = resources.firstOrNull()?.let(project::relativePath),
+                assets = assets.firstOrNull()?.let(project::relativePath),
                 manifest = manifestPath
             )
         )
@@ -82,6 +81,8 @@ internal fun AndroidSourceSet.toResourceSet(
         // set dir for Bazel.
         return LinkedHashSet<BazelSourceSet>().apply {
             resources.mapIndexedTo(this) { index, resDir ->
+                // No need to duplicate manifest across res directories for same source set, assign to
+                // first entry alone
                 val sourceSetManifest = if (index == 0) manifestPath else null
                 BazelSourceSet(
                     name = name,
@@ -91,6 +92,8 @@ internal fun AndroidSourceSet.toResourceSet(
                 )
             }
             assets.mapIndexedTo(this) { index, assets ->
+                // No need to duplicate manifest across res directories for same source set, assign to
+                // first entry alone
                 val sourceSetManifest = if (index == 0) manifestPath else null
                 BazelSourceSet(
                     name = name,

diff --git a/...src/test/kotlin/com/grab/grazel/migrate/android/DefaultAndroidLibraryDataExtractorTest.kt b/...src/test/kotlin/com/grab/grazel/migrate/android/DefaultAndroidLibraryDataExtractorTest.kt
@@ -22,6 +22,7 @@ import org.gradle.kotlin.dsl.dependencies
 import org.gradle.kotlin.dsl.the
 import org.junit.Test
 import java.nio.file.Files
+import kotlin.io.path.writeText
 import kotlin.test.assertEquals
 
 class DefaultAndroidLibraryDataExtractorTest {
@@ -53,6 +54,9 @@ class DefaultAndroidLibraryDataExtractorTest {
                     compileSdkVersion(32)
                 }
                 app(this)
+                sourceSets.named("main").configure {
+                    res.srcDirs("src/main/res-extra")
+                }
             }
             dependencies {
                 add("implementation", libraryProject)
@@ -93,13 +97,21 @@ class DefaultAndroidLibraryDataExtractorTest {
     }
 
     private fun createSources() {
-        val resMain = appProject.file("src/main/res")
-        resMain.toPath().let {
-            Files.createDirectories(it)
-            val values = it.resolve("values")
-            Files.createDirectories(values)
-            values.resolve("values.xml").toFile().writeText("")
-        }
+        appProject.file("src/main/res/values")
+            .toPath()
+            .also(Files::createDirectories)
+            .resolve("values.xml")
+            .writeText("")
+        appProject.file("src/debug")
+            .toPath()
+            .also(Files::createDirectories)
+            .resolve("AndroidManifest.xml")
+            .writeText("<manifest package=\"grazel\" />")
+        appProject.file("src/main/res-extra/values")
+            .toPath()
+            .also(Files::createDirectories)
+            .resolve("values.xml")
+            .writeText("")
     }
 
     private fun debugVariant(): MatchedVariant {
@@ -142,13 +154,25 @@ class DefaultAndroidLibraryDataExtractorTest {
     }
 
     @Test
-    fun `assert resource sets are calulcated for android binary only when file exists`() {
+    fun `assert resource sets are calculated correctly for variants`() {
         configure()
         val resourceSets = androidLibraryDataExtractor
             .extract(appProject, debugVariant())
             .resourceSets
         resourceSets.truth {
             containsExactly(
+                BazelSourceSet(
+                    name = "debug",
+                    res = null,
+                    assets = null,
+                    manifest = "src/debug/AndroidManifest.xml"
+                ),
+                BazelSourceSet(
+                    name = "main",
+                    res = "src/main/res-extra",
+                    assets = null,
+                    manifest = null,
+                ),
                 BazelSourceSet(
                     name = "main",
                     res = "src/main/res",
@@ -157,7 +181,7 @@ class DefaultAndroidLibraryDataExtractorTest {
                 )
             )
             containsNoDuplicates()
-            hasSize(1)
+            hasSize(3)
         }
     }
 }
diff --git a/sample-android-library/BUILD.bazel b/sample-android-library/BUILD.bazel
@@ -4,14 +4,23 @@ android_library(
     name = "sample-android-library-demo-free-debug",
     srcs = glob([
         "src/main/java/com/grab/grazel/android/sample/SampleViewModel.kt",
+        "src/debug/kotlin/Empty.kt",
     ]),
     custom_package = "com.grab.grazel.android.sample.lib",
     enable_data_binding = True,
     lint_options = {
         "enabled": True,
         "config": "//:lint.xml",
     },
-    manifest = "src/main/AndroidManifest.xml",
+    manifest = "src/debug/AndroidManifest.xml",
+    resource_sets = {
+        "debug": {
+            "manifest": "src/debug/AndroidManifest.xml",
+        },
+        "main": {
+            "manifest": "src/main/AndroidManifest.xml",
+        },
+    },
     visibility = [
         "//visibility:public",
     ],
@@ -24,14 +33,23 @@ android_library(
     name = "sample-android-library-demo-paid-debug",
     srcs = glob([
         "src/main/java/com/grab/grazel/android/sample/SampleViewModel.kt",
+        "src/debug/kotlin/Empty.kt",
     ]),
     custom_package = "com.grab.grazel.android.sample.lib",
     enable_data_binding = True,
     lint_options = {
         "enabled": True,
         "config": "//:lint.xml",
     },
-    manifest = "src/main/AndroidManifest.xml",
+    manifest = "src/debug/AndroidManifest.xml",
+    resource_sets = {
+        "debug": {
+            "manifest": "src/debug/AndroidManifest.xml",
+        },
+        "main": {
+            "manifest": "src/main/AndroidManifest.xml",
+        },
+    },
     visibility = [
         "//visibility:public",
     ],
@@ -44,14 +62,23 @@ android_library(
     name = "sample-android-library-full-free-debug",
     srcs = glob([
         "src/main/java/com/grab/grazel/android/sample/SampleViewModel.kt",
+        "src/debug/kotlin/Empty.kt",
     ]),
     custom_package = "com.grab.grazel.android.sample.lib",
     enable_data_binding = True,
     lint_options = {
         "enabled": True,
         "config": "//:lint.xml",
     },
-    manifest = "src/main/AndroidManifest.xml",
+    manifest = "src/debug/AndroidManifest.xml",
+    resource_sets = {
+        "debug": {
+            "manifest": "src/debug/AndroidManifest.xml",
+        },
+        "main": {
+            "manifest": "src/main/AndroidManifest.xml",
+        },
+    },
     visibility = [
         "//visibility:public",
     ],
@@ -64,14 +91,23 @@ android_library(
     name = "sample-android-library-full-paid-debug",
     srcs = glob([
         "src/main/java/com/grab/grazel/android/sample/SampleViewModel.kt",
+        "src/debug/kotlin/Empty.kt",
     ]),
     custom_package = "com.grab.grazel.android.sample.lib",
     enable_data_binding = True,
     lint_options = {
         "enabled": True,
         "config": "//:lint.xml",
     },
-    manifest = "src/main/AndroidManifest.xml",
+    manifest = "src/debug/AndroidManifest.xml",
+    resource_sets = {
+        "debug": {
+            "manifest": "src/debug/AndroidManifest.xml",
+        },
+        "main": {
+            "manifest": "src/main/AndroidManifest.xml",
+        },
+    },
     visibility = [
         "//visibility:public",
     ],

diff --git a/sample-android-library/src/debug/AndroidManifest.xml b/sample-android-library/src/debug/AndroidManifest.xml
@@ -0,0 +1,17 @@
+<?xml version="1.0" encoding="utf-8"?><!--
+  ~ Copyright 2022 Grabtaxi Holdings PTE LTD (GRAB)
+  ~
+  ~ Licensed under the Apache License, Version 2.0 (the "License");
+  ~ you may not use this file except in compliance with the License.
+  ~ You may obtain a copy of the License at
+  ~
+  ~     http://www.apache.org/licenses/LICENSE-2.0
+  ~
+  ~ Unless required by applicable law or agreed to in writing, software
+  ~ distributed under the License is distributed on an "AS IS" BASIS,
+  ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  ~ See the License for the specific language governing permissions and
+  ~ limitations under the License.
+  -->
+
+<manifest package="com.grab.grazel.android.sample.lib" />
diff --git a/sample-android-library/src/debug/kotlin/Empty.kt b/sample-android-library/src/debug/kotlin/Empty.kt
@@ -0,0 +1,16 @@
+/*
+ * Copyright 2024 Grabtaxi Holdings PTE LTD (GRAB)
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+