-
Notifications
You must be signed in to change notification settings - Fork 6k
Update default flutter_assets path for iOS embedding #7518
Conversation
| if (flutterAssetsName == nil) { | ||
| // Default to "flutter_assets" | ||
| flutterAssetsName = @"flutter_assets"; | ||
| flutterAssetsName = @"Frameworks/App.framework/flutter_assets"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will flutter_tool changes need to land simultaneously with this engine roll, or have we already made the change there and are using some trick like a symlink to support both paths temporarily?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See the linked WIP PR. That will be necessary to land this engine roll.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just noticed the linked WIP patch flutter/flutter#26713 and the PR description. I think I answered my own question :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to avoid using a symlink if we can - it seems like it increases opportunity for confusion or mistakes
cbracken
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My preference, just from a flexibility point of view, would be to pull from a plist value, but fall back to this path. Are there any disadvantages to that?
I ask because people have complained about our current assumptions/hardcoding of Runner. I can imagine cases where someone might also complain about hardcoding App.
|
That's what we're doing today - https://github.com/flutter/engine/pull/7518/files#diff-8e54aca7e9d4fd69a57fb86d6ec3dee2R183 |
|
This PR just updates the default that we fall back to |
* move flutter_assets to App.framework * Roll engine to 05fee4e 05fee4e Update default flutter_assets path for iOS embedding (flutter/engine#7518) 02205db Roll src/third_party/skia 5d052dac3ac1..02738a86e5fd (4 commits) (flutter/engine#7541) af907c0 Roll src/third_party/skia 5c7a3ac0e214..5d052dac3ac1 (7 commits) (flutter/engine#7540) dde2866 IWYU to get SkFontMetrics (flutter/engine#7539)
* move flutter_assets to App.framework * Roll engine to 05fee4e 05fee4e Update default flutter_assets path for iOS embedding (flutter/engine#7518) 02205db Roll src/third_party/skia 5d052dac3ac1..02738a86e5fd (4 commits) (flutter/engine#7541) af907c0 Roll src/third_party/skia 5c7a3ac0e214..5d052dac3ac1 (7 commits) (flutter/engine#7540) dde2866 IWYU to get SkFontMetrics (flutter/engine#7539)
flutter/flutter#26630 failed because this still points plugins to the current location for
flutter_assets, which was detected when a test tried to load the flutter_gallery video player page.This updates the default location so that we expect to find
flutter_assetsinsideFrameworks/App.framework. I'm not thrilled with this - but overriding it with a plist value by default would be much harder to make non-breaking for clients.This will require a manual roll with the changes from https://github.com/dnfield/flutter/tree/flutter_assets_ios