Always embed iOS Flutter.framework build mode version from Xcode configuration #42029

jmagman · 2019-10-05T00:30:37Z

Description

There were two links/copies of Flutter.framework inside my_flutter_app/ios/. One was symlinked from the cache during pod install and embedded by CocoaPods:

if [[ "$CONFIGURATION" == "Debug" ]]; then
  install_framework "${PODS_ROOT}/../.symlinks/flutter/ios/Flutter.framework"
fi
if [[ "$CONFIGURATION" == "Release" ]]; then
  install_framework "${PODS_ROOT}/../.symlinks/flutter/ios/Flutter.framework"
fi
if [[ "$CONFIGURATION" == "Profile" ]]; then
  install_framework "${PODS_ROOT}/../.symlinks/flutter/ios/Flutter.framework"
fi

The location of the framework was pulled from FLUTTER_FRAMEWORK_DIR which was only updated from a command line flutter build. This caused issues when a user runs flutter build --debug but then immediately tries to archive their app from Xcode. The FLUTTER_FRAMEWORK_DIR would still point to the Debug version even though the Xcode build config was Release.

The second version is copied during xcode_backend.sh. This used the Xcode build configuration to decide which version of Flutter.framework to use. The script runs on every Xcode build from the command line or from Xcode itself, and correctly (assuming schemes are well-named) chooses the right build mode version of the framework.

The CocoaPods version always gets the last word since it is embedded last after xcode_backend runs. Users experience various production blank screens/crashes/app submission rejections due to accidentally submitting the debug version of their engine.

This change removes the symlinked version and points to the xcode_backend location. It pulls a similar trick as podhelper.rb and copies the FLUTTER_FRAMEWORK_DIR version of the engine to the correct place if xcode_backend has not yet run. This fall-back lets pod install succeed, and the correct version of the framework is always copied over by xcode_backend on every Xcode build/test/archive.

This has the side effect of no longer changing the Podfile.lock (which should be checked into source control) every time the build mode changes.

 EXTERNAL SOURCES:
   Flutter:
    :path: ".symlinks/flutter/ios-release"

becomes

 
 EXTERNAL SOURCES:
   Flutter:
    :path: Flutter

Additionally, the Ruby code was updated to make it more idiomatic.

Related Issues

Fixes #24641
Fixes #25167
Fixes #25370
Fixes #28802

Some of these issues have become muddied so it will fix some people chiming in but not others:
#22765
#24887
#37850

Related:
#36247

Tests

Added a few file location checks to deploy_gallery. It will also be exercised by other integration tests that build or run on iOS.

Checklist

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

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 · 2019-10-05T00:32:13Z

packages/flutter_tools/templates/package/.gitignore.tmpl

@@ -56,7 +56,7 @@ build/
 **/ios/**/xcuserdata
 **/ios/.generated/
 **/ios/Flutter/App.framework
-**/ios/Flutter/Flutter.framework
+**/ios/Flutter/Flutter.*


The Flutter.podspec is now getting copied in. People will have to add it to their .gitignore or check it in... ☹️

jmagman · 2019-10-05T00:33:00Z

packages/flutter_tools/templates/cocoapods/Podfile-ios-swift

+      puts "Invalid plugin specification: #{line}"
+    end
+  end
+  generated_key_values
 end

 target 'Runner' do
  use_frameworks!


This is the same file as Podfile-ios-obj except for use_frameworks!.

jmagman · 2019-10-05T00:33:58Z

examples/platform_view/ios/Podfile.lock

@@ -1,23 +1,16 @@
 PODS:
  - Flutter (1.0.0)


This seems to be the only Podfile.lock in the entire repo. You can see how it changes.
I don't know why MaterialControls was in there, but it doesn't seem to be a real dependency and was automatically removed?

jmagman · 2019-10-05T00:34:21Z

dev/integration_tests/android_views/ios/Podfile

@@ -15,49 +15,64 @@ def parse_KV_file(file, separator='=')
  if !File.exists? file_abs_path
    return [];
  end
-  pods_ary = []
+  generated_key_values = {}


This is a copy of the template.

jmagman · 2019-10-05T00:34:31Z

dev/integration_tests/release_smoke_test/ios/Podfile

@@ -15,49 +15,64 @@ def parse_KV_file(file, separator='=')
  if !File.exists? file_abs_path
    return [];
  end
-  pods_ary = []
+  generated_key_values = {}


Copy of the template.

Do these mean this will break for consumers?

No, the original format of the Generated.xcconfig is the same, and this method isn't accessible outside this file (unless someone was doing some really weird hackery). The method return type is different now, but I changed the two places we call it.

Sorry, I mean the fact that you had to update all of these - will it not be automatic for people running Flutter on existing projects?

The only place this method is called is within this Podfile. So if they regenerate the Podfile they will get the updated method and the updated usage.

P.S. The stale framework issue this is fixing happens when developers fail to follow the deployment instructions and do not run flutter build ios in release before archiving. So if the Podfile isn't updated, but they follow instructions, they will be fine.

Maybe we could read the podfile an dlook for the the offending line?

A more detailed message, or a link to some docs on the website, could help folks do the necessary update manually if they have local edits they want to keep. Like, "Your Podfile is out of date. Regenerate it with .... If you have local edits that you'd like to keep, see ... for instructions."

It's easy to see if their Podfile is out of date without parsing (either .symlinks/flutter exists or it doesn't since that directory gets removed and recreated each pod install run).
The minimum Podfile change (without the mapping refactor) is pretty large, so it's not just a one-line swap:

generated_xcode_build_settings = parse_KV_file('./Flutter/Generated.xcconfig') if generated_xcode_build_settings.empty? puts "Generated.xcconfig must exist. If you're running pod install manually, make sure flutter pub get is executed first." end generated_xcode_build_settings.map { |p| if p[:name] == 'FLUTTER_FRAMEWORK_DIR' symlink = File.join('.symlinks', 'flutter') File.symlink(File.dirname(p[:path]), symlink) pod 'Flutter', :path => File.join(symlink, File.basename(p[:path])) end }

becomes:

copied_flutter_dir = File.join(__dir__, 'Flutter') copied_framework_path = File.join(copied_flutter_dir, 'Flutter.framework') copied_podspec_path = File.join(copied_flutter_dir, 'Flutter.podspec') unless File.exist?(copied_framework_path) && File.exist?(copied_podspec_path) generated_xcode_build_settings = parse_KV_file('./Flutter/Generated.xcconfig') if generated_xcode_build_settings.empty? puts "Generated.xcconfig must exist. If you're running pod install manually, make sure flutter pub get is executed first." end generated_xcode_build_settings = parse_KV_file(generated_xcode_build_settings_path) generated_xcode_build_settings.map { |p| if p[:name] == 'FLUTTER_FRAMEWORK_DIR' cached_framework_dir = p[:path] unless File.exist?(copied_framework_path) FileUtils.cp_r(File.join(cached_framework_dir, 'Flutter.framework'), copied_flutter_dir) end unless File.exist?(copied_podspec_path) FileUtils.cp(File.join(cached_framework_dir, 'Flutter.podspec'), copied_flutter_dir) end end } end pod 'Flutter', :path => 'Flutter'

Here's what it looks like when I have an old version, build, get the migration instructions, remove the Podfile, then build again:

Note I need to actually add instructions to that issue.

jmagman · 2019-10-05T00:34:41Z

examples/flutter_gallery/ios/Podfile

@@ -4,37 +4,75 @@
 # CocoaPods analytics sends network stats synchronously affecting flutter build latency.
 ENV['COCOAPODS_DISABLE_STATS'] = 'true'

-def parse_KV_file(file,separator='=')
+project 'Runner', {


Copy of the template.

jmagman · 2019-10-05T00:34:48Z

examples/platform_view/ios/Podfile

@@ -4,56 +4,75 @@
 # CocoaPods analytics sends network stats synchronously affecting flutter build latency.
 ENV['COCOAPODS_DISABLE_STATS'] = 'true'

+project 'Runner', {


Copy of the template.

jmagman · 2019-10-05T00:35:31Z

packages/flutter_tools/templates/cocoapods/Podfile-ios-objc

+      puts "Invalid plugin specification: #{line}"
+    end
+  end
+  generated_key_values


Return a map of keys => values instead of an array of single key => value maps.

jmagman · 2019-10-05T00:36:53Z

packages/flutter_tools/templates/cocoapods/Podfile-ios-objc

+
+    generated_xcode_build_settings_path = File.join(copied_flutter_dir, 'Generated.xcconfig')
+    unless File.exist?(generated_xcode_build_settings_path)
+      raise "Generated.xcconfig must exist. If you're running pod install manually, make sure flutter pub get is executed first"


This actually stops the script now instead of just printing, and it's in red (ignore the double period at the end, I fixed that but I was too lazy to generate another screen shot)!

jmagman · 2019-10-05T00:37:36Z

packages/flutter_tools/templates/cocoapods/Podfile-ios-objc

+    unless File.exist?(copied_framework_path)
+      FileUtils.cp_r(File.join(cached_framework_dir, 'Flutter.framework'), copied_flutter_dir)
+    end
+    unless File.exist?(copied_podspec_path)


This is essentially a migration step since people may already have ios/Flutter/Flutter.framework but will definitely not have ios/Flutter/Flutter.podspec.

jmagman · 2019-10-05T00:38:03Z

packages/flutter_tools/templates/cocoapods/Podfile-ios-objc

-    File.symlink(p[:path], symlink)
-    pod p[:name], :path => File.join(symlink, 'ios')
-  }
+  plugin_pods.each do |name, path|


This is more idiomatic Ruby.

I'll take your word for it :)

codecov · 2019-10-05T01:32:13Z

Codecov Report

Merging #42029 into master will decrease coverage by 0.5%.
The diff coverage is 81.81%.

@@            Coverage Diff             @@
##           master   #42029      +/-   ##
==========================================
- Coverage   60.63%   60.13%   -0.51%     
==========================================
  Files         194      194              
  Lines       18886    18897      +11     
==========================================
- Hits        11452    11364      -88     
- Misses       7434     7533      +99

Flag	Coverage Δ
#flutter_tool	`60.13% <81.81%> (-0.51%)`	⬇️

Impacted Files	Coverage Δ
...ackages/flutter_tools/lib/src/macos/cocoapods.dart	`91.15% <100%> (+0.58%)`	⬆️
packages/flutter_tools/lib/src/project.dart	`87.5% <50%> (-0.47%)`	⬇️
...ges/flutter_tools/lib/src/commands/ide_config.dart	`30.39% <0%> (-53.93%)`	⬇️
...tter_tools/lib/src/build_system/targets/linux.dart	`38.29% <0%> (-46.81%)`	⬇️
...tools/lib/src/windows/visual_studio_validator.dart	`61.29% <0%> (-32.26%)`	⬇️
...utter_tools/lib/src/build_system/build_system.dart	`82.72% <0%> (-6.37%)`	⬇️
...ages/flutter_tools/lib/src/base/user_messages.dart	`47.74% <0%> (-3.61%)`	⬇️
packages/flutter_tools/lib/src/base/os.dart	`24.6% <0%> (-2.39%)`	⬇️
packages/flutter_tools/lib/src/version.dart	`93.23% <0%> (-1.94%)`	⬇️
packages/flutter_tools/lib/src/cache.dart	`48.47% <0%> (-0.71%)`	⬇️
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dda74a1...f3d5013. Read the comment docs.

jonahwilliams

What test are we missing that would have caught this? I suppose since the devicelab is building through flutter it wouldn't have tickled this bug at all?

jonahwilliams · 2019-10-05T01:33:15Z

packages/flutter_tools/templates/cocoapods/Podfile-ios-objc

+
+    generated_xcode_build_settings_path = File.join(copied_flutter_dir, 'Generated.xcconfig')
+    unless File.exist?(generated_xcode_build_settings_path)
+      raise "Generated.xcconfig must exist. If you're running pod install manually, make sure flutter pub get is executed first"


flutter packages get or are we rebranding to flutter pub get? Both probably work, not sure if we're consistent with one or the other.

🤷‍♀ I didn't change the text of this line, I made it a raise instead of a puts. I feel like I've seen flutter packages get in more places, so I'll change it.

Never mind, flutter packages get is only used in the Podfile-macos (which I didn't change in this PR because I want to test that thoroughly and separately). flutter pub get is used in 26 places in the repo.

jonahwilliams · 2019-10-05T01:33:35Z

packages/flutter_tools/templates/cocoapods/Podfile-ios-objc

-    File.symlink(p[:path], symlink)
-    pod p[:name], :path => File.join(symlink, 'ios')
-  }
+  plugin_pods.each do |name, path|


I'll take your word for it :)

jmagman · 2019-10-05T01:41:19Z

What test are we missing that would have caught this? I suppose since the devicelab is building through flutter it wouldn't have tickled this bug at all?

Something like:

flutter/dev/bots/deploy_gallery.sh

Line 56 in 054e887

flutter build ios --release --no-codesign -t lib/main_publish.dart

but builds with --debug that deploys (uses Release) and then does a smoke test of the archived app.

I'll think more about how to test this...

.gitignore

dnfield · 2019-10-05T02:18:28Z

For a test, couldn't we take one of the integration tests we have and assert that the .symlinks folder doesn't have Flutter in it?

genert · 2019-10-14T13:32:45Z

What is the status of this PR?

jmagman · 2019-10-14T23:58:35Z

For a test, couldn't we take one of the integration tests we have and assert that the .symlinks folder doesn't have Flutter in it?

I added a few location checks to deploy_gallery.

genert · 2019-10-15T07:28:30Z

@jonahwilliams @zanderso

…ation

…ation (flutter#42029)

jmagman added platform-ios iOS applications specifically tool Affects the "flutter" command-line tool. See also t: labels. t: xcode "xcodebuild" on iOS and general Xcode project management labels Oct 5, 2019

jmagman requested review from dnfield and jonahwilliams October 5, 2019 00:30

jmagman self-assigned this Oct 5, 2019

fluttergithubbot added d: examples Sample code and demos team Infra upgrades, team productivity, code health, technical debt. See also team: labels. customer: gallery Relating to flutter/gallery repository. Please transfer non-framework issues there. labels Oct 5, 2019

googlebot added the cla: yes label Oct 5, 2019

jmagman commented Oct 5, 2019

View reviewed changes

jonahwilliams reviewed Oct 5, 2019

View reviewed changes

dnfield reviewed Oct 5, 2019

View reviewed changes

.gitignore Outdated Show resolved Hide resolved

jmagman force-pushed the pod-path branch 2 times, most recently from 5671d55 to f886874 Compare October 7, 2019 22:58

jmagman mentioned this pull request Oct 10, 2019

Crash on archived iOS ipa with bitcode enabled (App Store install), Library not loaded: ./libFlutter.dylib #42458

Closed

jmagman force-pushed the pod-path branch from f4faa75 to d4195ed Compare October 14, 2019 23:55

dnfield approved these changes Oct 15, 2019

View reviewed changes

Always embed Flutter.framework build mode version from Xcode configur…

f3d5013

…ation

jmagman force-pushed the pod-path branch from d4195ed to f3d5013 Compare October 15, 2019 19:45

jmagman merged commit 357d02c into flutter:master Oct 15, 2019

jmagman deleted the pod-path branch October 15, 2019 22:34

jmagman mentioned this pull request Oct 16, 2019

Run flutter pub get before pod install in platform_view_ios__start_up test #42808

Merged

9 tasks

jmagman mentioned this pull request Oct 28, 2019

flutter build ios --release may use a debug Flutter.framework by mistake #37850

Closed

Inconnu08 pushed a commit to Inconnu08/flutter that referenced this pull request Nov 26, 2019

Always embed Flutter.framework build mode version from Xcode configur…

25a9383

…ation (flutter#42029)

This was referenced Dec 4, 2019

All Flutter Apps Run White Screen From Binary - Works As Expected Fine From Code/Debugger #44205

Closed

White Screen - App content failed to load after launch released iOS app #24887

Closed

This was referenced Jan 18, 2020

Last plugin pod is still embedded in an iOS app even after it is removed from the pubspec #11819

Open

Xcode Archive builds should automatically build in release mode #12774

Closed

jmagman mentioned this pull request Feb 26, 2020

Move embedding and linking Flutter frameworks into the tool #51453

Merged

10 tasks

This was referenced Mar 6, 2020

Flutter can't run for iOS but flutter build ios works fine #29834

Closed

BUILD FAILED : Flutter.framework: No such file or directory #47712

Closed

flutter clean doesn't clean everything #12631

Closed

jmagman mentioned this pull request Apr 7, 2020

xcode11 Archive and remind a error "IPA processing failed" #42917

Closed

jmagman mentioned this pull request Apr 23, 2020

Add PodFile.lock be added to the template gitignore #9299

Closed

jmagman mentioned this pull request May 20, 2020

Change release archive check to warning #57688

Merged

10 tasks

jmagman mentioned this pull request May 27, 2021

iPad running iOS 12 -- Stuck on Splash Screen #22765

Closed

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Always embed iOS Flutter.framework build mode version from Xcode configuration #42029

Always embed iOS Flutter.framework build mode version from Xcode configuration #42029

jmagman commented Oct 5, 2019 •

edited

Loading

jmagman Oct 5, 2019

jmagman Oct 5, 2019

jmagman Oct 5, 2019

jmagman Oct 5, 2019

jmagman Oct 5, 2019

dnfield Oct 5, 2019

jmagman Oct 5, 2019 •

edited

Loading

dnfield Oct 5, 2019

jmagman Oct 7, 2019

jmagman Oct 7, 2019

dnfield Oct 7, 2019

zanderso Oct 7, 2019

jmagman Oct 7, 2019 •

edited

Loading

jmagman Oct 7, 2019 •

edited

Loading

jmagman Oct 5, 2019

jmagman Oct 5, 2019

jmagman Oct 5, 2019

jmagman Oct 5, 2019

jmagman Oct 5, 2019

jmagman Oct 5, 2019

jonahwilliams Oct 5, 2019

codecov bot commented Oct 5, 2019 •

edited

Loading

jonahwilliams left a comment

jonahwilliams Oct 5, 2019

jmagman Oct 5, 2019

jmagman Oct 7, 2019

jmagman Oct 7, 2019

jonahwilliams Oct 5, 2019

jmagman commented Oct 5, 2019 •

edited

Loading

dnfield commented Oct 5, 2019

genert commented Oct 14, 2019

jmagman commented Oct 14, 2019

genert commented Oct 15, 2019

Always embed iOS Flutter.framework build mode version from Xcode configuration #42029

Always embed iOS Flutter.framework build mode version from Xcode configuration #42029

Conversation

jmagman commented Oct 5, 2019 • edited Loading

Description

Related Issues

Tests

Checklist

Breaking Change

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmagman Oct 5, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmagman Oct 7, 2019 • edited Loading

Choose a reason for hiding this comment

jmagman Oct 7, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Oct 5, 2019 • edited Loading

Codecov Report

jonahwilliams left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmagman commented Oct 5, 2019 • edited Loading

dnfield commented Oct 5, 2019

genert commented Oct 14, 2019

jmagman commented Oct 14, 2019

genert commented Oct 15, 2019

jmagman commented Oct 5, 2019 •

edited

Loading

jmagman Oct 5, 2019 •

edited

Loading

jmagman Oct 7, 2019 •

edited

Loading

jmagman Oct 7, 2019 •

edited

Loading

codecov bot commented Oct 5, 2019 •

edited

Loading

jmagman commented Oct 5, 2019 •

edited

Loading