Skip to content

Update Android Gradle plugin to 4.0.1 #29013

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

friederbluemle
Copy link
Contributor

@friederbluemle friederbluemle commented May 31, 2020

This is a major version update that needs to be tested thoroughly.

Summary

Android Studio 4.0.1 is now available in the stable channel

https://androidstudio.googleblog.com/2020/05/android-studio-40-available-in-stable.html
https://developer.android.com/studio/releases/gradle-plugin#4.0.1

Changelog

[Android] [Changed] - Update Android Gradle plugin to 4.0.1

Test Plan

Build project

Closes #29044

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label May 31, 2020
@react-native-bot react-native-bot added the Platform: Android Android applications. label May 31, 2020
@analysis-bot
Copy link

analysis-bot commented May 31, 2020

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 7,385,801 12,920
android hermes armeabi-v7a 7,009,021 9,068
android hermes x86 7,823,917 10,536
android hermes x86_64 7,717,499 12,568
android jsc arm64-v8a 9,528,006 11,448
android jsc armeabi-v7a 9,147,130 11,364
android jsc x86 9,392,682 10,044
android jsc x86_64 9,974,392 10,844

Base commit: c421838

@analysis-bot
Copy link

analysis-bot commented May 31, 2020

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

Base commit: c421838

@friederbluemle friederbluemle changed the title Update Android Gradle plugin to 4.0.0 Update Android Gradle plugin to 4.0.1 Jul 30, 2020
@friederbluemle friederbluemle force-pushed the update-gradle-plugin-4 branch from fe355a1 to b575aee Compare July 30, 2020 07:26
@friederbluemle friederbluemle force-pushed the update-gradle-plugin-4 branch from b575aee to fa3bc8c Compare August 24, 2020 18:29
@friederbluemle friederbluemle marked this pull request as ready for review September 1, 2020 22:56
@friederbluemle
Copy link
Contributor Author

Rebased, since #28170 was merged. Ready for early review/testing, not ready for merge.

@dulmandakh
Copy link
Contributor

dulmandakh commented Sep 25, 2020

Please revert NDK change. It just works fine for me on RN 0.63 app.

@dulmandakh
Copy link
Contributor

Let's merge this before cutting 0.64, which should be coming soon 😇

@fkgozali
Copy link
Contributor

Please revert NDK change. It just works fine for me on RN 0.63 app.

waiting for this update then I'll merge

@friederbluemle
Copy link
Contributor Author

Please revert NDK change. It just works fine for me on RN 0.63 app.

You mean the older version works fine? Did you try the newer one? The default for 4.0.1 is 21.0.6113669, according to this page:
https://developer.android.com/studio/projects/install-ndk#default-ndk-per-agp

@fkgozali
Copy link
Contributor

If we keep the latest: ndkVersion = "21.0.6113669", do we need to update the helloworld android template too?

@friederbluemle
Copy link
Contributor Author

friederbluemle commented Sep 25, 2020

The project in template/ is the only build.gradle file that currently (c015f48) has a ndkVersion specified. Two more are proposed to be added via #29987.

@gedeagas
Copy link
Contributor

gedeagas commented Sep 26, 2020

@friederbluemle do you have any reason to update the NDK version other than it is the default version for AGP 4.0.1?

cc @dulmandakh I think we should keep notes that once this pr ( with updated NDK version ) merge we need to update our CI NDK version to 21.0.6113669 so that a specific state of the repository is always built with the same exact tools.

@dulmandakh
Copy link
Contributor

AFAIK, CI still use NDK 20.x. A while ago, I tried to bump it to 21, but failed. So I propose to use same NDK version for React Native and RN apps, even we don't have known issues yet. For me, it's one less NDK version to install on my system at least.

@gedeagas
Copy link
Contributor

I agree with you @dulmandakh, if we don't have known issues, or need specific function I think it's better to stick with the current NDK version.

@fkgozali
Copy link
Contributor

Yeah, let's keep all versions in the repo consistent, and if an upgrade is needed, we'll do it in separate PR. I think it's also important to put this version down in one place so that all configs depending on it can get the same exact version (perhaps we should just add a constant in react.gradle?).

@fkgozali
Copy link
Contributor

@friederbluemle: thoughts?

@friederbluemle friederbluemle force-pushed the update-gradle-plugin-4 branch from 7e40076 to c23943a Compare October 1, 2020 02:32
@friederbluemle
Copy link
Contributor Author

Sure, as requested, I removed the alignment of the ndk from this PR again.

The main reason why I initially wanted to align the NDK with the AGP version is that the website linked above states:

Before release, each AGP version is thoroughly tested with the latest stable NDK release at that time.

In this case, if everything works fine with the older version, I guess we can update the AGP first.

@friederbluemle
Copy link
Contributor Author

@dulmandakh

AFAIK, CI still use NDK 20.x. A while ago, I tried to bump it to 21, but failed. So I propose to use same NDK version for React Native and RN apps, even we don't have known issues yet. For me, it's one less NDK version to install on my system at least.

Yes, definitely agree we should use the same NDK version for React Native and RN apps. Ideally, it should also be the same (default) version that the AGP uses (and is thoroughly tested by Google) - If there is some problem with the newer NDK, then of course - we should not update it yet.

@gedeagas

if we don't have known issues, or need specific function I think it's better to stick with the current NDK version.

Yes, I also agree with that! Although the same could be said about the update of the AGP as well "if we don't have an issue it's better to stick with the current version" ;)

Either way, I'm fine updating AGP first, and NDK later - just wanted to make sure this doesn't fall off the radar because everything is "working fine". Does anyone have logs/details on what issues there are with the current repo and NDK 21? Should we open an issue to at least keep track of it?

@fkgozali
Copy link
Contributor

fkgozali commented Oct 1, 2020

Thanks, I'll merge this ASAP. Re: NDK v21, let's get this set up in CI so that we can upgrade the versions to 21 all at once. cc @hramos re: CI.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

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

@friederbluemle
Copy link
Contributor Author

Thanks @fkgozali 👍

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @friederbluemle in 553fb8b.

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

@react-native-bot react-native-bot added the Merged This PR has been merged. label Oct 1, 2020
@friederbluemle friederbluemle deleted the update-gradle-plugin-4 branch October 2, 2020 17:20
@evelant
Copy link

evelant commented Oct 14, 2020

Android gradle plugin v4+ seems to be causing some issues. JS bundles are failing to get copied into the aab or apk output seemingly randomly (running build twice sometimes adds the js bundle to the output) #29398

Sorry to comment on a closed pr @fkgozali @friederbluemle but this change may bite a lot of people if it goes out in a release so I thought this would be the best place to alert those likely to know how to mitigate it.

@fkgozali
Copy link
Contributor

I'll defer to @friederbluemle and @dulmandakh for recommendation.

@evelant
Copy link

evelant commented Oct 14, 2020

Seems there's a change in task ordering behavior somewhere that causes bundle to get copied after assets are merged so bundle gets left out of assets. See here: react-native-community/upgrade-support#38 (comment)

@friederbluemle
Copy link
Contributor Author

@AndrewMorsillo - Thanks for the report and sorry to hear this update is causing some issues on your end.
Is there an easy way to set up a test project that reproducibly shows this problem? Unfortunately I won't have much in the next week or two to investigate this further. So unless @dulmandakh or someone else can come up with a solution, and this is causing issues for a sizable number of people, please feel free to revert this PR.

That said, we have been struggling with a separate (?) issue (libhermes.so not found) even before this update that might be related to an obscure internal task ordering change that happened between the 3.4 and 3.5 series. There is still no clear solution until today, and I am not sure it's related.

chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this pull request Sep 8, 2021
We decided to postpone taking 4.0.1 or later until after the RN
v0.64 upgrade. That's because someone reported an issue with
4.0.1 [1], and a supposed fix landed in
facebook/react-native@53f55001a, which was a change to RN's internal
code released in v0.64.

Release notes here:
  https://developer.android.com/studio/releases/gradle-plugin#4-1-0

Quite a few major changes announced, hopefully they're all good
ones. Our tests pass, and a debug build on Android built and ran
fine for me.

Part of the RN v0.63 -> v0.64 changes to the template app,
corresponding to:

  facebook/react-native@cf8368f20
  facebook/react-native@553fb8b28
  facebook/react-native@dfa9db49e

See discussion at
  https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/Pin.20to.20explicit.20NDK.20version.3F/near/1210089.

[1] facebook/react-native#29013 (comment)
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this pull request Sep 8, 2021
We decided to postpone taking 4.0.1 or later until after the RN
v0.64 upgrade. That's because someone reported an issue with
4.0.1 [1], and a supposed fix landed in
facebook/react-native@53f55001a, which was a change to RN's internal
code released in v0.64.

Release notes here:
  https://developer.android.com/studio/releases/gradle-plugin#4-1-0

Quite a few major changes announced, hopefully they're all good
ones. Our tests pass, and a debug build on Android built and ran
fine for me.

Part of the RN v0.63 -> v0.64 changes to the template app,
corresponding to:

  facebook/react-native@cf8368f20
  facebook/react-native@553fb8b28
  facebook/react-native@dfa9db49e

See discussion at
  https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/Pin.20to.20explicit.20NDK.20version.3F/near/1210089.

[1] facebook/react-native#29013 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Platform: Android Android applications.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants