Set MinimumOSVersion in iOS Info.plist based on buildroot setting #28743

jmagman · 2021-09-21T01:01:09Z

The Flutter.framework minimum version was bumped to 9.0 as of #28572 / flutter/buildroot#509. However, the framework's Info.plist was not updated, which causes App Store "Invalid bundle" submission failures.

Adopt argparse in copy_info_plist.py to make it easier to add new arguments to the script
Add --minversion to the script, to be used as the MinimumOSVersion value in the iOS Info.plist to keep the value in sync with the buildroot setting.
Add a test to validate the Info.plist values. Embed (but do not link) the Flutter.framework in the test so it can be parsed.

Follow up to #28783, would have also fixed flutter/flutter#90209

Pre-launch Checklist

I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
I read the [Tree Hygiene] wiki page, which explains my responsibilities.
I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides].
I listed at least one issue that this PR fixes in the description above.
I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See [testing the engine] for instructions on
writing and running engine tests.
I updated/added relevant documentation (doc comments with ///).
I signed the [CLA].
All existing and new tests are passing.

jmagman · 2021-09-22T00:07:22Z

testing/ios/IosUnitTests/IosUnitTests.xcodeproj/project.pbxproj

@@ -264,7 +267,6 @@
 			isa = PBXResourcesBuildPhase;
 			buildActionMask = 2147483647;
 			files = (
-				0D6AB73F22BD8F0200EEE540 /* FlutterEngineConfig.xcconfig in Resources */,


Not related to this change, but xcconfigs should not be embedded as resources in the app.

Good catch.

jmagman · 2021-09-22T00:09:23Z

#28783 is a more targeted fix. I will rebase on it when it lands so the App Store submission issue isn't regressed in case this needs to be reverted for some reason.

chinmaygarde

I don't think the test should be hardcoded for iOS 9.0. But otherwise mostly just nits.

chinmaygarde · 2021-09-22T01:14:05Z

testing/ios/IosUnitTests/IosUnitTests.xcodeproj/project.pbxproj

@@ -264,7 +267,6 @@
 			isa = PBXResourcesBuildPhase;
 			buildActionMask = 2147483647;
 			files = (
-				0D6AB73F22BD8F0200EEE540 /* FlutterEngineConfig.xcconfig in Resources */,


Good catch.

chinmaygarde · 2021-09-22T01:17:46Z

shell/platform/darwin/ios/framework/Source/FlutterEngineTest.mm

+- (void)testInfoPlist {
+  // Check the embedded Flutter.framework Info.plist, not the linked dylib.
+  NSURL* flutterFrameworkURL =
+      [NSBundle.mainBundle.privateFrameworksURL URLByAppendingPathComponent:@"Flutter.framework"];


Instead of attempting to resolve the bundle by path, [NSBundle bundleForClass:[FlutterEngine class]] might be less brittle. For instance, I am not sure why this is in the private frameworks URL (presumably because its in the test bundle but not sure).

I tried -[NSBundle bundleForClass:] first, unfortunately it doesn't work, the bundle is nil. Before this PR, the test does not link against/embed/load the Flutter.framework with the Info.plist. Instead, it embeds libios_test_flutter, which has the compiled Flutter and test code.

engine/shell/platform/darwin/ios/BUILD.gn

Line 232 in 0292339

"-Wl,-install_name,@rpath/Frameworks/libios_test_flutter.dylib",

engine/testing/ios/IosUnitTests/IosUnitTests.xcodeproj/project.pbxproj

Line 39 in 266435d

F7521D7826BB68BC005F15C5 /* libios_test_flutter.dylib in Embed Libraries */,

$ nm -g out/ios_debug_sim_unopt/libios_test_flutter.dylib | grep FlutterEngine 0000000003987fd8 S _OBJC_CLASS_$_FlutterEngine 0000000003988078 S _OBJC_CLASS_$_FlutterEngineGroup 0000000003987880 S _OBJC_CLASS_$_FlutterEngineGroupTest 0000000003987ba0 S _OBJC_CLASS_$_FlutterEnginePartialMock 00000000039878d0 S _OBJC_CLASS_$_FlutterEngineTest 0000000003988028 S _OBJC_METACLASS_$_FlutterEngine 00000000039880a0 S _OBJC_METACLASS_$_FlutterEngineGroup 0000000003987858 S _OBJC_METACLASS_$_FlutterEngineGroupTest 0000000003987b78 S _OBJC_METACLASS_$_FlutterEnginePartialMock 00000000039878a8 S _OBJC_METACLASS_$_FlutterEngineTest

But I want to test the Info.plist in the final bundle, so I embedded it in the app in this PR so I could leverage the NSBundle API. I'm open to suggestions for a better way to test this.

chinmaygarde · 2021-09-22T01:21:41Z

shell/platform/darwin/ios/framework/Source/FlutterEngineTest.mm

+  NSBundle* flutterBundle = [NSBundle bundleWithURL:flutterFrameworkURL];
+
+  NSDictionary<NSString*, id>* infoDictionary = flutterBundle.infoDictionary;
+  XCTAssertEqualObjects(infoDictionary[@"MinimumOSVersion"], @"9.0");


__ENVIRONMENT_IPHONE_OS_VERSION_MIN_REQUIRED__ maybe? Or, is the test harness not compiled with the same minimum OS version requirement? The next time we bump this up, we will need to update test too when all this test is doing is checking that what we specify in the GN rule ends up as verbatim in the Info.plist.

Or, is the test harness not compiled with the same minimum OS version requirement?

It's not, the test lib is built with ios_testing_deployment_target

engine/shell/platform/darwin/ios/BUILD.gn

Line 228 in 0292339

"-mios-simulator-version-min=$ios_testing_deployment_target",

And the test app is built with 12.2

engine/testing/ios/IosUnitTests/IosUnitTests.xcodeproj/project.pbxproj

Line 374 in 266435d

IPHONEOS_DEPLOYMENT_TARGET = 12.2;

The next time we bump this up, we will need to update test too when all this test is doing is checking that what we specify in the GN rule ends up as verbatim in the Info.plist.

Well, it's also testing that the copy_info_plist.py script works. I can just test that it's a well-formatted version string, if you prefer. Otherwise, you're right, the buildroot SHA update that includes the config change will also need to update this test in the same commit.

I can just test that it's a well-formatted version string, if you prefer.

Don't have too strong of an opinion on this but I'd definitely prefer components and tests not making assumptions about the version floor. Just checking for a well formatted string should be fine.

I replaced the check with a regex.

…ting (flutter/engine#28743)

* a1400b4 Roll Dart SDK from 6e17953f4e7e to ba343ec30c69 (1 revision) (flutter/engine#28807) * 22fe15a Set MinimumOSVersion in iOS Info.plist based on buildroot setting (flutter/engine#28743) * b872267 Roll Fuchsia Linux SDK from bWZcp2jnx... to tBL8VjITE... (flutter/engine#28804)

…utter#28743)

Ignoring the web framework test since the failure originates from a repo synchronization issue between engine and framework. The new failing tests were sync'ed in from head in the test and the tested code doesn't exist in the engine at the time of branch cut. * Fixes FlutterSemanticsScrollView to not implement accessibility container API (#28846) * add branch to test * Replace jcenter() for mavenCentral() (#28777) * Set MinimumOSVersion to iOS 9 in Info.plist (#28783) * Do not use jcenter(), switch to mavenCentral() (#28738) Co-authored-by: chunhtai <[email protected]> Co-authored-by: Emmanuel Garcia <[email protected]> Co-authored-by: Jenn Magder <[email protected]> Co-authored-by: Dan Field <[email protected]>

* a1400b4 Roll Dart SDK from 6e17953f4e7e to ba343ec30c69 (1 revision) (flutter/engine#28807) * 22fe15a Set MinimumOSVersion in iOS Info.plist based on buildroot setting (flutter/engine#28743) * b872267 Roll Fuchsia Linux SDK from bWZcp2jnx... to tBL8VjITE... (flutter/engine#28804)

jmagman added platform-ios platform-macos labels Sep 21, 2021

jmagman self-assigned this Sep 21, 2021

flutter-dashboard bot added the embedder Related to the embedder API label Sep 21, 2021

google-cla bot added the cla: yes label Sep 21, 2021

jmagman marked this pull request as ready for review September 21, 2021 07:16

jmagman force-pushed the ios-min branch from 6aae71d to 1e979a9 Compare September 21, 2021 18:12

jmagman mentioned this pull request Sep 21, 2021

Set MinimumOSVersion to iOS 9 in Info.plist #28783

Merged

8 tasks

jmagman force-pushed the ios-min branch from 4d06700 to 067d45c Compare September 21, 2021 23:43

jmagman commented Sep 22, 2021

View reviewed changes

jmagman requested review from dnfield and chinmaygarde September 22, 2021 00:17

chinmaygarde approved these changes Sep 22, 2021

View reviewed changes

dnfield approved these changes Sep 22, 2021

View reviewed changes

jmagman added 4 commits September 22, 2021 11:06

Set MinimumOSVersion in iOS Info.plist based on buildroot setting

94e567c

ul

78cfb65

Embed framework

b9dd2d9

Genericize version test

42650e6

jmagman force-pushed the ios-min branch from 067d45c to 42650e6 Compare September 22, 2021 18:42

jmagman added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Sep 22, 2021

fluttergithubbot merged commit 22fe15a into flutter:master Sep 22, 2021

engine-flutter-autoroll mentioned this pull request Sep 22, 2021

Roll Engine from dd28b6f1c8e6 to b872267e8ff7 (3 revisions) flutter/flutter#90558

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 22, 2021

22fe15a Set MinimumOSVersion in iOS Info.plist based on buildroot set…

5da6d8b

…ting (flutter/engine#28743)

engine-flutter-autoroll mentioned this pull request Sep 23, 2021

Roll Engine from dd28b6f1c8e6 to b872267e8ff7 (3 revisions) flutter/flutter#90567

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 23, 2021

22fe15a Set MinimumOSVersion in iOS Info.plist based on buildroot set…

6e734e4

…ting (flutter/engine#28743)

xster pushed a commit to xster/engine that referenced this pull request Sep 29, 2021

Set MinimumOSVersion in iOS Info.plist based on buildroot setting (fl…

c7ed135

…utter#28743)

xster mentioned this pull request Sep 29, 2021

Cherry-pick #28846 and #28743 into flutter-2.6-candidate.11 #28937

Merged

jmagman mentioned this pull request Oct 11, 2021

Build arm64 during Mac Host Engine build flutter/flutter#84453

Closed

This was referenced Apr 15, 2022

☂️ Increase minimum support iOS version from 9 to 11 flutter/flutter#101959

Closed

Set buildroot ios_deployment_target to 11.0 flutter/flutter#101964

Closed

jmagman mentioned this pull request Jul 14, 2022

Bump the min-deployment-target on macOS desktop targets to 10.13 flutter/flutter#107492

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Set MinimumOSVersion in iOS Info.plist based on buildroot setting #28743

Set MinimumOSVersion in iOS Info.plist based on buildroot setting #28743

Uh oh!

jmagman commented Sep 21, 2021 •

edited

Loading

Uh oh!

jmagman Sep 22, 2021

Uh oh!

chinmaygarde Sep 22, 2021

Uh oh!

jmagman commented Sep 22, 2021

Uh oh!

chinmaygarde left a comment

Uh oh!

chinmaygarde Sep 22, 2021

Uh oh!

chinmaygarde Sep 22, 2021

Uh oh!

jmagman Sep 22, 2021

Uh oh!

chinmaygarde Sep 22, 2021

Uh oh!

jmagman Sep 22, 2021

Uh oh!

chinmaygarde Sep 22, 2021

Uh oh!

jmagman Sep 22, 2021

Uh oh!

Uh oh!

Set MinimumOSVersion in iOS Info.plist based on buildroot setting #28743

Set MinimumOSVersion in iOS Info.plist based on buildroot setting #28743

Uh oh!

Conversation

jmagman commented Sep 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmagman commented Sep 22, 2021

Uh oh!

chinmaygarde left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jmagman commented Sep 21, 2021 •

edited

Loading