Skip to content

fix crash resulted from no flavor in mergeAssets task #29648

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
Closed
tarek360 wants to merge 1 commit into from
Closed

fix crash resulted from no flavor in mergeAssets task #29648

tarek360 wants to merge 1 commit into from

Conversation

@tarek360
Copy link

@tarek360 tarek360 commented Mar 20, 2019

Description

In packages/flutter_tools/gradle/flutter.gradle, if the Android existing app has flavors, we don't append the current flavor to find the correct mergeAssets task path.

Changes in this PR allows the developer to pass the current flavor of the app.

include ':app'
setBinding(new Binding([gradle: this, currentFlavor: 'demo']))
evaluate(new File(                                                     
        settingsDir.parentFile,                                              
        'my_flutter/.android/include_flutter.groovy'                         
))

Besides,
wiki: https://github.com/flutter/flutter/wiki/Add-Flutter-to-existing-apps
should be updated after merging this PR.

Related Issues

This should fix #29646

Breaking Change

this is not a breaking change.

Similar PR

The idea of this PR is similar to this #27154 but here we pass the current flavor.

@googlebot
Copy link

googlebot commented Mar 20, 2019

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.

What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@tarek360
Copy link
Author

tarek360 commented Mar 20, 2019

I signed it!

@googlebot
Copy link

googlebot commented Mar 20, 2019

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@jonahwilliams jonahwilliams added tool Affects the "flutter" command-line tool. See also t: labels. t: gradle "flutter build" and "flutter run" on Android labels Mar 20, 2019
@dnfield
Copy link
Contributor

dnfield commented Apr 9, 2019

Are we not able to use the --flavor parameter to solve this?

@tarek360
Copy link
Author

tarek360 commented May 8, 2019

@dnfield not sure, can we do it in the gradle script? how can I check this?

@dnfield
Copy link
Contributor

dnfield commented May 8, 2019

I'm asking if the existing flavor logic fails here.

@lengzds
Copy link

lengzds commented Jun 3, 2019

are you push this into master? I have meet the same problem now.

@ViVitalij
Copy link

ViVitalij commented Jul 18, 2019

Hello, are there any updates on this topic? Spent already 2 days to find that solution is here (since March) but no further steps have been taken since then.

@blasten
Copy link

blasten commented Jul 18, 2019 •
edited
Loading

This sounds more like a bug in the way we merge the assets in add2app scenarios. I'd prefer we fix that logic in flutter.gradle directly. cc @xster

@blasten blasten added the a: existing-apps Integration with existing apps via the add-to-app flow label Jul 18, 2019
@blasten blasten mentioned this pull request Jul 19, 2019
Closed
@xster
Copy link
Member

xster commented Jul 19, 2019

+1. Having users pass something additional explicitly in is a bit inconvenient. We should fix the root issue in flutter.gradle.

@blasten blasten mentioned this pull request Jul 24, 2019
Merged
9 tasks
blasten pushed a commit that referenced this pull request Jul 29, 2019
Allow flavors and custom build types in host app (#36805)
9f39cad 
Fixes these issues:
#30916
#34089
#36479
#29648
@blasten
Copy link

blasten commented Jul 29, 2019

This is fixed in master.

@blasten blasten closed this Jul 29, 2019
@tarek360
Copy link
Author

tarek360 commented Jul 30, 2019

Hi @blasten
Thank you for watching this PR but I wonder why you didn't mention the fix (another PR or commit)? it's 4 months ago since I did this effort and now you close without details :)

@xster
Copy link
Member

xster commented Jul 30, 2019

He did reference #36805 (linked the other way around)

johnsonmh pushed a commit to johnsonmh/flutter that referenced this pull request Jul 30, 2019
@johnsonmh
Allow flavors and custom build types in host app (flutter#36805)
122a54a 
Fixes these issues:
flutter#30916
flutter#34089
flutter#36479
flutter#29648
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 5, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

a: existing-apps Integration with existing apps via the add-to-app flow t: gradle "flutter build" and "flutter run" on Android tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add2App: crash resulted from no flavor appended to get the mergeAssets task

8 participants

@tarek360 @googlebot @dnfield @lengzds @ViVitalij @blasten @xster @jonahwilliams