Skip to content
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

Proposal: remove enableSeparateBuildPerCPUArchitecture from the Android Template #602

Closed
cortinico opened this issue Feb 20, 2023 · 1 comment

Comments

@cortinico
Copy link
Member

cortinico commented Feb 20, 2023

Introduction

I'd like to propose to remove the enableSeparateBuildPerCPUArchitecture config from the new app template.

I'm not opening a full RFC as I believe this is going to be quite of a small change, but before we proceed with it, I'd like to collect some feedback and understand if there are any concerns.

Motivation

Currently the new app template includes a field called enableSeparateBuildPerCPUArchitecture which is defaulted to false. When set to true, this field controls if the APK produced by the :app:assemble<Flavor> tasks should be split per ABI (so produce 4 of them ["armeabi-v7a", "x86", "x86_64", "arm64-v8a"]) or should instead by a single APK.

This field was introduced in 2016 (see here) when APK was the only format to upload apps to the play store.

Now in 2023, App Bundles are enforced to upload to the Play Store (https://en.wikipedia.org/wiki/Android_App_Bundle).

The only valid use case of setting enableSeparateBuildPerCPUArchitecture to true, is if you're looking into uploading split APKs to alternative stores (like FDroid) or distribute your app manually.

On top of this, users could still create an App Bundle, and use bundletool to create split APKs from it as they wish.

Usage

From a quick Github Search, it seems like we have:

  • ~200 projects having the flag set to true (source)
  • ~12500 projects having the flag set to false (source)

Benefits

Given it's low usage, I believe we can remove this configuration (i.e. effectively hardcode it to false) and reduce the template size. We're effectively distributing build logic that won't be executed by the majority of users, but that can easily break upon app udpates.

Specifically the applicationVariants.all { block is probably going away in a future version of AGP and will break all the apps once we'll have to update.

I don't believe the app template should offer configuration for all possible scenarios, but just handle the most common use cases, here being using App Bundles or Universal APKs.

Details

Practically, I'd like to remove the following lines from the android/app/build.gradle:

diff --git a/template/android/app/build.gradle b/template/android/app/build.gradle
index 096417b06c7..322128499bf 100644
--- a/template/android/app/build.gradle
+++ b/template/android/app/build.gradle
@@ -1,8 +1,6 @@
 apply plugin: "com.android.application"
 apply plugin: "com.facebook.react"
 
-import com.android.build.OutputFile
-
 /**
  * This is the configuration block to customize your React Native Android app.
  * By default you don't need to apply any configuration, just uncomment the lines you need.
@@ -52,14 +50,6 @@ react {
     // hermesFlags = ["-O", "-output-source-map"]
 }
 
-/**
- * Set this to true to create four separate APKs instead of one,
- * one for each native architecture. This is useful if you don't
- * use App Bundles (https://developer.android.com/guide/app-bundle/)
- * and want to have separate APKs to upload to the Play Store.
- */
-def enableSeparateBuildPerCPUArchitecture = false
-
 /**
  * Set this to true to Run Proguard on Release builds to minify the Java bytecode.
  */
@@ -78,16 +68,6 @@ def enableProguardInReleaseBuilds = false
  */
 def jscFlavor = 'org.webkit:android-jsc:+'
 
-/**
- * Private function to get the list of Native Architectures you want to build.
- * This reads the value from reactNativeArchitectures in your gradle.properties
- * file and works together with the --active-arch-only flag of react-native run-android.
- */
-def reactNativeArchitectures() {
-    def value = project.getProperties().get("reactNativeArchitectures")
-    return value ? value.split(",") : ["armeabi-v7a", "x86", "x86_64", "arm64-v8a"]
-}
-
 android {
     ndkVersion rootProject.ext.ndkVersion
 
@@ -102,14 +82,6 @@ android {
         versionName "1.0"
     }
 
-    splits {
-        abi {
-            reset()
-            enable enableSeparateBuildPerCPUArchitecture
-            universalApk false  // If true, also generate a universal APK
-            include (*reactNativeArchitectures())
-        }
-    }
     signingConfigs {
         debug {
             storeFile file('debug.keystore')
@@ -130,22 +102,6 @@ android {
             proguardFiles getDefaultProguardFile("proguard-android.txt"), "proguard-rules.pro"
         }
     }
-
-    // applicationVariants are e.g. debug, release
-    applicationVariants.all { variant ->
-        variant.outputs.each { output ->
-            // For each separate APK per architecture, set a unique version code as described here:
-            // https://developer.android.com/studio/build/configure-apk-splits.html
-            // Example: versionCode 1 will generate 1001 for armeabi-v7a, 1002 for x86, etc.
-            def versionCodes = ["armeabi-v7a": 1, "x86": 2, "arm64-v8a": 3, "x86_64": 4]
-            def abi = output.getFilter(OutputFile.ABI)
-            if (abi != null) {  // null for the universal-debug, universal-release variants
-                output.versionCodeOverride =
-                        defaultConfig.versionCode * 1000 + versionCodes.get(abi)
-            }
-
-        }
-    }
 }
 
 dependencies {

We can then move the instructions to a dedicate guide on the reactnative.dev website on how to "publish to alternate stores" or use Split APKs.

Discussion points

Happy to collect feedback on this proposal and collect new ideas.

cortinico added a commit to cortinico/react-native that referenced this issue Mar 1, 2023
…tirely

Summary:
See react-native-community/discussions-and-proposals#602 for more context.
TL;DR: this flag is used for an edge case. We should not expose it to every users but move it to a guide.

I'll publish a guide on the website on how to achieve the same feature.

Changelog:
[Android] [Changed] - Remove the enableSeparateBuildPerCPUArchitecture from the template entirely

Reviewed By: cipolleschi

Differential Revision: D43695574

fbshipit-source-id: 4c5aa2df1c993da651169fd80944be552400f252
cortinico added a commit to cortinico/react-native that referenced this issue Mar 1, 2023
…tirely (facebook#36341)

Summary:
Pull Request resolved: facebook#36341

See react-native-community/discussions-and-proposals#602 for more context.
TL;DR: this flag is used for an edge case. We should not expose it to every users but move it to a guide.

I'll publish a guide on the website on how to achieve the same feature.

Changelog:
[Android] [Changed] - Remove the enableSeparateBuildPerCPUArchitecture from the template entirely

Reviewed By: cipolleschi

Differential Revision: D43695574

fbshipit-source-id: 0a8bc7a169804de41e0b0cf813d184301635b907
facebook-github-bot pushed a commit to facebook/react-native that referenced this issue Mar 1, 2023
…tirely (#36341)

Summary:
Pull Request resolved: #36341

See react-native-community/discussions-and-proposals#602 for more context.
TL;DR: this flag is used for an edge case. We should not expose it to every users but move it to a guide.

I'll publish a guide on the website on how to achieve the same feature.

Changelog:
[Android] [Changed] - Remove the enableSeparateBuildPerCPUArchitecture from the template entirely

Reviewed By: cipolleschi

Differential Revision: D43695574

fbshipit-source-id: a4f2df755f1d7bd0319a8e418c4ce96b541009ed
@cortinico
Copy link
Member Author

cortinico commented Mar 2, 2023

OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this issue May 22, 2023
…tirely (facebook#36341)

Summary:
Pull Request resolved: facebook#36341

See react-native-community/discussions-and-proposals#602 for more context.
TL;DR: this flag is used for an edge case. We should not expose it to every users but move it to a guide.

I'll publish a guide on the website on how to achieve the same feature.

Changelog:
[Android] [Changed] - Remove the enableSeparateBuildPerCPUArchitecture from the template entirely

Reviewed By: cipolleschi

Differential Revision: D43695574

fbshipit-source-id: a4f2df755f1d7bd0319a8e418c4ce96b541009ed
personalizedrefrigerator added a commit to personalizedrefrigerator/joplin that referenced this issue Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant