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.

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

Move flutter_assets to App.framework #26630

Merged
merged 2 commits into from Jan 16, 2019

Conversation

Projects
None yet
4 participants
@dnfield
Copy link
Member

commented Jan 16, 2019

This moves the flutter_assets folder to App.framework. The engine side is already set up to use this correctly, and has been for some time.

  • Updates xcode_backend.sh to write flutter_assets to App.framework folder
  • Removes need to copy that folder when embedding assets for Add2App
  • Changes upgradePbxProjWithFlutterAssets to remove references to flutter_assets and app.flx.
    • Adds test for that method
    • Makes that method more efficient rather than doing multiple iterations of the pbxproj lines. Even if we can't land this patch, we should land a fix for that.
  • Updates pbxproj.tmpl files to remove references to flutter_assets.

This will make our Add2App story take one less step for iOS. It will also improve our ability to support Carthage (we can just vend the frameworks and the assets are included).

Fixes #23839

Helps with #26099

@dnfield dnfield requested review from chinmaygarde and cbracken Jan 16, 2019

@googlebot googlebot added the cla: yes label Jan 16, 2019

@dnfield dnfield requested review from a14n and xster and removed request for a14n Jan 16, 2019

@dnfield dnfield requested review from chinmaygarde and removed request for chinmaygarde Jan 16, 2019

@dnfield

This comment has been minimized.

Copy link
Member Author

commented Jan 16, 2019

For reviewers - 1215156 is the meat of this change, the second commit is updating all the pbxproj files in our various samples/tests so we don't end up rewriting them every time.

@dnfield

This comment has been minimized.

Copy link
Member Author

commented Jan 16, 2019

Helps with #24176

final Match match = oldAssets.firstMatch(line);
if (match != null) {
if (printedStatuses.add(match.group(1)))
printStatus('Removing obsolete reference to ${match.group(1)} from ${project.hostAppBundleName}');

This comment has been minimized.

Copy link
@xster

xster Jan 16, 2019

Contributor

Where are you removing it? If someone flutter created before, have the flutter_assets 'folder' in pbxproj, what's going to remove it for them?

This comment has been minimized.

Copy link
@dnfield

dnfield Jan 16, 2019

Author Member

The continue statement on the next line skips adding it to the StringBuffer.

This comment has been minimized.

Copy link
@dnfield

dnfield Jan 16, 2019

Author Member

Think that'd be preferable as an else clause?

@xster

This comment has been minimized.

Copy link
Contributor

commented Jan 16, 2019

LGTM

@dnfield dnfield merged commit d8db70a into flutter:master Jan 16, 2019

27 checks passed

WIP Ready for review
Details
analyze Task Summary
Details
analyze
Details
build_tests-linux Task Summary
Details
build_tests-linux
Details
build_tests-macos Task Summary
Details
build_tests-macos
Details
build_tests-windows Task Summary
Details
build_tests-windows
Details
cla/google All necessary CLAs are signed
codelabs-build-test Task Summary
Details
codelabs-build-test
Details
docs Task Summary
Details
docs
Details
flutter-build
tests-linux Task Summary
Details
tests-linux
Details
tests-macos Task Summary
Details
tests-macos
Details
tests-windows Task Summary
Details
tests-windows
Details
tool_tests-linux Task Summary
Details
tool_tests-linux
Details
tool_tests-macos Task Summary
Details
tool_tests-macos
Details
tool_tests-windows Task Summary
Details
tool_tests-windows
Details

@dnfield dnfield deleted the dnfield:flutter_assets_ios branch Jan 16, 2019

dnfield added a commit to dnfield/flutter that referenced this pull request Jan 16, 2019

dnfield added a commit that referenced this pull request Jan 17, 2019

kangwang1988 added a commit to alibaba-flutter/flutter that referenced this pull request Feb 12, 2019

Move flutter_assets to App.framework (flutter#26630)
* move flutter_assets to App.framework

* remove flutter_assets references from all pbxproj files checked in

kangwang1988 added a commit to alibaba-flutter/flutter that referenced this pull request Feb 12, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.