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

[kgritesh]

This is a small fix in the runAndroid that instead of using the AndroidManifest file from the src at app/src/main/AndroidManifest.xml 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

[ghost]

By analyzing the blame information on this pull request, we identified @burgalon and @jimthedev to be potential reviewers.

[mkonicek]

From the issue:

I saw that for the command react-native run-android you guys are getting the package name of the application out of app/src/main/AndroidManifest.xml. This is ok if you change the application package in the gradle file AND in the AndroidManifest.xml.

What's the reason to change only one and not the other? Do you have an example when you'd do that?

This PR contains a lot of code, is there a way to solve this say with a few lines of code?

One thing not clear from the description of this PR: what's the new --activity argument for? When would someone use it?

[tume]

Sorry if i'm discussing in wrong place but I also had problems with applicationId as described and this pull request came closest when searching.

Our case is that we have same application built for different environments (dev/qa/prod) and we need to install those side-by-side so that is the reason for different applicationIds. If I have understood correctly the applicationId should be decoupled from the Androidmanifest.xml package_name exactly for this kind of purpose.

See more: http://tools.android.com/tech-docs/new-build-system/applicationid-vs-packagename

Not experienced enough on Android build systems to comment on actual implementation but would think that simpler option would be just to read straight the applicationId from build.gradle if there is no other problems with it.

[ghost]

@kgritesh updated the pull request - view changes

[enterline]

Since we are already in the android directory: https://github.com/facebook/react-native/pull/8950/files#diff-a003f89d34db18b4567fdb55080a003eR81

I believe these two file paths should be app/build.gradle and app/build/intermediates/manifests/full

[kgritesh]

@enterline yes you are right these two paths should be app/build.gradle and app/build/intermediates/manifests/full . I have updated my PR accordingly.

[facebook-github-bot]

@kgritesh updated the pull request - view changes

[ghost]

@kgritesh updated the pull request - view changes

[ghost]

@kgritesh updated the pull request - view changes

[kgritesh]

@mkonicek sorry for missing your comments earlier.
Android has concept of product flavors where for each flavour you can define a different package name inside build.gradle but the AndroidManifest.xml will still only have package name for your default flavor. For eg for our app, we have a product flavor called staging and we add a .staging to the package name , however android manifest file has the base package name, hence as the issue suggests its better to get the package name from the generated AndroidManifest file and not the default one

As per your other comment about that this pr contains lot of code, i don't think there is a simpler and cleaner way to solve this for all kind of corner cases

Finally the --activity options allows you to pass name of your launch activity incase its not called MainActivity. Most common use case is when you are adding react native to an existing app and the default launch activity is named something else

[ghost]

@kgritesh updated the pull request - view changes

[StevePotter]

💯 . This fixed my problem. Our build.gradle looks like:

 buildTypes {
debug {
applicationIdSuffix '.debug'
versionNameSuffix '-DEBUG'
}
staging {
applicationIdSuffix '.staging'
versionNameSuffix '-STAGING'
}
}

And so on. This PR fixes it perfectly. I worked like mad on #10178 and it just got merged. So should this one. The build scripts in react-native are weak and people like @kgritesh are improving it. Let's get this merged because right now I have to patch it. Thanks @kgritesh for your hard work!

[kgritesh]

thanks @StevePotter for your kind words. I am also waiting for this to get merged.

[facebook-github-bot]

@kgritesh updated the pull request - view changes

[VictoriaQ]

This fixed our issues, thanks!

Note that (at least in Linux), if you run it with react-native run-android --variant=myflavorDebug it will complain because it cannot find 'app/build/intermediates/manifests/full/myflavor/Debug/AndroidManifest.xml.

Running the task with the variant name in lowercase react-native run-android --variant=myflavordebug, works, however.

I think that buildType should be converted to lowercase in getManifestFile.

[hramos]

@mkonicek thoughts on this PR?

[chetstone]

@mkonicek Any progress on merging this PR?

[fkrauthan]

Yeah what is going on? This merge request is super old. I already opened a bug about exactly this but it seems like no love for Android in react native :(

[hramos]

Thanks for the PR. I'd like to push for solving this issue with fewer lines of code as well.

[kgritesh]

Hey Hector, I will submit a new PR by end of this week making this better works?

…

On Fri, Jan 20, 2017 at 2:46 AM, Héctor Ramos ***@***.***> wrote: Thanks for the PR. I'd like to push for solving this issue with fewer lines of code as well. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#8950 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADLpr1wgBLx-plIEyVrZYJ5AjDEgJNAZks5rT9KUgaJpZM4JSGM1> .

[hramos]

Does it need to be a new PR or can you land the commits on this one?

[vongohren]

Hows the work with this going?

[vongohren]

Is this getting anywhere? @kgritesh?
Or is there any other way of getting different environmentapps on the phone?
I would like to get 3 apps on my phone, dev, staging and prod. But currently this is not working with react beacuse it does not support applicationIdSuffix.

Can this be solved with any other solution? @hramos@mkonicek
I guess you guys have environmentapps on your projects?

[hramos]

@Snorlock@chuyik the original author has not responded in a while. If you're interested in fixing this, feel free to send a PR.

[kgritesh]

Sorry guys for not responding. I will surely try to refix this over next week and send another PR for latest version of react native.

…

On Thu, Feb 23, 2017 at 1:02 AM, Héctor Ramos ***@***.***> wrote: @Snorlock <https://github.com/Snorlock> @chuyik <https://github.com/chuyik> the original author has not responded in a while. If you're interested in fixing this, feel free to send a PR. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#8950 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADLprxAVkZDzxQqWcgtYHlCA1P-2uoZnks5rfI1FgaJpZM4JSGM1> .

[vongohren]

It was closed because?

[hramos]

@Snorlock the author stated multiple times they will open a new PR that addresses the feedback provided above.

[kgritesh]

@Snorlock new PR submitted at #12820