Include a directory with Flutter assets by szakarias · Pull Request #12944 · flutter/flutter

szakarias · 2017-11-09T14:09:40Z

The flutter_assets directory is meant to eventually replace app.flx.

This PR also changes the way an asset is retrieved. On asset channel queries, engine now returns the path to the asset instead of returning the asset as byte data.

Before merging this I will have another PR ready which can upgrade the project.pbxproj file of existing projects to include flutter_assets.

Engine PR flutter/engine#4343

Hixie · 2017-11-09T18:25:29Z

packages/flutter/lib/src/services/asset_bundle.dart

just assert(bytes is Uint8List). If it's ever NOT a Uint8List this is a serious performance concern.

szakarias · 2017-11-13T08:21:32Z

cc @tvolkert

tvolkert · 2017-11-13T17:59:11Z

Before merging this I will have another PR ready which can upgrade
the project.pbxproj file of existing projects to include flutter_assets.

FYI @xster who's been working on a "flutter project upgrade" mechanism.

tvolkert

Once the engine changes land, this should include the engine roll to the appropriate revision.

tvolkert · 2017-11-13T20:14:38Z

packages/flutter_tools/bin/xcode_backend.sh

Not build_dir?

Nope, I am however considering renaming the option to assets-dir when doing the FLX removal cleanup.

Rename sgtm. And the reason we're putting this in derived_dir rather than build_dir is that we need to reference it from project.pbxproj (like the current app.flx, App.framework, etc.), right?

Yes, exactly. Should have added that, sorry.

xster · 2017-11-14T02:17:05Z

I'm probably still at least a week away from a functioning flutter project upgrade mechanism. How does this PR work with flutter projects created in the past?

szakarias · 2017-11-14T08:54:17Z

Old flutter projects will need to get their project.pbxproj updated for this to work. The PR for it is here #13011 .

jakobr-google

LGTM

jakobr-google · 2017-11-14T10:28:40Z

packages/flutter_tools/bin/xcode_backend.sh

Rename sgtm. And the reason we're putting this in derived_dir rather than build_dir is that we need to reference it from project.pbxproj (like the current app.flx, App.framework, etc.), right?

Hixie · 2017-11-22T23:00:48Z

@szakarias What is the status of this PR?

szakarias · 2017-11-23T08:21:52Z

I am waiting for review of the corresponding engine PR flutter/engine#4343. cc @chinmaygarde.

chinmaygarde · 2017-11-29T23:55:59Z

I have added a few more comments to the engine PR. I am still a bit unclear about how we are going to address the following issues:

How are the *.pbxproj template upgrades going to be applied to existing users of Flutter? I see no indication about providing upgrade guidance or warning in either the engine or framework patches.
- My hacky attempt to copy the resources manually in the xcode_backend.sh build invocation to the directory specified in ${TARGET_BUILD_DIR}/${WRAPPER_NAME}/flutter_assets seemed to work. I grabbed those variables from the Xcode build log by clicking the "Show environement variables in build log." checkbox. Maybe we dont need the templates to change.
Hardcoding a single directory named flutter_assets means we can't have multiple Flutter applications in the same application bundle.

szakarias · 2017-12-11T14:57:01Z

Reverted the change in asset retrieval. We still return assets as byte data instead of the asset paths. I am postponing this to a later change.

With respect to upgrading existing projects, there is this PR #13011.

With respect to having multiple Flutter applications in the same application bundle, I am not sure how hardcoding flutter_assets is worse than the already hardcoded app.flx?

tvolkert

LGTM

Hixie · 2017-12-13T22:14:45Z

BTW, this was checked in when the tree was red. Please avoid checking anything in when the tree is red except for attempts to fix the tree, because it makes it much harder to detect, and later identify, additional failures introduced while the previous failure was ongoing. Thanks!

This reverts commit 3af6b9c.

* Revert "Include a directory with Flutter assets (#12944)" This reverts commit 3af6b9c. * Revert "Upgrade project.pbxproj to include flutter_assets (#13011)" This reverts commit 08128cb. * Revert "Upgrade complex_layout project.pbxproj to include flutter_assets (#13544)" This reverts commit 35f1a04. * mark complex_layout_ios__start_up as flaky

mravn-google · 2017-12-16T21:55:02Z

@Hixie Tree was red because of a test we had identified, but not marked, as flaky. It was so marked a little later (#13542).

Hixie · 2017-12-16T22:46:40Z

Ah, excellent. It's good to leave a message on the PR when that happens so that everyone knows it was intentional. Thanks for the update!

* Revert "Include a directory with Flutter assets (flutter#12944)" This reverts commit 3af6b9c. * Revert "Upgrade project.pbxproj to include flutter_assets (flutter#13011)" This reverts commit 08128cb. * Revert "Upgrade complex_layout project.pbxproj to include flutter_assets (flutter#13544)" This reverts commit 35f1a04. * mark complex_layout_ios__start_up as flaky

googlebot added the cla: yes label Nov 9, 2017

szakarias requested a review from jakobr-google November 9, 2017 14:11

szakarias mentioned this pull request Nov 9, 2017

Handle Flutter assets outside FLX flutter/engine#4343

Merged

szakarias force-pushed the flutterAssets branch from 00b26c2 to cce3596 Compare November 9, 2017 14:46

Hixie reviewed Nov 9, 2017

View reviewed changes

tvolkert reviewed Nov 13, 2017

View reviewed changes

szakarias mentioned this pull request Nov 14, 2017

Upgrade project.pbxproj to include flutter_assets #13011

Merged

jakobr-google approved these changes Nov 14, 2017

View reviewed changes

szakarias force-pushed the flutterAssets branch from 6801fe7 to 0ef320e Compare November 14, 2017 14:08

This was referenced Nov 20, 2017

Select fonts from asset directory instead of FLX. flutter/engine#4376

Closed

Read platform kernel blob from asset directory instead of FLX flutter/engine#4385

Closed

szakarias force-pushed the flutterAssets branch from 3ce02f4 to 9f9c4fd Compare December 11, 2017 14:26

tvolkert approved these changes Dec 12, 2017

View reviewed changes

szakarias added 4 commits December 13, 2017 20:27

Move flutter assets out of flx

dfbb45f

comment

db818fe

Revert asset path change

86eea77

roll engine

8e87f35

szakarias force-pushed the flutterAssets branch from 9f9c4fd to 8e87f35 Compare December 13, 2017 19:39

szakarias merged commit 3af6b9c into flutter:master Dec 13, 2017

yjbanov added a commit to yjbanov/flutter that referenced this pull request Dec 13, 2017

Revert "Include a directory with Flutter assets (flutter#12944)"

4295253

This reverts commit 3af6b9c.

Hixie mentioned this pull request Dec 15, 2017

Gallery release builds increased in size #13618

Closed

DaveShuckerow pushed a commit to DaveShuckerow/flutter that referenced this pull request May 14, 2018

Include a directory with Flutter assets (flutter#12944)

f7e5f88

zoechi mentioned this pull request Dec 18, 2018

Would be nice to have a way to validate layouts at many sizes #5885

Closed

github-actions bot locked as resolved and limited conversation to collaborators Aug 12, 2021

Conversation

szakarias commented Nov 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Hixie Nov 9, 2017

Choose a reason for hiding this comment

Uh oh!

szakarias commented Nov 13, 2017

Uh oh!

tvolkert commented Nov 13, 2017

Uh oh!

tvolkert left a comment

Choose a reason for hiding this comment

Uh oh!

tvolkert Nov 13, 2017

Choose a reason for hiding this comment

Uh oh!

szakarias Nov 14, 2017

Choose a reason for hiding this comment

Uh oh!

jakobr-google Nov 14, 2017

Choose a reason for hiding this comment

Uh oh!

szakarias Nov 14, 2017

Choose a reason for hiding this comment

Uh oh!

xster commented Nov 14, 2017

Uh oh!

szakarias commented Nov 14, 2017

Uh oh!

jakobr-google left a comment

Choose a reason for hiding this comment

Uh oh!

jakobr-google Nov 14, 2017

Choose a reason for hiding this comment

Uh oh!

Hixie commented Nov 22, 2017

Uh oh!

szakarias commented Nov 23, 2017

Uh oh!

chinmaygarde commented Nov 29, 2017

Uh oh!

szakarias commented Dec 11, 2017

Uh oh!

tvolkert left a comment

Choose a reason for hiding this comment

Uh oh!

Hixie commented Dec 13, 2017

Uh oh!

mravn-google commented Dec 16, 2017

Uh oh!

Hixie commented Dec 16, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

szakarias commented Nov 9, 2017 •

edited

Loading