Skip to content

Commit

Permalink
RNGP - Add Variant Support (#35063)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #35063

This is part of a series of tasks to make the React Native Gradle Plugin (RNGP) variant-aware.

Here I'm add Variant support to RNGP via the Variant API from AGP 7.x

A short summary of changes:
- I've pushed a number of breaking changes to ReactExtension (we should document them in the release notes). Specifically I've removed properties which I believe were unnecessary and confusing
- I've removed all the extra logic to do the .so cleanups and use the new tasks that use the Artifacts API
- I've introduced only a `debuggableVariants` to make all the decisions for the bundling and minification which should be sufficient for users
- I've removed all the funcional interfaces are replaced them with lists as they're easy to handle and understand for users.

Changelog:
[Android] [Changed] - Added Flavor Support to React Native Gradle Plugin (RNGP)

Reviewed By: cipolleschi

Differential Revision: D40335028

fbshipit-source-id: d9ac1437de8a27db2e93df15b13772b221e036b2
  • Loading branch information
cortinico authored and facebook-github-bot committed Oct 24, 2022
1 parent 2cc2ca1 commit 8ad86c7
Show file tree
Hide file tree
Showing 6 changed files with 96 additions and 466 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,12 @@

package com.facebook.react

import com.android.build.gradle.api.BaseVariant
import com.facebook.react.utils.projectPathToLibraryName
import java.io.File
import javax.inject.Inject
import org.gradle.api.Project
import org.gradle.api.file.DirectoryProperty
import org.gradle.api.file.RegularFileProperty
import org.gradle.api.provider.ListProperty
import org.gradle.api.provider.MapProperty
import org.gradle.api.provider.Property

abstract class ReactExtension @Inject constructor(project: Project) {
Expand Down Expand Up @@ -54,10 +51,10 @@ abstract class ReactExtension @Inject constructor(project: Project) {
val bundleCommand: Property<String> = objects.property(String::class.java).convention("bundle")

/**
* Custom configuration for the [bundleCommand]. If provided it will be passed over with a
* Custom configuration file for the [bundleCommand]. If provided, it will be passed over with a
* `--config` flag to the bundle command.
*/
val bundleConfig: Property<String> = objects.property(String::class.java)
val bundleConfig: RegularFileProperty = objects.fileProperty()

/**
* The Bundle Asset name. This name will be used also for deriving other bundle outputs such as
Expand All @@ -69,69 +66,24 @@ abstract class ReactExtension @Inject constructor(project: Project) {
objects.property(String::class.java).convention("index.android.bundle")

/**
* Variant Name to File destination map that allows to specify where is the resource dir for a
* specific variant. If a value is supplied, the plugin will copy the bundled resource for that
* variant from `generated/res/react/<variant>` into the custom specified location. Default: {}
* Toggles the .so Cleanup step. If enabled, we will clean up all the unnecessary files before the
* bundle task. If disabled, the developers will have to manually cleanup the files. Default: true
*/
val resourcesDir: MapProperty<String, File> =
objects.mapProperty(String::class.java, File::class.java).convention(emptyMap())

/**
* Variant Name to File destination map that allows to specify where is the asset dir for a
* specific variant. If a value is supplied, the plugin will copy the bundled JS for that variant
* from `generated/assets/react/<variant>` into the custom specified location. Default: {}
*/
val jsBundleDir: MapProperty<String, File> =
objects.mapProperty(String::class.java, File::class.java).convention(emptyMap())

/** ANT-style excludes for the bundle command. Default: ["android / **", "ios / **"] */
val inputExcludes: ListProperty<String> =
objects.listProperty(String::class.java).convention(listOf("android/**", "ios/**"))

/**
* Toggles the VM Cleanup step. If enabled, before the bundle task we will clean up all the
* unnecessary files. If disabled, the developers will have to manually cleanup the files.
* Default: true
*/
val enableVmCleanup: Property<Boolean> = objects.property(Boolean::class.java).convention(true)
val enableSoCleanup: Property<Boolean> = objects.property(Boolean::class.java).convention(true)

/** Extra args that will be passed to the [bundleCommand] Default: [] */
val extraPackagerArgs: ListProperty<String> =
objects.listProperty(String::class.java).convention(emptyList())

/**
* Allows to disable dev mode for certain variants. That's useful if you have a production variant
* (say `canary`) where you don't want dev mode to be enabled. Default: []
*/
val devDisabledInVariants: ListProperty<String> =
objects.listProperty(String::class.java).convention(emptyList())

/**
* Functional interface to disable dev mode only on specific [BaseVariant] Default: will check
* [devDisabledInVariants] or return True for Release variants and False for Debug variants.
*/
var disableDevForVariant: (BaseVariant) -> Boolean = { variant ->
variant.name in devDisabledInVariants.get() || variant.isRelease
}

/**
* Variant Name to Boolean map that allows to toggle the bundle command for a specific variant.
* Default: {}
*/
// todo maybe lambda as for hermes?
val bundleIn: MapProperty<String, Boolean> =
objects.mapProperty(String::class.java, Boolean::class.java).convention(emptyMap())

/**
* Functional interface to toggle the bundle command only on specific [BaseVariant] Default: will
* check [bundleIn] or return True for Release variants and False for Debug variants.
* Allows to specify the debuggable variants (by default just 'debug'). Variants in this list
* will:
* - Not be bundled (the bundle file will not be created and won't be copied over).
* - Have the Hermes Debug flags set. That's useful if you have another variant (say `canary`)
* where you want dev mode to be enabled. Default: ['debug']
*/
var bundleForVariant: (BaseVariant) -> Boolean = { variant ->
if (bundleIn.getting(variant.name).isPresent) bundleIn.getting(variant.name).get()
else if (bundleIn.getting(variant.buildType.name).isPresent)
bundleIn.getting(variant.buildType.name).get()
else variant.isRelease
}
val debuggableVariants: ListProperty<String> =
objects.listProperty(String::class.java).convention(listOf("debug"))

/** Hermes Config */

Expand All @@ -141,35 +93,18 @@ abstract class ReactExtension @Inject constructor(project: Project) {
*/
val hermesCommand: Property<String> = objects.property(String::class.java).convention("")

/** Toggle Hermes for the whole build. Default: false */
val enableHermes: Property<Boolean> = objects.property(Boolean::class.java).convention(false)

/**
* Functional interface to selectively enabled Hermes only on specific [BaseVariant] Default: will
* return [enableHermes] for all the variants.
*/
var enableHermesForVariant: (BaseVariant) -> Boolean = { enableHermes.get() }

/**
* Functional interface specify flags for Hermes on specific [BaseVariant] Default: will return
* [hermesFlagsRelease] for Release variants and [hermesFlagsDebug] for Debug variants.
*/
var hermesFlagsForVariant: (BaseVariant) -> List<String> = { variant ->
if (variant.isRelease) hermesFlagsRelease.get() else hermesFlagsDebug.get()
}

/**
* Functional interface to delete debug files only on specific [BaseVariant] Default: will return
* True for Release variants and False for Debug variants.
* Whether to enable Hermes only on certain variants. If specified as a non-empty list, hermesc
* and the .so cleanup for Hermes will be executed only for variants in this list. An empty list
* assumes you're either using Hermes for all variants or not (see [enableHermes]).
*
* Default: []
*/
var deleteDebugFilesForVariant: (BaseVariant) -> Boolean = { variant -> variant.isRelease }

/** Flags to pass to Hermes for Debug variants. Default: [] */
val hermesFlagsDebug: ListProperty<String> =
val enableHermesOnlyInVariants: ListProperty<String> =
objects.listProperty(String::class.java).convention(emptyList())

/** Flags to pass to Hermes for Release variants. Default: ["-O", "-output-source-map"] */
val hermesFlagsRelease: ListProperty<String> =
/** Flags to pass to Hermesc. Default: ["-O", "-output-source-map"] */
val hermesFlags: ListProperty<String> =
objects.listProperty(String::class.java).convention(listOf("-O", "-output-source-map"))

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
package com.facebook.react

import com.android.build.api.variant.AndroidComponentsExtension
import com.android.build.gradle.AppExtension
import com.android.build.gradle.internal.tasks.factory.dependsOn
import com.facebook.react.tasks.BuildCodegenCLITask
import com.facebook.react.tasks.GenerateCodegenArtifactsTask
Expand Down Expand Up @@ -47,10 +46,8 @@ class ReactPlugin : Plugin<Project> {
configureBuildConfigFields(project)
configureDevPorts(project)

project.afterEvaluate {
project.extensions.getByType(AppExtension::class.java).applicationVariants.all {
project.configureReactTasks(variant = it, config = extension)
}
project.extensions.getByType(AndroidComponentsExtension::class.java).onVariants { variant ->
project.configureReactTasks(variant = variant, config = extension)
}
configureCodegen(project, extension, isLibrary = false)
}
Expand Down
Loading

2 comments on commit 8ad86c7

@fabOnReact
Copy link
Contributor

@fabOnReact fabOnReact commented on 8ad86c7 Oct 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cortinico

rm -rf node_modules
rm -rf ~/.gradle
git clean -fdx
./gradlew cleanAll
yarn install
cd packages/rn-tester
../../gradlew :packages:rn-tester:android:app:installHermesDebug -PreactNativeArchitectures=arm64-v8a

error log

> Task :packages:rn-tester:android:app:configureCMakeDebug[arm64-v8a]
C/C++: CMake Warning at /Users/fabriziobertoglio/Library/Android/sdk/ndk/23.1.7779620/build/cmake/android-legacy.toolchain.cmake:416 (message):
C/C++:   An old version of CMake is being used that cannot automatically detect
C/C++:   compiler attributes.  Compiler identification is being bypassed.  Some
C/C++:   values may be wrong or missing.  Update to CMake 3.19 or newer to use
C/C++:   CMake's built-in compiler identification.
C/C++: Call Stack (most recent call first):
C/C++:   /Users/fabriziobertoglio/Library/Android/sdk/ndk/23.1.7779620/build/cmake/android.toolchain.cmake:55 (include)
C/C++:   /Users/fabriziobertoglio/Sourcecode/opensource/react-native/packages/rn-tester/android/app/.cxx/Debug/4j5q4768/arm64-v8a/CMakeFiles/3.18.1-g262b901/CMakeSystem.cmake:6 (include)
C/C++:   CMakeLists.txt:9 (project)

> Task :packages:rn-tester:android:app:installHermesDebug FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':packages:rn-tester:android:app:installHermesDebug'.
> java.util.concurrent.ExecutionException: java.lang.NullPointerException: Parameter specified as non-null is null: method com.android.build.gradle.internal.test.BuiltArtifactsSplitOutputMatcher.computeBestOutput, parameter builtArtifacts

* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.

* Get more help at https://help.gradle.org
complete debug log

➜  rn-tester git:(8ad86c70b63) ✗ yband --debug
yarn run v1.22.19
$ ../../gradlew :packages:rn-tester:android:app:installHermesDebug -PreactNativeArchitectures=arm64-v8a

> Task :ReactAndroid:hermes-engine:downloadHermes UP-TO-DATE
Download https://github.com/facebook/hermes/tarball/main

> Task :ReactAndroid:downloadBoost UP-TO-DATE
Download https://boostorg.jfrog.io/artifactory/main/release/1.76.0/source/boost_1_76_0.tar.gz

> Task :ReactAndroid:downloadDoubleConversion UP-TO-DATE
Download https://github.com/google/double-conversion/archive/v1.1.6.tar.gz

> Task :ReactAndroid:downloadFmt UP-TO-DATE
Download https://github.com/fmtlib/fmt/archive/6.2.1.tar.gz

> Task :ReactAndroid:downloadFolly UP-TO-DATE
Download https://github.com/facebook/folly/archive/v2021.07.22.00.tar.gz

> Task :ReactAndroid:downloadGlog UP-TO-DATE
Download https://github.com/google/glog/archive/v0.3.5.tar.gz

> Task :ReactAndroid:downloadLibevent UP-TO-DATE
Download https://github.com/libevent/libevent/releases/download/release-2.1.12-stable/libevent-2.1.12-stable.tar.gz

> Task :ReactAndroid:hermes-engine:configureBuildForHermes
CMake Deprecation Warning at CMakeLists.txt:42 (cmake_policy):
  The OLD behavior for policy CMP0026 will be removed from a future version
  of CMake.

  The cmake-policies(7) manual explains that the OLD behaviors of all
  policies are deprecated and that a policy should be set to OLD only under
  specific short-term circumstances.  Projects should be ported to the NEW
  behavior and not rely on setting a policy to OLD.


-- Threads enabled.
-- Doxygen disabled.
-- Go bindings disabled.
-- Found ld64 - /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ld
-- Could NOT find Python module pygments
-- Could NOT find Python module pygments.lexers.c_cpp
-- Could NOT find Python module yaml
-- CMAKE_HOST_SYSTEM_NAME = Darwin
-- CMAKE_SYSTEM_NAME = Darwin
-- HERMES_APPLE_TARGET_PLATFORM =
-- CMAKE_CROSSCOMPILING = FALSE
-- Configuring done
-- Generating done
-- Build files have been written to: /Users/fabriziobertoglio/Sourcecode/opensource/react-native/ReactAndroid/hermes-engine/build/hermes

> Task :ReactAndroid:hermes-engine:buildHermes
[  0%] Built target hermesPublic
[ 10%] Built target LLVHDemangle
[ 10%] Built target dtoa
[ 10%] Built target zip
[ 36%] Built target LLVHSupport
[ 36%] Built target hermesFrontEndDefs
[ 40%] Built target hermesPlatformUnicode
[ 50%] Built target hermesOptimizer
[ 50%] Built target hermesRegex
[ 63%] Built target hermesSupport
[ 66%] Built target hermesInst
[ 66%] Built target hermesADT
[ 66%] Built target hermesFlowParser
[ 70%] Built target hermesAST
[ 70%] Built target hermesAST2JS
[ 73%] Built target hermesParser
[ 76%] Built target hermesSourceMap
[ 83%] Built target hermesFrontend
[ 83%] Built target hermesBackend
[ 96%] Built target hermesHBCBackend
[100%] Built target hermesCompilerDriver
[100%] Built target hermesc

> Task :packages:rn-tester:android:app:configureCMakeDebug[arm64-v8a]
C/C++: CMake Warning at /Users/fabriziobertoglio/Library/Android/sdk/ndk/23.1.7779620/build/cmake/android-legacy.toolchain.cmake:416 (message):
C/C++:   An old version of CMake is being used that cannot automatically detect
C/C++:   compiler attributes.  Compiler identification is being bypassed.  Some
C/C++:   values may be wrong or missing.  Update to CMake 3.19 or newer to use
C/C++:   CMake's built-in compiler identification.
C/C++: Call Stack (most recent call first):
C/C++:   /Users/fabriziobertoglio/Library/Android/sdk/ndk/23.1.7779620/build/cmake/android.toolchain.cmake:55 (include)
C/C++:   /Users/fabriziobertoglio/Sourcecode/opensource/react-native/packages/rn-tester/android/app/.cxx/Debug/4j5q4768/arm64-v8a/CMakeFiles/3.18.1-g262b901/CMakeSystem.cmake:6 (include)
C/C++:   CMakeLists.txt:9 (project)

> Task :packages:rn-tester:android:app:installHermesDebug FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':packages:rn-tester:android:app:installHermesDebug'.
> java.util.concurrent.ExecutionException: java.lang.NullPointerException: Parameter specified as non-null is null: method com.android.build.gradle.internal.test.BuiltArtifactsSplitOutputMatcher.computeBestOutput, parameter builtArtifacts

* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.

* Get more help at https://help.gradle.org

BUILD FAILED in 7s
112 actionable tasks: 14 executed, 98 up-to-date
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
➜  rn-tester git:(8ad86c70b63) ✗

The previous commit builds with patch f8cd9f1

@cortinico
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the heads up @fabriziobertoglio1987

The fix for this is here:
#35093

Please sign in to comment.