build: Bump AGP to 7.1.1 and fix bundle inclusion in release mode #33057

gabrieldonadel · 2022-02-06T20:45:13Z

Summary

Upgrade Android Gradle to 7.1.0 on template and fix a bug where the bundle was not getting included when building the app in release mode

Closes #33002
Closes #33018
Closes #33046
Potentially fixes #33029

Changelog

[Android] [Changed] - Bump AGP to 7.1.0 and fix bundle inclusion in release mode

Test Plan

Run ./scripts/test-manual-e2e.sh
Select A new RN app using the template and Android
Test app on the emulator
Open Android studio build the app using release variant
Check the release apk using Android studio "Analyze APK" tool and ensure the bundle is included

analysis-bot · 2022-02-06T21:10:10Z

Platform	Engine	Arch	Size (bytes)	Diff
ios	-	universal	n/a	--

Base commit: 802b3f7
Branch: main

analysis-bot · 2022-02-07T01:48:55Z

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	8,191,599	+3,271
android	hermes	armeabi-v7a	7,791,727	+3,277
android	hermes	x86	8,561,577	+3,300
android	hermes	x86_64	8,514,620	+3,302
android	jsc	arm64-v8a	9,860,190	+2,527
android	jsc	armeabi-v7a	8,844,872	+2,541
android	jsc	x86	9,826,274	+2,568
android	jsc	x86_64	10,423,442	+2,556

Base commit: 802b3f7
Branch: main

cortinico

Awesome stuff! 🚀 The only thing missing is to replicate the login also here: https://github.com/facebook/react-native/blob/main/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/TaskConfiguration.kt

cortinico · 2022-02-07T17:28:22Z

react.gradle

-                        from(jsBundleDir)
+                        // Workaround for Android Gradle Plugin 3.4+ new asset directory
+                        into ("merged_assets/${variant.name}/out") {
+                            from(jsBundleDir)


Can I ask you to refactor a bit here. We don't need all those from(jsBundleDir). We just need one into the top-level task (line 306 essentially).

Sure, I've kept just one from(jsBundleDir) as you suggested

cortinico · 2022-02-07T17:29:09Z

react.gradle

-                    into ("assets/${targetPath}") {
+                    // Workaround for Android Gradle Plugin 7.1 asset directory
+                    if (androidComponents.pluginVersion.major == 7 && androidComponents.pluginVersion.minor == 1) {


I'm not entirely sure we need a if-then-else. We can just add another into directive. I believe other folders will be ignored (or at the end of the day, you'll end up having just one bundle file regardless).

Makes sense, I've just removed it

facebook-github-bot · 2022-02-08T14:45:26Z

@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

cortinico · 2022-02-09T17:11:49Z

There is a bump missing inside the build.gradle.kts in the top folder (which I believe could require further changes).

cortinico · 2022-02-10T12:10:38Z

I see that the CI is failing with:

* What went wrong:
Execution failed for task ':ReactAndroid:configureNdkBuildDebug[arm64-v8a]'.
> [CXX1405] error when building with ndkBuild using /root/react-native/ReactAndroid/src/main/jni/react/jni/Android.mk: Build command failed.
  Error while executing process /opt/android/ndk/21.4.7075529/ndk-build with arguments {NDK_PROJECT_PATH=null APP_BUILD_SCRIPT=/root/react-native/ReactAndroid/src/main/jni/react/jni/Android.mk APP_ABI=arm64-v8a NDK_ALL_ABIS=arm64-v8a NDK_DEBUG=1 APP_PLATFORM=android-21 NDK_OUT=/root/react-native/ReactAndroid/build/intermediates/cxx/Debug/6a6p3w1c/obj NDK_LIBS_OUT=/root/react-native/ReactAndroid/build/intermediates/cxx/Debug/6a6p3w1c/lib NDK_APPLICATION_MK=/root/react-native/ReactAndroid/src/main/jni/Application.mk THIRD_PARTY_NDK_DIR=/root/react-native/ReactAndroid/build/third-party-ndk REACT_COMMON_DIR=/root/react-native/ReactAndroid/../ReactCommon REACT_GENERATED_SRC_DIR=/root/react-native/ReactAndroid/build/generated/source REACT_SRC_DIR=/root/react-native/ReactAndroid/src/main/java/com/facebook/react -j2 APP_SHORT_COMMANDS=false LOCAL_SHORT_COMMANDS=false -B -n}
  Android NDK: WARNING: Ignoring unknown import directory: /root/react-native/ReactAndroid/build/generated/source    
  Android NDK: /root/react-native/ReactAndroid/../ReactCommon/butter/Android.mk: Cannot find module with tag 'glog' in import path    
  Android NDK: Are you sure your NDK_MODULE_PATH variable is properly defined ?    
  Android NDK: The following directories were searched:    
  Android NDK:         
  
  /root/react-native/ReactAndroid/../ReactCommon/butter/Android.mk:28: *** Android NDK: Aborting.    .  Stop.

That usually happens when the downloadGlog and other tasks are not executed before the ReactAndroid:configureNdkBuildDebug.

I had to address this for AGP 7.0.x here:

react-native/ReactAndroid/build.gradle

Lines 418 to 421 in 1a83dc3

    
           // Needed as some of the native sources needs to be downloaded 
        
           // before configureNdkBuildDebug could be executed. 
        
           configureNdkBuildDebug.dependsOn(preBuild) 
        
           configureNdkBuildRelease.dependsOn(preBuild)

I'm unsure why this broke for 7.1.x. I believe the task name/ordering changed inside AGP.

gabrieldonadel · 2022-02-11T04:56:51Z

That usually happens when the downloadGlog and other tasks are not executed before the ReactAndroid:configureNdkBuildDebug.

@cortinico do you know if we need to specify the Active ABI to set that it depends on preBuild? Something like configureNdkBuildDebug[arm64-v8a].dependsOn(preBuild) ? Although I'm not that this would be possible/necessary

cortinico · 2022-02-11T10:27:16Z

do you know if we need to specify the Active ABI to set that it depends on preBuild?

Nope we should not (I believe). The ABI you see in square brackets is just an indication of which ABI has been built at the moment, and is not a real task.

As a tip for debugging: ./gradlew :ReactAndroid:configureNdkBuildDebug --dry-run will show you a bit of the task dependency graph. I suppose preBuild and all the download* tasks are not in the list 🤔

gabrieldonadel · 2022-02-14T04:30:39Z

As a tip for debugging: ./gradlew :ReactAndroid:configureNdkBuildDebug --dry-run will show you a bit of the task dependency graph. I suppose preBuild and all the download* tasks are not in the list 🤔

@cortinico Actaully running ./gradlew :ReactAndroid:configureNdkBuildDebug --dry-run does show the preBuild and all the download*/prepare* tasks, but configureNdkBuildDebug[arm64-v8a], configureNdkBuildDebug[armeabi-v7a], configureNdkBuildDebug[x86] and configureNdkBuildDebug[x86_64] are the first 4 tasks shown although configureNdkBuildDebug is the last one

cortinico · 2022-02-14T16:54:42Z

Nope we should not (I believe).

Then I was wrong here :/ It seems like AGP introduced per-ABI tasks (which is going to be a bit of a pain for us). Then, you'll have to add explicit dependency between configureNdkBuildDebug[x86] (and so on...) and the preBuild task.

Also please star this issue: https://issuetracker.google.com/issues/207403732

gabrieldonadel · 2022-02-15T21:45:39Z

It seems like AGP introduced per-ABI tasks (which is going to be a bit of a pain for us). Then, you'll have to add explicit dependency between configureNdkBuildDebug[x86] (and so on...) and the preBuild task.

I explicitly added the preBuild task dependency to all configureNdkBuild variants +ABIs and that seems to fix the issue, now builds are failing due to missing libfb.so but I'm already looking into why this is failing

gabrieldonadel · 2022-02-16T22:51:57Z

I explicitly added the preBuild task dependency to all configureNdkBuild variants +ABIs and that seems to fix the issue, now builds are failing due to missing libfb.so but I'm already looking into why this is failing

@cortinico this was failing due to a missing task dependency on "configureNdkBuild${variant}[${architecture}]" inside RNTester but now tests are green. Do you think this change of behavior on AGP could affect other places as well?

cortinico · 2022-02-16T23:01:42Z

Do you think this change of behavior on AGP could affect other places as well?

The only place where I think it's worth extra care is here:

react-native/template/android/app/build.gradle

Lines 183 to 189 in fa85417

    
           afterEvaluate { 
        
               // If you wish to add a custom TurboModule or component locally, 
        
               // you should uncomment this line. 
        
               // preBuild.dependsOn("generateCodegenArtifactsFromSchema") 
        
               preDebugBuild.dependsOn(packageReactNdkDebugLibs) 
        
               preReleaseBuild.dependsOn(packageReactNdkReleaseLibs) 
        
           }

Have you tried if a new template works out of the box. I'm under the impression we need this extra handling also there, because the configureNdk* tasks won't find the shared libraries which are built by the packageReactNdk* tasks.

gabrieldonadel · 2022-02-17T04:00:19Z

Have you tried if a new template works out of the box. I'm under the impression we need this extra handling also there, because the configureNdk* tasks won't find the shared libraries which are built by the packageReactNdk* tasks.

Yeah, I've just double-checked it using test-manual-e2e.sh and selecting A new RN app using the template and also by opening the /tmp/RNTestProject/android on Android studio and building a release build, both run with any issues

facebook-github-bot · 2022-02-17T12:08:46Z

@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

cortinico · 2022-02-18T19:24:21Z

So some updates here. For @gabrieldonadel, in order to land this, we'll need to setup buildToolsVersion to 31.0.0 on every com.android.library/com.android.application Gradle module we have. That's a bit of a bummer but it's potentially the easiest solution we have at the moment to circumvent the failure (and I'm also not totally sure it will fix it but, let's give it a try).

I'm a little late the the party here but I wanted to note that this either needs a merge and a pick to 0.68 (if it's not tracking for that yet, should be) or it needs a documentation note that folks cannot update past android gradle plugin 7.0.x yet (plus likely an associated react-native doctor compatibility check). I guess the important thing is whether it's backwards compatible with android gradle plugin 7.0.x or not? If it is, then a pick to 0.68 stable branch makes sense, otherwise a doctor check + docs makes sense

I'd say let's go for the Doctor check + docs approach for 0.68. I'd like to have a stable RN version that runs on AGP 7.0.x. 7.1 was out just one month ago and I want to make sure we don't push too strong on consumer for updating their deps (that's true that there are users updating eagerly, but we also have a lot of other users which are not updating that fast).

Either way, this gradle plugin version bump thing catches out the bundling process about once every 1-2 years it seems (I remember a similar issue from gradle plugin 4.x days), thrilled to see a PR so far advanced here, pixelated trophies for all involved --> 🏆 😆

To follow up on this: I'd love to migrate some of our tasks to use the AGP Artifacts API. That would allow to have a more declarative approach on the transformation we apply to the APK and will ideally break less often in the future.

gabrieldonadel · 2022-02-18T22:46:58Z

For @gabrieldonadel, in order to land this, we'll need to setup buildToolsVersion to 31.0.0 on every com.android.library/com.android.application Gradle module we have. That's a bit of a bummer but it's potentially the easiest solution we have at the moment to circumvent the failure (and I'm also not totally sure it will fix it but, let's give it a try).

Sounds like a good plan, I might need some help on this because I'm not 100% sure on what you meant by

setup buildToolsVersion to 31.0.0 on every com.android.library/com.android.application Gradle module

Do you know in which places other than ReactAndroid/build.gradle we would need to addbuildToolsVersion = "31.0.0"?

cortinico · 2022-02-18T22:56:56Z

Do you know in which places other than ReactAndroid/build.gradle we would need to addbuildToolsVersion = "31.0.0"?

Essentially every place where we set minSdkVersion:

$ rg "minSdkVersion" -g "*.gradle*"

ReactAndroid/build.gradle
291:        minSdkVersion(21)

packages/rn-tester/android/app/build.gradle
165:        minSdkVersion 21

template/android/build.gradle
6:        minSdkVersion = 21

template/android/app/build.gradle
138:        minSdkVersion rootProject.ext.minSdkVersion

template/android/build.gradle is not needed, but the other 2 for sure.

cortinico · 2022-02-19T17:41:03Z

Since we're at it, could you bump to 7.1.1 so at least we avoid having to do another AGP bump shortly after? 👍

facebook-github-bot · 2022-02-21T11:19:37Z

@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

… architectures

facebook-github-bot · 2022-02-21T13:07:56Z

@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

react-native-bot · 2022-02-21T14:25:04Z

This pull request was successfully merged by @gabrieldonadel in 200488e.

^{When will my fix make it into a release? | Upcoming Releases}

…cebook#33057) Summary: Upgrade Android Gradle to 7.1.0 on template and fix a bug where the bundle was not getting included when building the app in release mode Closes facebook#33002 Closes facebook#33018 Closes facebook#33046 Potentially fixes facebook#33029 ## Changelog [Android] [Changed] - Bump AGP to 7.1.0 and fix bundle inclusion in release mode Pull Request resolved: facebook#33057 Test Plan: 1. Run `./scripts/test-manual-e2e.sh` 2. Select `A new RN app using the template` and `Android` 3. Test app on the emulator 4. Open Android studio build the app using release variant 4. Check the release apk using Android studio "Analyze APK" tool and ensure the bundle is included ![image](https://user-images.githubusercontent.com/11707729/152700410-3bcb80b0-35b6-4bdc-bf57-98a42a29e5a6.png) Reviewed By: ShikaSD Differential Revision: D34076884 Pulled By: cortinico fbshipit-source-id: da4392af37e08e22dbcafba38476fd712141474a

…n startup This is a new incarnation of the same underlying React Native issue seen in 8f8a266 and 833d083, back in 2019 and 2018 respectively. RN hard-codes what intermediate directory it expects the Android Gradle Plugin to look for assets in; but that directory isn't any kind of stable API, and in fact it changes from time to time in new versions of AGP. It changed again in AGP 7.1.0. More recent versions of RN know about this latest change (with yet another hardcoding, conditioned on a new range of AGP versions): facebook/react-native#33057 But ours doesn't, because we haven't been updating RN for a while in this legacy codebase. So just update the directory we force it to. See discussion: https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/release.20build.20missing.20JS.20bundle/near/1746718 [greg: expanded commit message]

facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Feb 6, 2022

react-native-bot added the Platform: Android Android applications. label Feb 6, 2022

gabrieldonadel force-pushed the feat/bump-agp-7 branch 2 times, most recently from ab972e0 to cb6aedb Compare February 7, 2022 01:09

cortinico reviewed Feb 7, 2022

View reviewed changes

gabrieldonadel force-pushed the feat/bump-agp-7 branch from 3c0db91 to ed1098a Compare February 7, 2022 20:42

gabrieldonadel mentioned this pull request Feb 8, 2022

RNTester Android instacrash on release mode #33065

Closed

gabrieldonadel requested a review from cortinico February 8, 2022 04:49

cortinico approved these changes Feb 8, 2022

View reviewed changes

gabrieldonadel force-pushed the feat/bump-agp-7 branch from c0fb276 to 3a69046 Compare February 10, 2022 03:08

gabrieldonadel force-pushed the feat/bump-agp-7 branch from 3a69046 to 00361a5 Compare February 11, 2022 00:30

AlexanderEggers mentioned this pull request Feb 11, 2022

[Android] Release builds broken when using Android Gradle plugin 7.1.0 or higher #33002

Closed

gabrieldonadel force-pushed the feat/bump-agp-7 branch from 3663869 to 00361a5 Compare February 11, 2022 05:02

gabrieldonadel force-pushed the feat/bump-agp-7 branch from 00361a5 to 704fe25 Compare February 15, 2022 17:18

gabrieldonadel requested a review from cortinico February 16, 2022 00:32

sergey-cheperis added a commit to casting-video/react-native that referenced this pull request Feb 18, 2022

Gradle 7.1 compat (facebook#33057)

0125dea

mikehardy mentioned this pull request Feb 19, 2022

doctor: combo check for android gradle plugin < 7.1 iff react-native <= 0.68 react-native-community/cli#1566

Closed

gabrieldonadel force-pushed the feat/bump-agp-7 branch from f02c083 to 41a689a Compare February 19, 2022 16:54

gabrieldonadel added 9 commits February 21, 2022 09:59

build: Bump AGP to 7.1.0 and fix bundle inclusion in release mode

a62df1f

refactor(react.gladle): Remove unnecessary function calls

b11ebbd

chore(react.gladle): Remove gradle version check

085951e

fix(react-native-gradle-plugin): Add AGP 7.1 asset directory workaround

2ce684f

build(gradle): Bump AGP to 7.1.0 on main build.gradle.kts

f0d0c1f

build(gradle): Fix configureNdkBuild task dependencies with different…

4164032

… architectures

chore(rn-tester): Fix configureNdkBuild task

4ec308b

build(gradle): Manually specify buildToolsVersion

c3bae2a

build: Bump AGP to 7.1.1

51b060c

gabrieldonadel force-pushed the feat/bump-agp-7 branch from 41a689a to 51b060c Compare February 21, 2022 13:00

cortinico changed the title ~~build: Bump AGP to 7.1.0 and fix bundle inclusion in release mode~~ build: Bump AGP to 7.1.1 and fix bundle inclusion in release mode Feb 21, 2022

facebook-github-bot closed this in 200488e Feb 21, 2022

react-native-bot added the Merged This PR has been merged. label Feb 21, 2022

gabrieldonadel deleted the feat/bump-agp-7 branch February 21, 2022 14:25

mikehardy mentioned this pull request Mar 1, 2022

Connectivity listener doesn't work on android 'RELEASE' version react-native-netinfo/react-native-netinfo#576

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: Bump AGP to 7.1.1 and fix bundle inclusion in release mode #33057

build: Bump AGP to 7.1.1 and fix bundle inclusion in release mode #33057

gabrieldonadel commented Feb 6, 2022 •

edited by mikehardy

Loading

analysis-bot commented Feb 6, 2022 •

edited

Loading

analysis-bot commented Feb 7, 2022 •

edited

Loading

cortinico left a comment

cortinico Feb 7, 2022

gabrieldonadel Feb 7, 2022

cortinico Feb 7, 2022

gabrieldonadel Feb 7, 2022

facebook-github-bot commented Feb 8, 2022

cortinico commented Feb 9, 2022

cortinico commented Feb 10, 2022

gabrieldonadel commented Feb 11, 2022

cortinico commented Feb 11, 2022

gabrieldonadel commented Feb 14, 2022

cortinico commented Feb 14, 2022

gabrieldonadel commented Feb 15, 2022

gabrieldonadel commented Feb 16, 2022

cortinico commented Feb 16, 2022

gabrieldonadel commented Feb 17, 2022

facebook-github-bot commented Feb 17, 2022

cortinico commented Feb 18, 2022

gabrieldonadel commented Feb 18, 2022

cortinico commented Feb 18, 2022

cortinico commented Feb 19, 2022

facebook-github-bot commented Feb 21, 2022

facebook-github-bot commented Feb 21, 2022

react-native-bot commented Feb 21, 2022

build: Bump AGP to 7.1.1 and fix bundle inclusion in release mode #33057

build: Bump AGP to 7.1.1 and fix bundle inclusion in release mode #33057

Conversation

gabrieldonadel commented Feb 6, 2022 • edited by mikehardy Loading

Summary

Changelog

Test Plan

analysis-bot commented Feb 6, 2022 • edited Loading

analysis-bot commented Feb 7, 2022 • edited Loading

cortinico left a comment

Choose a reason for hiding this comment

cortinico Feb 7, 2022

Choose a reason for hiding this comment

gabrieldonadel Feb 7, 2022

Choose a reason for hiding this comment

cortinico Feb 7, 2022

Choose a reason for hiding this comment

gabrieldonadel Feb 7, 2022

Choose a reason for hiding this comment

facebook-github-bot commented Feb 8, 2022

cortinico commented Feb 9, 2022

cortinico commented Feb 10, 2022

gabrieldonadel commented Feb 11, 2022

cortinico commented Feb 11, 2022

gabrieldonadel commented Feb 14, 2022

cortinico commented Feb 14, 2022

gabrieldonadel commented Feb 15, 2022

gabrieldonadel commented Feb 16, 2022

cortinico commented Feb 16, 2022

gabrieldonadel commented Feb 17, 2022

facebook-github-bot commented Feb 17, 2022

cortinico commented Feb 18, 2022

gabrieldonadel commented Feb 18, 2022

cortinico commented Feb 18, 2022

cortinico commented Feb 19, 2022

facebook-github-bot commented Feb 21, 2022

facebook-github-bot commented Feb 21, 2022

react-native-bot commented Feb 21, 2022

gabrieldonadel commented Feb 6, 2022 •

edited by mikehardy

Loading

analysis-bot commented Feb 6, 2022 •

edited

Loading

analysis-bot commented Feb 7, 2022 •

edited

Loading