Skip to content

Commit

Permalink
RNGP - Honor the --active-arch-only when configuring the NDK (#35860)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #35860

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

Reviewed By: cipolleschi

Differential Revision: D42547987

fbshipit-source-id: 5a34e7087bb4f89de74cc52f9c505e36896fbf03
  • Loading branch information
cortinico authored and facebook-github-bot committed Jan 18, 2023
1 parent 235887a commit 470f79b
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,13 @@ internal object ProjectUtils {
internal fun Project.needsCodegenFromPackageJson(model: ModelPackageJson?): Boolean {
return model?.codegenConfig != null
}

internal fun Project.getReactNativeArchitectures(): List<String> {
val architectures = mutableListOf<String>()
if (project.hasProperty("reactNativeArchitectures")) {
val architecturesString = project.property("reactNativeArchitectures").toString()
architectures.addAll(architecturesString.split(",").filter { it.isNotBlank() })
}
return architectures
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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])
}
}

0 comments on commit 470f79b

Please sign in to comment.