Ensure the main AndroidManifest.xml is used#10050
Ensure the main AndroidManifest.xml is used#10050lnikkila wants to merge 1 commit intofacebook:masterfrom lnikkila:master
Conversation
Some Android projects might opt to use multiple manifest files, e.g. for choosing different permissions based on the build type. Android's build process merges the manifests; secondary manifests can omit everything present in the main manifest. We shouldn’t rely on those secondary manifests for information since crucial information like the package name might not be present.
|
By analyzing the blame information on this pull request, we identified @grabbou to be a potential reviewer. |
|
Thank you for your pull request. As you may know, we require contributors to sign our Contributor License Agreement, and we don't seem to have you on file and listed as active anymore. In order for us to review and merge your code, please email cla@fb.com with your details so we can update your status. |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
|
Looks like the E2E tests are failing since bdc4731 in master. |
|
It's been a while since the last commit was reviewed and the labels show this pull request needs review. Based on the blame information for the files in this pull request we identified @grabbou as a potential reviewer. Could you take a look please or cc someone with more context? |
|
Thanks for the PR! That addition looks interesting though I think it's a bit too much specific to your app needs. I mean - there's nothing wrong with that, but we would like to keep a balance of what CLI can do vs what we can maintain and test really well. To make your requested changes still const manifestPath = findManifest(sourceDir);with const manifestPath = userConfig.manifestPath
? path.join(sourceDir, userConfig.manifestPath)
: findManifest(sourceDir)and you should be able to specify a path to your PS. Don't forget to do the same for dependencyConfig! Your tests are |
|
Hey, thanks everyone for contributing here. It seems like this pull request has been abandoned so I am going to close it. If you would like to work on this more then please feel free to reopen it! |
Summary: `react-native link` often fails due to the wrong manifest being used when you use a debug manifest. `findManifest` returns `debug/AndroidManifest.xml` instead of `main/AndroidManifest.xml`. And the debug manifest usually does not have the package name defined so `projectConfigAndroid` throws a cryptic "Cannot read property 'replace' of undefined" error. This fixes the issue by throwing a more user friendly error and providing a `manifestPath` userConfig. This is mostly based on comments to #10050. Closes #13373 Differential Revision: D4945690 Pulled By: shergin fbshipit-source-id: b177f916fd4799c873d2515c18cbb87bef3203f0
Rationale from the commit message:
The story is that I was using multiple manifests and trying to run
react-native linkto link some libraries, that specific command kept failing with:Turns out it couldn’t find the package name from the manifest since it was looking into the debug build type’s manifest that didn’t have it.
Links for reference:
Please let me know if you need more info or if anything needs to be changed. Thanks!