android: Update targetSdkVersion to 34, i.e., Android 14

[chrisbobbe]

With a prep commit that came from @gnprice's investigation into some code in react-native that needed to change; thanks Greg for that!

Fixes: #5877

[chrisbobbe]

Greg, when you have some time to do so, could you test this on your device running Android 14?

[gnprice]

Great, thanks for taking care of this!

The changes look good. I'll test when I'm at home (since I discovered the other day at #5891 (comment) that I no longer have a working zulip-mobile environment on my office desktop).

[gnprice]

Pretty sure this targetSdkVersion check shouldn't be here and is just cruft that makes the code more complicated to think about. But 🤷 React Native's code quality is rapidly becoming not our problem — and definitely the right thing here is to backport the commit verbatim, like this. (So long as it doesn't have bugs with direct behavior impact, anyway.)

[gnprice]

Oh, do you have these changes as a branch in a react-native Git repo? It'd be good to (a) push that branch to our fork zulip/react-native, and (b) mention the branch name and commit ID here, in the commit that updates the patch.

[chrisbobbe]

Oh, do you have these changes as a branch in a react-native Git repo? It'd be good to (a) push that branch to our fork zulip/react-native, and (b) mention the branch name and commit ID here, in the commit that updates the patch.

I don't, but I'll create such a branch and do those things.

[chrisbobbe]

Done; revision pushed.

[gnprice]

Thanks!

I just tried this version with tools/run-android on my Android 14 device (a Pixel 8). It still crashes on startup, with the same stack trace as at #5877 (comment) .

I looked at the Java source file that's supposed to be patched, and it has the patch:

$ grep -C2 compatRegister node_modules/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerBase.java
        IntentFilter filter = new IntentFilter();
        filter.addAction(getReloadAppAction(mApplicationContext));
        compatRegisterReceiver(mApplicationContext, mReloadAppBroadcastReceiver, filter, true);
        mIsReceiverRegistered = true;
      }
--
   * <p>https://developer.android.com/about/versions/14/behavior-changes-14#runtime-receivers-exported
   */
  private void compatRegisterReceiver(
      Context context, BroadcastReceiver receiver, IntentFilter filter, boolean exported) {
    if (Build.VERSION.SDK_INT >= 34 && context.getApplicationInfo().targetSdkVersion >= 34) {

I suspect what's happening is that the build of the Android side is using a published binary artifact for the 0.68.7 release, not the source code in node_modules/. That's the reason for the Gradle-related steps described here:
https://github.com/zulip/zulip-mobile/blob/main/docs/howto/rn-from-git.md#per-project-setup

So I think those steps will still be necessary.

The other two changes we currently have in this file are in JS code (for tests only, in fact) and in a build-time script for the iOS build. That's why they didn't need those changes.

When I read this PR on Friday, I looked in the history and saw our previous use of patch-package for react-native, at b546b07, which is in native code that I expect is part of that same build. So that caused me to think that maybe things had changed since I last studied how this aspect of the build worked, and those setup changes weren't needed.

But looking again now, I see from the commit message that the problem it was meant to solve was only in a build with Xcode, for iOS. Those have always built from source, rather than using a published build artifact like the Android build. So I think the story must be that that patch never did have an effect on Android — and that just was fine, because the problem it was solving didn't exist there.

[chrisbobbe]

I see. So, more work to do here.

In step 4 of that doc:

4. Follow the various steps in this
   doc
   that involve tweaking several Gradle files located under android/ in
   the app. Commit the result.

I followed that link and added the includeBuild lines it says to add:

  // ...
  include ':app'
  includeBuild('../node_modules/@react-native/gradle-plugin')
  
+ includeBuild('../node_modules/react-native') {
+     dependencySubstitution {
+         substitute(module("com.facebook.react:react-android")).using(project(":packages:react-native:ReactAndroid"))
+         substitute(module("com.facebook.react:react-native")).using(project(":packages:react-native:ReactAndroid"))
+         substitute(module("com.facebook.react:hermes-android")).using(project(":packages:react-native:ReactAndroid:hermes-engine"))
+         substitute(module("com.facebook.react:hermes-engine")).using(project(":packages:react-native:ReactAndroid:hermes-engine"))
+     }
+ }

But that caused a build failure:

$ tools/gradle :app:assembleDebug

FAILURE: Build failed with an exception.

* Where:
Settings file '/Users/chrisbobbe/dev/zulip-mobile/android/settings.gradle' line: 13

* What went wrong:
Project with path ':packages:react-native:ReactAndroid' not found in build 'react-native'.

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

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

BUILD FAILED in 1s

That docs website has been updated a few times, and I think we'll need to follow an older version of these instructions.

I tried the next-to-latest version, before facebook/react-native-website@22da0cc63 "Update how-to-build-from-source for 0.72+". That failed with Project with path ':ReactAndroid' not found in build 'react-native'..

So I think the next thing for me to try is the version just before facebook/react-native-website@50a01ac0d 'Update the "Build from source" page'. Those instructions are much more complicated, though, so I'd like to check in before I get started. Does that seem like the right next step? Thanks in advance :)

[gnprice]

So I think the next thing for me to try […] Does that seem like the right next step?

Let's move to a chat thread for that discussion:
https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/RN.20from.20Git/near/1948101

[chrisbobbe]

Thanks for all your help on this, @gnprice! Revision pushed.

[chrisbobbe]

OK, added a commit to let Gradle use more memory; that should help with the CI failure (which was showing out-of-memory errors).

[gnprice]

Great, glad to see this working!

The changes all look good. And I just tested making a debug build on my laptop and running it on my Android 14 device, and it seems to run fine. Merging now.