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.

Sign up for GitHub

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

Jump to bottom

Fix incorrect merged asset path with flavor for Android Gradle Plugin 3.2. #21782

Closed
wants to merge 1 commit into from
Closed

Fix incorrect merged asset path with flavor for Android Gradle Plugin 3.2. #21782

wants to merge 1 commit into from

Conversation

yatatsu
Copy link
Contributor

@yatatsu yatatsu commented Oct 14, 2018

When we use product flavor (e.g. develop/production) with Android Gradle Plugin 3.2,
javascript bundle is not copied due to that merged asset path point to incorrect location.

related #21408

Test Plan:

  1. Create new plain react native project.
  2. Edit android/app/build.gradle, add simple product flavor one, two.
  3. Assemble release apk, and we can find index.android.bundle in apk.

Release Notes:

[ANDROID][BUGFIX][react.gradle] - Fix incorrect merged asset path for Android Gralde Plugin 3.2.

@yatatsu
Fix android merged assets dir path.
43775d9 
When use product flavor such as develop/production
in android with Android Gradle Plugin 3.2,
javascript bundle is not copied
due to incorrect merged asset path.

related facebook#21408
@facebook-github-bot
Copy link
Contributor

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 14, 2018
@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@kelset
Copy link
Contributor

kelset commented Oct 15, 2018

@dulmandakh what do you think?

@dulmandakh
Copy link
Contributor

dulmandakh commented Oct 15, 2018 •
edited
Loading

@kelset I'll test it tomorrow, and see if it works 👍

@ikesyo
Copy link
Contributor

ikesyo commented Oct 22, 2018

@dulmandakh Any progress on this?

@mikemorris
Copy link

mikemorris commented Oct 24, 2018 •
edited
Loading

I'm building for multiple flavors and #21408 was an insufficient fix for me (runtime crash withindex.bundle.android missing from APK assets when building with Gradle 3.2+) but everything is working again with this patch applied. Thanks!

Would love to see this land in master and a patch release soon.

@ikesyo
Copy link
Contributor

ikesyo commented Oct 25, 2018

As @nokite and @mikemorris say this is a working solution, it should be okay to merge this. Could you please land this @kelset?

@kelset
Copy link
Contributor

kelset commented Oct 25, 2018

Thanks for the feedback - I'll double check this soon and we'll try to import it.

cc @hramos

@mikemorris mikemorris mentioned this pull request Oct 25, 2018
Closed
@kelset kelset changed the title Fix incorrect merged asset path with flavor for Android Gralde Plugin 3.2. Fix incorrect merged asset path with flavor for Android Gradle Plugin 3.2. Oct 26, 2018
@ikesyo ikesyo mentioned this pull request Oct 30, 2018
Closed
@ikesyo
Copy link
Contributor

ikesyo commented Oct 30, 2018 •
edited
Loading

Hi @kelset @dulmandakh @hramos

Could you take a look and let's import this (and cherry-pick into 0.57-branch).

ikesyo referenced this pull request Oct 30, 2018
@edilaic @facebook-github-bot
Add workaround for Android Gradle Plugin 3.2 change to asset dir (#21409
ff084a4 
)

Summary:
Android Gradle Plugin 3.2 uses a new intermediates/merged_assets directory instead of intermediates/assets. This workaround copies the javascript bundle to both directories for compatibility purposes.

Fixes #21132
Fixes #18357
Pull Request resolved: #21409

Differential Revision: D10141860

Pulled By: hramos

fbshipit-source-id: 0fb20fcec67ec2bfd7a8d9052599bbc70464b466
@dulmandakh
Copy link
Contributor

@facebook-github-bot shipit

@facebook-github-bot facebook-github-bot added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Oct 31, 2018
facebook-github-bot
facebook-github-bot approved these changes Oct 31, 2018
View reviewed changes
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

dulmandakh is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@react-native-bot
Copy link
Collaborator

@yatatsu merged commit e90319e into facebook:master.

@facebook facebook locked as resolved and limited conversation to collaborators Oct 31, 2018
@react-native-bot react-native-bot added the Merged This PR has been merged. label Oct 31, 2018
kelset pushed a commit that referenced this pull request Nov 9, 2018
@yatatsu @kelset
Fix incorrect merged asset path with flavor for Android Gradle Plugin…
09184a7 
… 3.2. (#21782)

Summary:
When we use product flavor (e.g. develop/production) with Android Gradle Plugin 3.2,
javascript bundle is not copied due to that merged asset path point to incorrect location.

related #21408

<!--
Thank you for sending the PR! We appreciate you spending the time to work on these changes.
Help us understand your motivation by explaining why you decided to make this change.

If this PR fixes an issue, type "Fixes #issueNumber" to automatically close the issue when the PR is merged.

_Pull requests that expand test coverage are more likely to get reviewed. Add a test case whenever possible!_
-->
Pull Request resolved: #21782

Differential Revision: D12854056

Pulled By: hramos

fbshipit-source-id: d1e6a395e762cc4a4f133977d54d0f469fa66b8c
@hramos hramos removed Import Started This pull request has been imported. This does not imply the PR has been approved. labels Feb 6, 2019
t-nanava pushed a commit to microsoft/react-native-macos that referenced this pull request Jun 17, 2019
@yatatsu @facebook-github-bot
Fix incorrect merged asset path with flavor for Android Gradle Plugin…
6c23724 
… 3.2. (facebook#21782)

Summary:
When we use product flavor (e.g. develop/production) with Android Gradle Plugin 3.2,
javascript bundle is not copied due to that merged asset path point to incorrect location.

related facebook#21408

<!--
Thank you for sending the PR! We appreciate you spending the time to work on these changes.
Help us understand your motivation by explaining why you decided to make this change.

If this PR fixes an issue, type "Fixes #issueNumber" to automatically close the issue when the PR is merged.

_Pull requests that expand test coverage are more likely to get reviewed. Add a test case whenever possible!_
-->
Pull Request resolved: facebook#21782

Differential Revision: D12854056

Pulled By: hramos

fbshipit-source-id: d1e6a395e762cc4a4f133977d54d0f469fa66b8c
KusStar pushed a commit to KusStar/react-native that referenced this pull request Nov 11, 2020
@yatatsu @KusStar
Fix incorrect merged asset path with flavor for Android Gradle Plugin…
6427d48 
… 3.2. (facebook#21782)

Summary:
When we use product flavor (e.g. develop/production) with Android Gradle Plugin 3.2,
javascript bundle is not copied due to that merged asset path point to incorrect location.

related facebook#21408

<!--
Thank you for sending the PR! We appreciate you spending the time to work on these changes.
Help us understand your motivation by explaining why you decided to make this change.

If this PR fixes an issue, type "Fixes #issueNumber" to automatically close the issue when the PR is merged.

_Pull requests that expand test coverage are more likely to get reviewed. Add a test case whenever possible!_
-->
Pull Request resolved: facebook#21782

Differential Revision: D12854056

Pulled By: hramos

fbshipit-source-id: d1e6a395e762cc4a4f133977d54d0f469fa66b8c
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@dulmandakh dulmandakh dulmandakh approved these changes

@facebook-github-bot facebook-github-bot facebook-github-bot approved these changes

Assignees
No one assigned
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Platform: Android Android applications.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@yatatsu @facebook-github-bot @kelset @dulmandakh @ikesyo @mikemorris @react-native-bot @nokite @hramos