Skip to content

Always install the ephemeral engine copy instead of fetching from CocoaPods specs#37906

Merged
jmagman merged 1 commit intoflutter:masterfrom
jmagman:engine-pod
Aug 10, 2019
Merged

Always install the ephemeral engine copy instead of fetching from CocoaPods specs#37906
jmagman merged 1 commit intoflutter:masterfrom
jmagman:engine-pod

Conversation

@jmagman
Copy link
Member

@jmagman jmagman commented Aug 9, 2019 •
edited
Loading

Description

For add-to-app the existing host app was running correctly, but the module flutter run will not launch:

[VERBOSE-2:dart_vm.cc(265)] VM snapshot must be valid.                  
[VERBOSE-3:shell.cc(210)] Check failed: vm. Must be able to initialize the VM

The Flutter pod was being fetched by CocoaPods instead of using the local Flutter.framework.

Related Issues

Fixes #37756.
Fixes #37852.

Tests

I added the following tests:

"Run ephemeral host app with CocoaPods"

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I signed the [CLA].
  • I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
  • I updated/added relevant documentation (doc comments with ///).
  • All existing and new tests are passing.
  • The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require Flutter developers to manually update their apps to accommodate your change?

  • Yes, this is a breaking change (Please read [Handling breaking changes]). Replace this with a link to the e-mail where you asked for input on this proposed change.
  • No, this is not a breaking change.

@jmagman jmagman added platform-ios iOS applications specifically tool Affects the "flutter" command-line tool. See also t: labels. a: existing-apps Integration with existing apps via the add-to-app flow labels Aug 9, 2019
@jmagman jmagman requested a review from xster August 9, 2019 01:43
@jmagman jmagman self-assigned this Aug 9, 2019
@fluttergithubbot
Copy link
Contributor

fluttergithubbot commented Aug 9, 2019

It looks like this pull request may not have tests. Please make sure to add tests before merging. While there are exceptions to this rule, if this patch modifies code it is probably not an exception.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@fluttergithubbot fluttergithubbot added the c: contributor-productivity Team-specific productivity, code health, technical debt. label Aug 9, 2019
@xster
Copy link
Member

xster commented Aug 9, 2019

Reading this PR, I'm guessing the hypothetical thing this fixes is like if I flutter build in the flutter module once, then pod installed in the iOS outer app, then I went back into the flutter module and called flutter build ios --local_engine something. Then I go back to the iOS outer app and just pressed run, the engine and the App.framework would mismatch?

But I'm not sure how that relates to those reported issues which seem to relate to plugins etc.

@xster
Copy link
Member

xster commented Aug 9, 2019

Maybe they toggled between debug and profile? That might break them too if they changed their scheme/build config in between and didn't run pod install?

@jmagman
Copy link
Member Author

jmagman commented Aug 9, 2019

By "local" engine I really just mean the one already in the asset cache.
The plugins podspec has a dependency on Flutter
https://github.com/flutter/plugins/blob/master/packages/android_alarm_manager/ios/android_alarm_manager.podspec#L17

If pod install can't find a local pod (:path) it fetches whatever remote CocoaPod it can find, which would be some random engine version that doesn't have the same version of Dart.

The old podhelper always pod 'Flutter' but with the new one you have to call install_flutter_engine_pod
bd47a31#diff-b528f954e369c153bb31a631a8033be3L49

@jmagman
Copy link
Member Author

jmagman commented Aug 9, 2019

So this has nothing to do with cleanup between build modes. It fails 100% and on the first run.
I had been testing the host app (which works) and the module app with flutter build ios but not flutter run.

@vishna
Copy link

vishna commented Aug 9, 2019

This fixes our #37852 issue - we're now able to run iOS flutter module on simulator. Thank you!

@xster
Copy link
Member

xster commented Aug 9, 2019

LGTM on the fix

@jmagman
Copy link
Member Author

jmagman commented Aug 9, 2019

@xster recommended I changed the test to parse the Podfile.lock instead of actually launching the app.

@xster
Copy link
Member

xster commented Aug 9, 2019

Still lgtm

@jmagman jmagman merged commit 78cca62 into flutter:master Aug 10, 2019
@jmagman jmagman deleted the engine-pod branch August 10, 2019 00:37
@kf6gpe kf6gpe added this to the December 2019 (Add-to-App) milestone Aug 29, 2019
@leoafarias leoafarias mentioned this pull request Oct 17, 2019
Closed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@xster xster Awaiting requested review from xster

Assignees

@jmagman jmagman

Labels

a: existing-apps Integration with existing apps via the add-to-app flow c: contributor-productivity Team-specific productivity, code health, technical debt. platform-ios iOS applications specifically tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Milestone

December 2019 (Add-to-App)

6 participants

@jmagman @fluttergithubbot @xster @vishna @kf6gpe @googlebot