From 4f68e45d12ae45673b5d0de0c058506dcebe6f61 Mon Sep 17 00:00:00 2001 From: Nicola Corti Date: Tue, 17 Jan 2023 08:29:05 -0800 Subject: [PATCH] RNGP - Honor the --active-arch-only when configuring the NDK Summary: I've just realized that the `--active-arch-only` is not correctly passed down to RNGP to set up an abiFilter so users on 0.71 on New Architecture end up building all the architectures even if `--active-arch-only` is set. This fix makes sure the `abiFilters` is applied if the user specified either the `--active-arch-only`, the `reactNativeArchitectures` property and is not using the Split ABI feature. Changelog: [Android] [Fixed] - RNGP - Honor the --active-arch-only when configuring the NDK Differential Revision: D42547987 fbshipit-source-id: f1170101b206aa5a0d32582c6950761f9e7dee37 --- .../react/utils/NdkConfiguratorUtils.kt | 8 ++++ .../com/facebook/react/utils/ProjectUtils.kt | 9 ++++ .../facebook/react/utils/ProjectUtilsTest.kt | 41 ++++++++++++++++++- 3 files changed, 56 insertions(+), 2 deletions(-) diff --git a/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/utils/NdkConfiguratorUtils.kt b/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/utils/NdkConfiguratorUtils.kt index 9d0e6b9740c8e4..74b85fa0d65af6 100644 --- a/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/utils/NdkConfiguratorUtils.kt +++ b/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/utils/NdkConfiguratorUtils.kt @@ -10,6 +10,7 @@ package com.facebook.react.utils import com.android.build.api.variant.AndroidComponentsExtension import com.android.build.api.variant.Variant import com.facebook.react.ReactExtension +import com.facebook.react.utils.ProjectUtils.getReactNativeArchitectures import com.facebook.react.utils.ProjectUtils.isNewArchEnabled import java.io.File import org.gradle.api.Project @@ -49,6 +50,13 @@ internal object NdkConfiguratorUtils { if ("-DANDROID_STL" !in cmakeArgs) { cmakeArgs.add("-DANDROID_STL=c++_shared") } + + val architectures = project.getReactNativeArchitectures() + // abiFilters are split ABI are not compatible each other, so we set the abiFilters + // only if the user hasn't enabled the split abi feature. + if (architectures.isNotEmpty() && !ext.splits.abi.isEnable) { + ext.defaultConfig.ndk.abiFilters.addAll(architectures) + } } } } diff --git a/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/utils/ProjectUtils.kt b/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/utils/ProjectUtils.kt index c18946a0cd0e2c..ba490af271951c 100644 --- a/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/utils/ProjectUtils.kt +++ b/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/utils/ProjectUtils.kt @@ -43,4 +43,13 @@ internal object ProjectUtils { internal fun Project.needsCodegenFromPackageJson(model: ModelPackageJson?): Boolean { return model?.codegenConfig != null } + + internal fun Project.getReactNativeArchitectures(): List { + val architectures = mutableListOf() + if (project.hasProperty("reactNativeArchitectures")) { + val architecturesString = project.property("reactNativeArchitectures").toString() + architectures.addAll(architecturesString.split(",").filter { it.isNotBlank() }) + } + return architectures + } } diff --git a/packages/react-native-gradle-plugin/src/test/kotlin/com/facebook/react/utils/ProjectUtilsTest.kt b/packages/react-native-gradle-plugin/src/test/kotlin/com/facebook/react/utils/ProjectUtilsTest.kt index 4851fb2b0d9638..164088ef132e4e 100644 --- a/packages/react-native-gradle-plugin/src/test/kotlin/com/facebook/react/utils/ProjectUtilsTest.kt +++ b/packages/react-native-gradle-plugin/src/test/kotlin/com/facebook/react/utils/ProjectUtilsTest.kt @@ -11,12 +11,12 @@ import com.facebook.react.TestReactExtension import com.facebook.react.model.ModelCodegenConfig import com.facebook.react.model.ModelPackageJson import com.facebook.react.tests.createProject +import com.facebook.react.utils.ProjectUtils.getReactNativeArchitectures import com.facebook.react.utils.ProjectUtils.isHermesEnabled import com.facebook.react.utils.ProjectUtils.isNewArchEnabled import com.facebook.react.utils.ProjectUtils.needsCodegenFromPackageJson import java.io.File -import org.junit.Assert.assertFalse -import org.junit.Assert.assertTrue +import org.junit.Assert.* import org.junit.Rule import org.junit.Test import org.junit.rules.TemporaryFolder @@ -169,4 +169,41 @@ class ProjectUtilsTest { assertFalse(project.needsCodegenFromPackageJson(extension)) } + + @Test + fun getReactNativeArchitectures_withMissingProperty_returnsEmptyList() { + val project = createProject() + assertTrue(project.getReactNativeArchitectures().isEmpty()) + } + + @Test + fun getReactNativeArchitectures_withEmptyProperty_returnsEmptyList() { + val project = createProject() + project.extensions.extraProperties.set("reactNativeArchitectures", "") + assertTrue(project.getReactNativeArchitectures().isEmpty()) + } + + @Test + fun getReactNativeArchitectures_withSingleArch_returnsSingleton() { + val project = createProject() + project.extensions.extraProperties.set("reactNativeArchitectures", "x86") + + val archs = project.getReactNativeArchitectures() + assertEquals(1, archs.size) + assertEquals("x86", archs[0]) + } + + @Test + fun getReactNativeArchitectures_withMultipleArch_returnsList() { + val project = createProject() + project.extensions.extraProperties.set( + "reactNativeArchitectures", "armeabi-v7a,arm64-v8a,x86,x86_64") + + val archs = project.getReactNativeArchitectures() + assertEquals(4, archs.size) + assertEquals("armeabi-v7a", archs[0]) + assertEquals("arm64-v8a", archs[1]) + assertEquals("x86", archs[2]) + assertEquals("x86_64", archs[3]) + } }