Require xcode_backend.sh arguments (introduced in Flutter v0.0.7) #65124

jmagman · 2020-09-02T21:48:08Z

Description

xcode_backend.sh supported pre-Flutter v0.0.7 created apps that were missing a post-compilation build phase. #64988 reland will require this build phase hook to change the bundled app's Info.plist.

Stop supporting the pre-v0.0.7 Xcode projects missing build phase. Generate a build error on the slim chance these still exist in the wild:

Related Issues

#7913
#64988

Tests

I added the following tests:

Update the benchmark integration test Xcode projects that were created pre-v0.0.7.
#64988 postsubmit failed on the devicelab:

2020-09-02T11:44:03.947711: stdout:             ♦ codesign --force --verbose --sign A5CFF7959BD4B17F97127C518E8504F2D9562490 -- /Users/flutter/.cocoon/flutter/dev/benchmarks/complex_layout/build/ios/Profile-iphoneos/Runner.app/Frameworks/Flutter.framework/Flutter
stdout:             /Users/flutter/.cocoon/flutter/dev/benchmarks/complex_layout/build/ios/Profile-iphoneos/Runner.app/Frameworks/Flutter.framework/Flutter: signed bundle with Mach-O universal (armv7 x86_64 arm64) [io.flutter.flutter]
stdout:             /Users/flutter/.cocoon/flutter/dev/benchmarks/complex_layout/build/ios/Profile-iphoneos/Runner.app/Info.plist: file does not exist or is not readable or is not a regular file (Error Domain=NSCocoaErrorDomain Code=260 "The file “Info.plist” couldn’t be opened because there is no such file." UserInfo={NSFilePath=/Users/flutter/.cocoon/flutter/dev/benchmarks/complex_layout/build/ios/Profile-iphoneos/Runner.app/Info.plist, NSUnderlyingError=0x7fe71440c460 {Error Domain=NSPOSIXErrorDomain Code=2 "No such file or directory"}})
stdout:             ♦ plutil -insert NSBonjourServices -json ["_dartobservatory._tcp"] /Users/flutter/.cocoon/flutter/dev/benchmarks/complex_layout/build/ios/Profile-iphoneos/Runner.app/Info.plist
stdout:             /Users/flutter/.cocoon/flutter/dev/benchmarks/complex_layout/build/ios/Profile-iphoneos/Runner.app/Info.plist: file does not exist or is not readable or is not a regular file (Error Domain=NSCocoaErrorDomain Code=260 "The file “Info.plist” couldn’t be opened because there is no such file." UserInfo={NSFilePath=/Users/flutter/.cocoon/flutter/dev/benchmarks/complex_layout/build/ios/Profile-iphoneos/Runner.app/Info.plist, NSUnderlyingError=0x7fc1a7c0ce70 {Error Domain=NSPOSIXErrorDomain Code=2 "No such file or directory"}})
stdout:             Command PhaseScriptExecution failed with a nonzero exit code

These tests now pass.

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 read the [Tree Hygiene] wiki page, which explains my responsibilities.
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

No, no existing tests failed, so this is not a breaking change.
Yes, this is a breaking change. If not, delete the remainder of this section.

jmagman · 2020-09-02T21:49:04Z

packages/flutter_tools/bin/xcode_backend.sh

+  # Named entry points were introduced in Flutter v0.0.7.
+  echo "error: Your Xcode project is incompatible with this version of Flutter. Run \"rm -rf ios/Runner.xcodeproj\" and \"flutter create .\" to regenerate."
+  exit -1


This is the actual change. The rest of the PR updates the benchmark Xcode projects.

jmagman · 2020-09-02T21:51:27Z

dev/benchmarks/complex_layout/ios/Runner.xcodeproj/project.pbxproj

 			);
 			runOnlyForDeploymentPostprocessing = 0;
 			shellPath = /bin/sh;
-			shellScript = "/bin/sh $FLUTTER_ROOT/packages/flutter_tools/bin/xcode_backend.sh";


This was what caused the devicelab errors on #64988 post-submit. xcode_backend.sh without any args was building, thinning, and embedding. However, at this point the generated Info.plist that we will need to edit for #64988 doesn't exist, so it was failing.

dnfield

LGTM

jonahwilliams

LGTM

xster

LGTM

xster · 2020-09-02T22:48:48Z

packages/flutter_tools/bin/xcode_backend.sh

-  BuildApp
-  EmbedFlutterFrameworks
+  # Named entry points were introduced in Flutter v0.0.7.
+  EchoError "error: Your Xcode project is incompatible with this version of Flutter. Run \"rm -rf ios/Runner.xcodeproj\" and \"flutter create .\" to regenerate."


ultra nit / optional: "Consider source controlling your project before regenerating to inspect the difference."

jmagman · 2020-09-03T01:32:30Z

Boo, missed macrobenchmarks...

jmagman · 2020-09-03T01:47:01Z

Boo, missed macrobenchmarks...

Never mind these tests are flaky.

…utter#65124)

liyuqian · 2020-09-16T18:33:14Z

Some compile time regression is detected after this commit. I'd like to confirm that this is expected due to build changes. I'd also like to know a little more about why it's increase significantly, and if there's any hope to make the compile a little faster. (The compile time became even slower when I introduced e2e in #65602). CC @jmagman @kf6gpe

jmagman · 2020-09-16T19:41:20Z

Some compile time regression is detected after this commit. I'd like to confirm that this is expected due to build changes. I'd also like to know a little more about why it's increase significantly, and if there's any hope to make the compile a little faster. (The compile time became even slower when I introduced e2e in #65602). CC @jmagman @kf6gpe

@liyuqian Before this PR the benchmarks were not thinning the Flutter frameworks to the targeted architecture because the Xcode projects were created (or copied from) pre-v0.0.7. https://github.com/flutter/flutter/pull/65124/files#diff-0d00f38df3883291f23cee8074ac075dL319-L320
They were calling:

BuildApp
EmbedFlutterFrameworks

instead of:

BuildApp
EmbedFlutterFrameworks
ThinAppFrameworks

The test benchmark app being transferred to the device should be much smaller now, which is a tradeoff we make in exchange for build time. I'd have to look at the logs before and after to confirm that's the cause. However as long as there isn't a compilation regression in a newly created/post v1.0 Flutter app, I'm not sure it's worth investigating too closely...

liyuqian · 2020-09-16T21:20:19Z

Sounds good. Thanks for the detailed explanation! We can move on as is.

Require xcode_backend.sh arguments (introduced in Flutter v0.0.7)

7cb6d07

jmagman added 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. t: xcode "xcodebuild" on iOS and general Xcode project management labels Sep 2, 2020

jmagman self-assigned this Sep 2, 2020

googlebot added the cla: yes label Sep 2, 2020

jmagman requested review from dnfield and xster September 2, 2020 21:48

jmagman commented Sep 2, 2020

View reviewed changes

jmagman added 2 commits September 2, 2020 15:16

Add license headers

0c66db0

Pipe error to stderr instead of stdout

79bb0ae

dnfield approved these changes Sep 2, 2020

View reviewed changes

jonahwilliams approved these changes Sep 2, 2020

View reviewed changes

xster reviewed Sep 2, 2020

View reviewed changes

jmagman added the waiting for tree to go green label Sep 2, 2020

jmagman merged commit 2e67bf2 into flutter:master Sep 2, 2020

jmagman deleted the xcodebackend-arg branch September 2, 2020 23:33

This was referenced Sep 3, 2020

Add observatory Bonjour service to built iOS Info.plist bundle #65138

Merged

Revert "Add observatory Bonjour service to built iOS Info.plist bundle" #65141

Merged

mingwandroid pushed a commit to mingwandroid/flutter that referenced this pull request Sep 6, 2020

Require xcode_backend.sh arguments (introduced in Flutter v0.0.7) (fl…

5a02136

…utter#65124)

liyuqian mentioned this pull request Sep 11, 2020

Reland "perf test for measuring scroll smoothness" #65602

Merged

Require xcode_backend.sh arguments (introduced in Flutter v0.0.7) #65124

Require xcode_backend.sh arguments (introduced in Flutter v0.0.7) #65124

Uh oh!

Conversation

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

Uh oh!

Description

Related Issues

Tests

Checklist

Breaking Change

Uh oh!

jmagman Sep 2, 2020

Choose a reason for hiding this comment

Uh oh!

jonahwilliams Sep 2, 2020

Choose a reason for hiding this comment

Uh oh!

jmagman Sep 2, 2020

Choose a reason for hiding this comment

Uh oh!

dnfield left a comment

Choose a reason for hiding this comment

Uh oh!

jonahwilliams left a comment

Choose a reason for hiding this comment

Uh oh!

xster left a comment

Choose a reason for hiding this comment

Uh oh!

xster Sep 2, 2020

Choose a reason for hiding this comment

Uh oh!

jmagman commented Sep 3, 2020

Uh oh!

jmagman commented Sep 3, 2020

Uh oh!

liyuqian commented Sep 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

liyuqian commented Sep 16, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

jmagman commented Sep 2, 2020 •

edited

Loading

liyuqian commented Sep 16, 2020 •

edited

Loading

jmagman commented Sep 16, 2020 •

edited

Loading