[Android] possible regression with :app:mergeReleaseResources in 0.60.0-rc.1
#25325
Comments
react-native-bot
added
Impact: Regression
|
Can you run If you believe this information is irrelevant to the reported issue, you may write `[skip envinfo]` alongside an explanation in your Environment: section.
|
|
@react-native-bot it is included already :( |
|
Update Gradle to newest version. Should resolve this issue |
|
Hi @rozPierog I'm already using the latest gradle 5.4.1 and gradle plugin 3.4.1 generated from RN 0.60 template. |
|
@robertying unfortunately this is coming from a PR I proposed. The underlying problem it was trying to solve is here #22234 and the PR I proposed was a version of a patch developed on that issue that seemed to work for everyone for nearly a year. Is there a better way to solve the original problem? |
|
Hi @mikehardy thanks for reaching out here :) I believe these commits have only recently been picked into release. So most of people have not had it yet, I assume? Regarding your question, I remember one guy from that thread said some other guys put extra resources in version control so that he had the issue. In that case, I believe they could just remove those generated resources from version control and it should work. What’s your opinion? |
|
So now I have tested the following:
These scripts somehow copy unnecessary files to resources dir starting in 0.60.0. So I think there are actually two issues I want to address:
|
|
Hi @mikehardy I carefully took a look at the issue #22234 and believe your commit may not be necessary.😥 So this issue #22234 happened because people may do this:
Those two can be avoid by correctly using What is your scenario of this problem? Can it be solved this way? If so, those generated assets may not need copying from build dir. ;) |
|
@robertying is correct. Absolutely no generated code should make its way into the |
|
@robertying thank you so much for looking at this! I do a CLI call like the one you posted, so apparently this is user error (read as: my error). I would be okay with reverting the patch I proposed obviously, but maybe there could be some canonical documentation of how exactly to manually bundle ? I for instance must manually bundle because (in the "did you know..." dept) that on iOS 9 and Android APIs < 18 at least, you can't do port forwarding etc, so you have to send the bundle in even on dev builds? So manual bundling is a vital feature for me but I clearly messed up the CLI invocation :-) |
|
What is the "perfect" CLI invocation for a manual bundle? Just knowing that would allow us to at least disseminate on related issues in combination with revert, and hopefully drop in docs somewhere |
|
@mikehardy I was not aware of this problem for older targets. But since run-android will eventually call bundle command, I believe we can find where the best asset dest is from its source code. :) I can take a look and try to come up with something workable. |
|
good point re: using existing example 😆 - I'm still handling androidx conversion user confusion, anything you can do is very appreciated by me. I'm still bummed my PR caused a regression - hate it when that happens! |
|
@mikehardy no worries! At least you helped a lot of people. |
|
@mikehardy I did some print on This is for Here it is! Is this helpful? 😊 |
|
IDK if this was mentioned before but building the android apk with a pre-built bundle can be done by appending If the bundle and assets are already at the locations expected by the gradle script, they will be packaged into the apk |
|
@robertying @mirceanis - I appreciate the pointers but neither of these appears sufficient to work in my testing. It seems like there is an interplay in gradle between different build / copy / merge phases for bundle generation and bundle+asset inclusion. I did have assets copied into src/assets, and removing them allows things to bundle fine if I define Further, all the documentation on the web indicates the command line I was using (that drops things in src) is what to do which is apparently what got us in this mess. So I think we have an opportunity to define a command and publish it for real for everyone, but the current style (in my opinion) of separately bundling is a bad developer experience. What I'm trying to work up now is a way to simply run I'll update if / when there is some success |
|
@mikehardy since most of the users would only use |
|
@robertying - Yes - please PR that up if you have time, and thanks in advance if so. The regression is real. And I have a solution at least at the gradle level now for toggling bundle construction. Just seeing if I can verify how to plumb it all the way up to react-native (or not) before I put that on the linked issue. |
|
Here's how to generate offline dev APKs without causing any of this hassle: #22234 (comment) |
|
@Fouppy Unfortunately, this is still an issue. I opened a PR facebook/metro#420 to metro but they don't seem to be responding. So I guess this problem will exist for some time. But meanwhile, you could use the patched file to get around. |
|
@robertying thanks for your reply. I'll use that :) |
|
I'm facing this issue with v0.60.4, is there any workaround? I know the PR was accepted and will be released, but right now I cannot generate a production app. Updatemy issue was related to this |
|
@amadeu01 does my patch work for you? Use npm script |
|
Full patch script based on @robertying's comment. Add this script to the root project directory and run it Then, inside your inside your Make sure you have The script applies the changes from the PR. |
|
@robertying I just removed the reference to |
Summary: **Summary** From dcb41e3#diff-235a3e5d21175615e1cc254dd8b17eb2, files with `json` extension will be considered as assets and returned as `outputAssets` for `react-native-community/cli`. This means `cli` will copy files with `json` extension to resource dir, in Android's case, `android/app/build/res`. Here comes a problem. If `package.json` at the project root gets copied, it won't be renamed as other `package.json` in `node_modules` (e.g. `raw/node_modules_reactnativecodepush_package.json`). Instead, it will reside in `res` as `package.json`, which causes the error: facebook/react-native#25325 ``` android/app/src/main/res/raw/package.json: Error: package is not a valid resource name (reserved Java keyword) ``` This regression (I believe ;)) was introduced to React Native 0.60.0-rc with metro bumping from 0.51 to 0.54. **Possible impact** 1. Typical usage concerning `package.json` at the root will be something like `import packageJson from "./package.json"` in JS files to get some project configurations and those content will be bundled into `index.android.bundle` no matter what. So removing `$projectRoot/package.json` won't have too much impact. 2. This new (or bad?) behavior has only been in React Native 0.60-rc so it won't affect current projects running 0.59. It should be fixed before the final release of 0.60, I assume. **Test plan** 1. `react-native init resoucesRepro --version react-native@next` (so it uses 0.60.0-rc) 1. Add `import packageJson from './package.json'` to `App.js`. 1. `cd android && ./gradlew assembleRelease` 1. Get the error. 1. Modify `node_modules/metro/src/DeltaBundler/Serializers/getAssets.js` to match this commit. 1. `cd android && ./gradlew assembleRelease` 1. It now works! Pull Request resolved: #420 Differential Revision: D16646741 Pulled By: cpojer fbshipit-source-id: fc5065cce3e95529c678a8efa5b86c0148d71724
|
Hi Guys, Execution failed for task ':app:mergeReleaseResources'.
This is happening only in android release version. I tried the solution specified here of adding the postInstall script but that didnt help me. My debug version is fine as well as my ios build works fine. Only android build is failing with the above mentioned error while doing release build. Any help is appreciated. |
|
@mailyokesh you can remove the postinstall script and install the latest
Also something that may be irrelevant: |
|
@mailyokesh Robert's suggestion is best, but just in case that's not possible for folks there was a typo in my script which I've now fixed. |
|
@robertying i tried the step you mentioned. Still getting the same error. @IgorBelyayev if I use the post install script you provided above and do npm install i get this below error (node:27142) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 SIGINT listeners added. Use emitter.setMaxListeners() to increase limit npm ERR! A complete log of this run can be found in: react-native info |
|
@mailyokesh can you post the |
|
@mailyokesh The error with the script is saying that |
|
@robertying Below is my package |
|
@IgorBelyayev now after installing babel-cli i dont see errors during npm install. But during android release build i still see the same issue. @robertying @IgorBelyayev Can you help me understand what is going on here? I understand that some duplicates are being copied during android release build, which we are trying to avoid. is this something introduced with build script? any thing i can do manually avoid this? What went wrong:
|
|
@mailyokesh can you totally remove the postinstall script, save the package.json and |
|
@robertying removed the postinstall script,
Error from Metro Bundler: Loading dependency graph, done. Here is the full error during the build process Deprecated Gradle features were used in this build, making it incompatible with Gradle 6.0. FAILURE: Build failed with an exception.
BUILD FAILED in 45s error Failed to install the app. Make sure you have the Android development environment set up: https://facebook.github.io/react-native/docs/getting-started.html#android-development-environment. Run CLI with --verbose flag for more details. FAILURE: Build failed with an exception.
BUILD FAILED in 45s |
|
@mailyokesh hmmm same error and it is still the issue of metro bundler. I think I have two possible ways we can try out:
|
|
@robertying i already tried method 1 you suggested. As far method 2, this project i just created fresh from cli. So I have my full app code in 0.59.10. The error happens only if I use my java script code. Looks like when I use my javascript code during release process is trying to copy these, perhaps one of the node modules dependency is the culprit? I used the "metro": "^0.56.0", which is the latest right? |
|
@mailyokesh I just realized I did not test the latest metro in release! So it is probably still the issue with metro. I need to check it again tomorrow. Sorry for the inconvenience if it was truly my misguidance! So for now I'm sure replacing the file would work. You do not need This only works for Linux and macOS though. But it is just simple copy and replace. |
|
@robertying really appreciate your timely help. so i need to add this to my package.json? "postinstall": "chmod +x patches/patch.sh && patches/patch.sh && jetify" and inside the patches you have couple of libraries..how do i know what are all the libraries I need to use ? Honesly I dont fully understand what is causing the issue here? Can you explain since you seem to have a clear idea of the issue. |
|
@mailyokesh you can take a look at the pull request facebook/metro@f3c9862 then you should know why we do this: Simply put, we just replace So everything that can add this line facebook/metro@f3c9862#diff-61b17a5623701d43923f4293e13ceab1R41 is helpful for the issue. So you can either use some script to add this line, or simply replace the file in And we use |
|
@robertying thanks a lot for patiently explaining. I was able to get the release app running by manully doing the change in the respective files. I can now work towards why the post install scripts didnt work. Really appreciate the help @robertying @IgorBelyayev |
|
@mailyokesh just check the node_modules after postinstall. It should help. You are welcome! Happy coding! |
|
also, a trick, if you have modified a package after install, by directly editing the files in node_modules, then your change won't work if you delete your own node_modules and reinstall, and it won't work on CI or co-workers machines etc, because the file needs to be changed every time. Easy solution though - this is the trick - install the wonderful patch-package npm module, hook it in your postinstall in package.json, and after editing in node modules run That diffs the original module vs the changes you made, creates a patch, puts it in the ./patches/ directory with a versioned name and everything, and now your change will be applied for everyone postinstall (so it works on CI etc) and later if the module is updated it will give you a nice warning to see if you still need your patch. react-native development is painful (to me) without this tool...with it, no problems |
…k#24778) (facebook#25363) Summary: Pull requests facebook#24518 facebook#24778 make Gradle copy all **generated** assets and resources into `android/app/src/res`, which is a bad behavior, because `src/res` goes into version control and should hold only those **original** resource files. These changes in facebook#24518 facebook#24778 were merged into 0.60.0-rc release and cause regression. This pull request will: - Revert pull requests facebook#24518 facebook#24778 - Close facebook#25325 ## Changelog [Android] [Fixed] - Fix regression of improper assets copy (revert facebook#24518 facebook#24778) Pull Request resolved: facebook#25363 Test Plan: It is a revert pull request and the reverted script should work the same as it has in 0.59.x. Differential Revision: D15963329 Pulled By: cpojer fbshipit-source-id: 5619a318dbdb40e816e37b6e37d4fe32caa46e9e
Summary: Issue #22234 includes a number of people (myself included) suffering with duplicate resource errors while building in Android. We have been collectively using a patch there but I don't believe any attempt has been made to PR it upstream. I am not excellent at gradle, so I may have approached this in completely the wrong way, but it works for me in the standard init templates, as well as my current work project which has a complicated 2 buildType + 4 productFlavor configuration. If there is a better way to achieve the result I am happy to learn The approach here is to determine the generated path the resources land in, then move them into the main build output tree. If they remain in the generated path android merging fails with duplicate resource errors, so that move (vs copy) is important. [Android] [Fixed] - Fix duplicate resource error in Android build Pull Request resolved: #24518 Differential Revision: D15276981 Pulled By: cpojer fbshipit-source-id: 3fe8c8556e4dcdac5f96a2d4658ac9b5d9b67379
Problems
Those problems were introduced in 0.60.0-rc:
import packageJson from './package.json'causes errorApparently
package.jsongot copied tores/rawwhich makes gradle complain. This works fine in 0.59.x, however.All those generated resources (e.g.
package.json,app.json...) get moved toandroid/app/src/main/res.buildrather thansrc.metrowill also complain:Possible relevant commits
962437f
eb534bc
React Native version:
Steps To Reproduce
The text was updated successfully, but these errors were encountered: