fix(bgp): merge res/values of embedded libraries instead of overwriting

.changeset/aar-merge-res-values.md

@@ -0,0 +1,5 @@
+---
+'@callstack/react-native-brownfield': patch
+---
+
+fix: keep the resources of every library embedded in the Android AAR; previously only one library's `res/values` survived and host apps failed at resource linking. As in a regular app, the module's own `res` and `resValue`s win over a library's, and when two libraries define the same resource the first one wins (previously the last one won for files such as drawables)

apps/RNApp/android/build.gradle

@@ -16,7 +16,7 @@ buildscript {
         classpath("com.android.tools.build:gradle")
         classpath("com.facebook.react:react-native-gradle-plugin")
         classpath("org.jetbrains.kotlin:kotlin-gradle-plugin")
-        classpath("com.callstack.react:brownfield-gradle-plugin:2.0.0-alpha11-SNAPSHOT")
+        classpath("com.callstack.react:brownfield-gradle-plugin:2.0.0-alpha13-SNAPSHOT")
     }
 }
 

apps/scripts/prepare-android-build-gradle-for-ci.ts

@@ -9,7 +9,7 @@ if (!projectDirName) {
 
 const __filename = fileURLToPath(import.meta.url);
 const __dirname = path.dirname(__filename);
-const SNAPSHOT_VERSION = '2.0.0-alpha11-SNAPSHOT';
+const SNAPSHOT_VERSION = '2.0.0-alpha13-SNAPSHOT';
 // Accepts a repo-relative app dir (e.g. "ExpoApp58") or an absolute path (e.g.
 // the scaffold-smoke app created outside the repo).
 const projectDir = path.isAbsolute(projectDirName)

gradle-plugins/react/brownfield/build.gradle.kts

@@ -113,10 +113,14 @@ dependencies {
     // minimum Gradle version) for every app that applies this plugin.
     compileOnly(libs.agp)
     compileOnly(libs.common)
+    compileOnly(libs.sdk.common)
+    compileOnly(libs.layoutlib.api)
     implementation(libs.asm.commons)
     implementation(libs.versioncompare)
     testImplementation(libs.agp)
     testImplementation(libs.common)
+    testImplementation(libs.sdk.common)
+    testImplementation(libs.layoutlib.api)
     testImplementation(libs.junit.jupiter)
     testImplementation(gradleTestKit())
     testImplementation(kotlin("test"))

gradle-plugins/react/brownfield/gradle.properties

@@ -1,6 +1,6 @@
 PROJECT_ID=com.callstack.react.brownfield
 ARTIFACT_ID=brownfield-gradle-plugin
-VERSION=2.0.0-alpha11
+VERSION=2.0.0-alpha13
 GROUP=com.callstack.react
 IMPLEMENTATION_CLASS=com.callstack.react.brownfield.plugin.RNBrownfieldPlugin
 

gradle-plugins/react/brownfield/gradle/libs.versions.toml

@@ -19,6 +19,8 @@ detekt = { id = "io.gitlab.arturbosch.detekt", version.ref = "detekt" }
 [libraries]
 agp = { module = "com.android.tools.build:gradle", name = "agp", version.ref = "agp" }
 common = { module = "com.android.tools:common", version.ref = "common" }
+sdk-common = { module = "com.android.tools:sdk-common", version.ref = "common" }
+layoutlib-api = { module = "com.android.tools.layoutlib:layoutlib-api", version.ref = "common" }
 asm-commons = { module = "org.ow2.asm:asm-commons", version.ref = "asm-commons" }
 versioncompare = { module = "io.github.g00fy2:versioncompare", version.ref = "versioncompare" }
 junit-jupiter = { module = "org.junit.jupiter:junit-jupiter", version.ref = "junit" }

gradle-plugins/react/brownfield/src/main/kotlin/com/callstack/react/brownfield/processors/ResourceTaskProcessor.kt

@@ -1,44 +1,206 @@
 package com.callstack.react.brownfield.processors
 
+import com.android.build.api.dsl.LibraryExtension
+import com.android.build.api.variant.LibraryAndroidComponentsExtension
 import com.android.build.api.variant.LibraryVariant
+import com.android.ide.common.rendering.api.ResourceNamespace
+import com.android.ide.common.resources.MergeConsumer
+import com.android.ide.common.resources.ResourceMerger
+import com.android.ide.common.resources.ResourceMergerItem
+import com.android.ide.common.resources.ResourceSet
 import com.callstack.react.brownfield.shared.ExplodeAarTask
+import com.callstack.react.brownfield.shared.GradleILogger
 import com.callstack.react.brownfield.utils.AndroidArchiveLibrary
 import com.callstack.react.brownfield.utils.capitalized
 import org.gradle.api.DefaultTask
 import org.gradle.api.Project
 import org.gradle.api.file.ConfigurableFileCollection
 import org.gradle.api.file.DirectoryProperty
+import org.gradle.api.provider.ListProperty
 import org.gradle.api.tasks.CacheableTask
+import org.gradle.api.tasks.Input
 import org.gradle.api.tasks.InputFiles
 import org.gradle.api.tasks.OutputDirectory
 import org.gradle.api.tasks.PathSensitive
 import org.gradle.api.tasks.PathSensitivity
 import org.gradle.api.tasks.TaskAction
 import org.gradle.api.tasks.TaskProvider
+import org.w3c.dom.Element
+import org.w3c.dom.Node
+import java.io.File
+import javax.xml.parsers.DocumentBuilderFactory
+import javax.xml.transform.TransformerFactory
+import javax.xml.transform.dom.DOMSource
+import javax.xml.transform.stream.StreamResult
 
 @CacheableTask
 abstract class CopyExplodedResTask : DefaultTask() {
     @get:InputFiles
     @get:PathSensitive(PathSensitivity.RELATIVE)
     abstract val inputResDirs: ConfigurableFileCollection
 
+    /** The module's own resources, which win over every library's, as an app's do over its dependencies. */
+    @get:InputFiles
+    @get:PathSensitive(PathSensitivity.RELATIVE)
+    abstract val moduleResDirs: ConfigurableFileCollection
+
+    /** `type/name` of the module's `resValue`s, which also win over every library's. */
+    @get:Input
+    abstract val moduleResValues: ListProperty<String>
+
+    /** The first library wins on a duplicate, so the order is part of the input. */
+    @get:Input
+    abstract val libraryOrder: ListProperty<String>
+
     @get:OutputDirectory
     abstract val outputDirectory: DirectoryProperty
 
+    /**
+     * Every exploded AAR keeps its values in `res/values/values.xml`, so libraries can't simply be
+     * copied over each other (#490), and keeping a resource twice in one dir fails the build (#125).
+     * AGP's own merger picks the winners, with the precedence it uses in an app: the module's own
+     * resources, then the first library.
+     */
     @TaskAction
     fun run() {
         val out = outputDirectory.get().asFile
         out.deleteRecursively()
         out.mkdirs()
-        inputResDirs.forEach { resDir ->
-            if (resDir.exists()) {
-                resDir.copyRecursively(out, overwrite = true)
-            }
+        val iLogger = GradleILogger(logger)
+        val moduleDirs = moduleResDirs.filter { it.exists() }.toList()
+        // 0 keeps every versioned resource. AGP applies the module's minSdk when it merges this dir.
+        val merger = ResourceMerger(0)
+        // Lowest priority first.
+        inputResDirs.filter { it.exists() }.reversed().forEachIndexed { index, resDir ->
+            merger.addDataSet(resourceSet("library$index", resDir, isLibrary = true, iLogger))
+        }
+        // One set per dir: two module dirs may override each other, like main and a flavor.
+        moduleDirs.forEachIndexed { index, resDir ->
+            merger.addDataSet(resourceSet("module$index", resDir, isLibrary = false, iLogger))
+        }
+        val writer = LibraryResWriter(moduleDirs, moduleResValues.get().toSet())
+        merger.mergeData(writer, false)
+        writer.messages.forEach { logger.warn("Brownfield: $it") }
+        writer.writeTo(out)
+    }
+
+    private fun resourceSet(
+        name: String,
+        resDir: File,
+        isLibrary: Boolean,
+        iLogger: GradleILogger,
+    ) = ResourceSet(name, ResourceNamespace.RES_AUTO, if (isLibrary) name else null, false, null).apply {
+        addSource(resDir)
+        loadFromFiles(iLogger)
+    }
+}
+
+/**
+ * Writes the merged library resources that the module doesn't define itself. AGP ranks this generated
+ * dir above the module's own res, so a library copy of a module resource would win there.
+ */
+private class LibraryResWriter(
+    private val moduleDirs: List<File>,
+    private val moduleResValues: Set<String>,
+) : MergeConsumer<ResourceMergerItem> {
+    private val files = mutableListOf<Pair<String, File>>()
+    private val values = sortedMapOf<String, MutableList<Node>>()
+    private val moduleAttrs = mutableMapOf<String, MutableSet<String>>()
+    private val styleableAttrWinners = mutableMapOf<String, MutableList<Node>>()
+    val messages = mutableListOf<String>()
+
+    override fun start(factory: DocumentBuilderFactory) = Unit
+
+    override fun end() = Unit
+
+    override fun removeItem(
+        removedItem: ResourceMergerItem,
+        replacedBy: ResourceMergerItem?,
+    ) = Unit
+
+    override fun ignoreItemInMerge(item: ResourceMergerItem) = false
+
+    override fun addItem(item: ResourceMergerItem) {
+        val folder = valuesFolder(item.qualifiers)
+        val source = item.sourceFile?.file
+        if (source != null && moduleDirs.any { source.startsWith(it) }) {
+            if (item.type.getName() == "attr") moduleAttrs.getOrPut(folder) { mutableSetOf() }.add(item.name)
+            return
+        }
+        if (item.qualifiers.isEmpty() && "${item.type.getName()}/${item.name}" in moduleResValues) return
+        // An attr declared inside a styleable. The merger picked this one, so it becomes the definition.
+        if (item.ignoredFromDiskMerge) {
+            styleableAttrWinners.getOrPut(folder) { mutableListOf() }.add(item.value)
+            return
+        }
+        if (item.isFileBased && source != null) {
+            files.add("${source.parentFile.name}/${source.name}" to source)
+        } else {
+            values.getOrPut(folder) { mutableListOf() }.add(item.value)
         }
     }
+
+    fun writeTo(out: File) {
+        files.forEach { (path, file) -> file.copyTo(File(out, path)) }
+        val transformer = TransformerFactory.newInstance().newTransformer()
+        (values.keys + styleableAttrWinners.keys).toSortedSet().forEach { folder ->
+            val nodes = values[folder].orEmpty()
+            val document = DocumentBuilderFactory.newInstance().newDocumentBuilder().newDocument()
+            val root = document.createElement("resources")
+            document.appendChild(root)
+            nodes.forEach { root.appendChild(document.importNode(it, true)) }
+            styleableAttrWinners[folder].orEmpty().forEach { root.appendChild(document.importNode(it, true)) }
+            keepOneAttrDefinition(root, folder, moduleAttrs[folder].orEmpty())
+            val target = File(out, "$folder/$folder.xml")
+            target.parentFile.mkdirs()
+            transformer.transform(DOMSource(document), StreamResult(target))
+        }
+    }
+
+    /**
+     * AGP also registers an attr with a `format` (or enum/flag values) inside a styleable as its own
+     * `attr/<name>`, so one dir may define it only once. The merger's winner is written as a top-level
+     * attr, and every styleable copy, or one of an attr the module defines, becomes a plain reference.
+     */
+    private fun keepOneAttrDefinition(
+        root: Element,
+        folder: String,
+        moduleAttrs: Set<String>,
+    ) {
+        val defined = root.childElements().filter { it.tagName == "attr" }.associateBy { it.getAttribute("name") }
+        root.childElements()
+            .filter { it.tagName == "declare-styleable" }
+            .flatMap { it.childElements() }
+            .filter { it.tagName == "attr" && !it.getAttribute("name").startsWith("android:") }
+            .filter { definition(it).isNotEmpty() }
+            .forEach { attr ->
+                val name = attr.getAttribute("name")
+                val winner = defined[name]
+                if (winner == null && name !in moduleAttrs) return@forEach
+                if (winner != null && definition(winner) != definition(attr)) {
+                    messages.add("$folder: attr/$name is defined differently by two libraries, keeping the first")
+                }
+                attr.removeAttribute("format")
+                attr.childElements().forEach { attr.removeChild(it) }
+            }
+    }
+
+    /** Format and enum/flag values of an attr; empty for a plain reference. */
+    private fun definition(attr: Element): String =
+        attr.getAttribute("format") +
+            attr.childElements().joinToString { "${it.tagName}:${it.getAttribute("name")}=${it.getAttribute("value")}" }
+
+    private fun valuesFolder(qualifiers: String) = if (qualifiers.isEmpty()) "values" else "values-$qualifiers"
+
+    private fun Element.childElements(): List<Element> {
+        val children = childNodes
+        return (0 until children.length).map { children.item(it) }.filterIsInstance<Element>()
+    }
 }
 
 object ResourceTaskProcessor {
+    private const val RES_VALUES_OFF_BY_DEFAULT_SINCE_AGP = 9
+
     fun process(
         project: Project,
         variant: LibraryVariant,
@@ -47,12 +209,20 @@ object ResourceTaskProcessor {
     ) {
         val capitalized = variant.name.capitalized()
         val resDirs = aarLibraries.map { it.getResDir() }
+        val buildDir = project.layout.buildDirectory.get().asFile
         val copyResTask =
             project.tasks.register(
                 "copyExplodedResFor$capitalized",
                 CopyExplodedResTask::class.java,
             ) { task ->
                 task.inputResDirs.setFrom(resDirs)
+                if (resValuesEnabled(project)) {
+                    task.moduleResValues.set(variant.resValues.map { values -> values.keys.map { "${it.type}/${it.name}" } })
+                } else {
+                    task.moduleResValues.set(emptyList())
+                }
+                variant.sources.res?.static?.let { static -> task.moduleResDirs.from(static.map { layers -> layers.flatten() }) }
+                task.libraryOrder.set(aarLibraries.map { it.getExplodedAarRootDir().relativeTo(buildDir).invariantSeparatorsPath })
                 task.dependsOn(explodeTask)
             }
 
@@ -61,4 +231,12 @@ object ResourceTaskProcessor {
             CopyExplodedResTask::outputDirectory,
         )
     }
+
+    /** AGP warns on every read of `variant.resValues` while the feature is off, the default from AGP 9 on. */
+    private fun resValuesEnabled(project: Project): Boolean {
+        val explicit = project.extensions.getByType(LibraryExtension::class.java).buildFeatures.resValues
+        val default = project.providers.gradleProperty("android.defaults.buildfeatures.resvalues").orNull?.toBooleanStrictOrNull()
+        val agpMajor = project.extensions.getByType(LibraryAndroidComponentsExtension::class.java).pluginVersion.major
+        return explicit ?: default ?: (agpMajor < RES_VALUES_OFF_BY_DEFAULT_SINCE_AGP)
+    }
 }

gradle-plugins/react/brownfield/src/test/kotlin/com/callstack/react/brownfield/plugin/AgpCompatibilityMatrixTest.kt

@@ -7,6 +7,7 @@ import org.junit.jupiter.api.io.TempDir
 import org.junit.jupiter.params.ParameterizedTest
 import org.junit.jupiter.params.provider.MethodSource
 import java.io.File
+import kotlin.test.assertFalse
 import kotlin.test.assertTrue
 
 /**
@@ -39,12 +40,19 @@ class AgpCompatibilityMatrixTest {
         // `--dry-run` still runs configuration in full — variant callbacks, source set wiring, task
         // graph construction — which is where every AGP API the plugin touches is exercised. It just
         // skips the task actions, which would need a real RN app to produce anything meaningful.
-        GradleRunner.create()
-            .withProjectDir(projectDir)
-            .withPluginClasspath(pluginClasspathFor(agpVersion))
-            .withArguments("assembleDebug", "--dry-run", "--stacktrace")
-            .forwardOutput()
-            .build()
+        val result =
+            GradleRunner.create()
+                .withProjectDir(projectDir)
+                .withPluginClasspath(pluginClasspathFor(agpVersion))
+                .withArguments("assembleDebug", "--dry-run", "--stacktrace")
+                .forwardOutput()
+                .build()
+
+        // AGP 9 turns resValues off by default and warns on every read of variant.resValues.
+        assertFalse(
+            result.output.contains("Accessing value resValues"),
+            "the plugin reads variant.resValues although the feature is disabled",
+        )
     }
 
     @Test