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

Fixed the run-android cli script to start the correct activity when using different applicationId for different variants #12820

Closed
wants to merge 1 commit into from

Conversation

kgritesh
Copy link

@kgritesh kgritesh commented Mar 9, 2017

I have fixed the old PR #8950 for the latest version of react-native. This PR fixes the runAndroid.js cli that uses the manifest file that is generated during the build process to find out the correct package name and activity name. As a result, it solves the issue where runAndroid command fails to start the app after installing it. Relevant Issue: #5546.

Some feedback i received for the last PR is to see if the desired result can be achieved using less lines of code. I looked at it again and am not sure i know how to achieve that. I list the step involved in order to fix this issue below.

  • Find the correct manifest file for the particular variant that was installed

    1. Find all the buildTypes defined in the android/app/build.gradle file.
    2. Use the buildTypes found above and split the variant (default: debug) into a buildType and a product flavor
    3. Find if separate build enabled for different processors
    4. Using the above information find the path for the correct AndroidManifest.xml that is generated for the current run.
  • Parse the manifest file found above and extract correct activity class.

  • Use this activity class to start the app after its installed

Let me know @hramos @sharnik @Snorlock @chuyik if this works or any pointers in how to fix this.

…sing different applicationId for different variants
@facebook-github-bot facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Mar 9, 2017
@vongohren
Copy link

Looks like someone else is doing the same: #12634

function isSeparateBuildEnabled(gradleFilePath) {
// Check if separate build enabled for different processors
const content = fs.readFileSync(gradleFilePath, 'utf8');
const separateBuild = content.match(/enableSeparateBuildPerCPUArchitecture\s+=\s+([\w]+)/)[1];
Copy link
Contributor

Choose a reason for hiding this comment

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

I think there should be a null check for separateBuild before you access [1] of it. Reason: I was integrating React Native in an existing project and by default it did not have this key in the build.gradle file, nor is it necessary. So it would keep throwing the wrong error because the code for getManifestFile is wrapped in a try catch. So the error that an end user actually gets is adb invocation failed. Do you have adb in your PATH?.

@karanjthakkar
Copy link
Contributor

@Snorlock I find this more reliable because:

  1. The other PR assumes that the --variant flag would consist of the build type and the product flavor, which is not always the case.
  2. Also, I'm not sure if extracting build type and product flavor from the variant is the right way to go because there is no way of marking they can exist together and individually as well.

I have tried this PR while integrating an existing project of mine and it worked perfectly with the exception of #12820 (comment).

@hramos @mkonicek Getting this in would really help solve issues faced by people who are integrating RN into existing apps which have different build types and product flavor configurations. What would it take to get this merged? I'd love to help!

// cc: @satya164

@Shane-Goodwin
Copy link

I came across this pull request while searching for a solution to this problem. The changes outlined worked almost perfectly except that line 288 should search for "buildTypes" plural. I changed it from:
const regex = new RegExp('buildType\\s+{', 'ig');
to:
const regex = new RegExp('buildTypes\\s+{', 'ig');
and all worked well for my additional buildTypes. Thanks!

@kgritesh
Copy link
Author

kgritesh commented Apr 8, 2017

@ericvicenti @karanjthakkar @Phredward since #13169 is already merged, i am assuming this PR is no longer necessary as it was trying to automatically get the appSuffixID. Please advise if I should close this or make it work with changes in the above mentioned issue.

@karanjthakkar
Copy link
Contributor

@kgritesh I don't think this is needed any longer. Does your use case solve with #13169?

@Phredward
Copy link
Contributor

Hi. I do still feel that it's worth trying to make runAndroid understand build.gradle so that developers only need to specify one argument to run their app (which variant to run).

That said, I no longer think the above is urgent, and I'm happy to take another stab at making this happen with the newer code, depending on what people think is the right way forward.

@aranda-adapptor
Copy link

Hi. I do still feel that it's worth trying to make runAndroid understand build.gradle so that developers only need to specify one argument to run their app (which variant to run).

Additionally, once we add flavors to build.gradle, the default react-native run-android no longer works (installDebug installs all test packages from all dependencies onto the target device!). If runAndroid understood build.gradle, it could pick the default (first?) to run installFirstFlavorDebug.

@rod-stuchi
Copy link

Working nicely. Thank you!

@facebook-github-bot
Copy link
Contributor

@kgritesh I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project.

@steinnes
Copy link

steinnes commented Aug 9, 2017

This works for me as well, we're using custom applicationIdSuffix different builds and since we started doing that react-native run-android stopped working. I replaced our runAndroid.js with the one from this PR and now it seems to find the right variant and supplies the right application id in the activity string.

@stale
Copy link

stale bot commented Oct 13, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Maybe the issue has been fixed in a recent release, or perhaps it is not affecting a lot of people. If you think this issue should definitely remain open, please let us know why. Thank you for your contributions.

@stale stale bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Oct 13, 2017
@mnsrv
Copy link

mnsrv commented Oct 13, 2017

why this is not merging?

@stale stale bot removed the Stale There has been a lack of activity on this issue and it may be closed soon. label Oct 13, 2017
@MrLoh
Copy link

MrLoh commented Nov 4, 2017

It seems like #13169 wasn't merged and there's still no solution for this, since the first pull request was release in July 2016?

@hramos
Copy link
Contributor

hramos commented Nov 13, 2017

#13169 was merged, see d8f23f7. Closing as this PR is no longer necessary.

@hramos hramos closed this Nov 13, 2017
@fgeorgsson
Copy link

fgeorgsson commented Nov 22, 2017

Whenever one of your flavors has a different package name #13169 still fails as it will use the packageName from the wrong AndroidManifest.xml. That is, it will not use the AndroidManifest.xml that was generated during build. This patch is still needed, your conclusion is wrong.

@Rovack
Copy link

Rovack commented Dec 21, 2017

@hramos, In light of the 11 people (at least) who believe the linked PR did not, in fact, solve the problem that necessitated this one, perhaps you could reconsider closing this?

@hramos
Copy link
Contributor

hramos commented Dec 21, 2017

First time I am aware of this. Can you please file a new issue?

@johanforssell
Copy link

I'm writing new versions of old apps, and thus I have inherited old app id's.

My code have the package com.common.package (in source files and AndroidManifest.xml).

My build.gradle contains

    buildTypes {
        debug {
        }
        release {
            minifyEnabled enableProguardInReleaseBuilds
            proguardFiles getDefaultProguardFile("proguard-android.txt"), "proguard-rules.pro"
        }
    }

    flavorDimensions "appName"
    productFlavors {
        appAAA {
            dimension "appName"
            applicationId "com.AAA.app"
            versionCode 100
            versionName "3.0.0"
        }
        appBBB {
            dimension "appName"
            applicationId "com.BBB.app"
            versionCode 100
            versionName '3.0.0'
        }
    }

First the app can be installed with gradlew

cd android && ./gradlew installAppBBBDebug

The I can start it manually with

adb -s emulator-5554 shell am start -n com.BBB.app/com.common.package.MainActivity

This is a bloody hassle. I would rather not have to mess with it like this.

@famousfilm
Copy link

Cross-posting what @Lavielle wrote because it solved my problem.
#17541 (comment)

Quote:
I ran into this issue and found some way to make it work:
I have two product flavors

  • prod, with applicationId "com.yetno.yetnoapp" (which is the main app id)
  • dev, with applicationId "com.yetno.yetno_app"

When building and running dev (react-native run-android --variant=devDebug) I was getting the error described in this issue.

Then I noticed, if I build the prod variant first and have it installed on the device, building and installing the dev variant works fine.
So the steps are:

  1. cd android && ./gradlew clean
  2. cd ..
  3. react-native run-android --variant=prodDebug
  4. react-native run-android --variant=devDebug

So this is not really an answer, but it worked for me and maybe it can help some of you guys out :)

@nicodelpiano
Copy link

nicodelpiano commented Nov 13, 2018

@famousfilm Are you sure that works? I'm guessing you are executing the Activity from prodDebug, not devDebug which is the one you want.

@famousfilm
Copy link

@nicodelpiano - I didn't use prodDebug for my setup, but that is one you can use. The instructions above were copied from a separate post just so no one one had to click links to get the content.

@philjoseph
Copy link

I have different ProductFlavors with completely different App ID's and this PR is resolving the pb of the application launch. I really think this shall be merged @hramos

@petergaultney
Copy link

petergaultney commented Feb 18, 2019

This is affecting us as well, on 0.56. Thanks @kgritesh for the PR. Maintainers: let's please see this re-opened and merged...

@hramos
Copy link
Contributor

hramos commented Feb 19, 2019

The CLI is no longer part of the repository in 0.59. Please open a new issue / PR against https://github.com/react-native-community/react-native-cli instead.

@elirangoshen
Copy link

I still have this issue. on rn 0.60.4 please reopen this

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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.