8000 Disable CocoaPods input and output paths in Xcode build phase and adopt new Xcode build system by jmagman · Pull Request #33684 · flutter/flutter · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Disable CocoaPods input and output paths in Xcode build phase and adopt new Xcode build system #33684

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 3, 2019

Conversation

jmagman
Copy link
Member
@jmagman jmagman commented May 31, 2019

Description

Previously the Xcode new build system could not be adopted due to two Xcode project build phases both outputting the Flutter.framework (one for Flutter w/o packages and the other from CocoaPods). The legacy build system was more lenient about duplicate outputs. See #20685 (comment) for a good description of the problem.

This change updates the Podfile templates to use the CocoaPod disable_input_output_paths installation option, which prevents the [CP] Embed Pods Frameworks build phase from outputting the Flutter.framework files. This has the unfortunate side-effect of also preventing any input files, which means the Pods-Runner-frameworks.sh script will run on every incremental build.

This change also removes the workaround from 231784e opting into the Xcode legacy build system.

Related Issues

Fixes #20685.

Tests

  1. Updated the CocoaPods version tests to check for the new minimum required version where disable_input_output_paths was introduced.
  2. Added a check to ensure the WorkspaceSettings file is no longer generated.

I manually tested the flutter create flow and confirmed the generated Xcode project builds. Then I added packages and confirmed that Xcode project builds.

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.

@jonahwilliams
Copy link
Contributor

I noticed that this also required changes to some of the existing generated config files, how will this affect apps which have not been updated?

  • Do we also need to update the templates so that new applications are set up correctly?

@jmagman jmagman changed the title Disable Cocoapods input and output paths in Xcode build phase and adopt new Xcode build system Disable CocoaPods input and output paths in Xcode build phase and adopt new Xcode build system May 31, 2019
@jmagman
Copy link
Member Author
jmagman commented May 31, 2019

I noticed that this also required changes to some of the existing generated config files, how will this affect apps which have not been updated?

Existing apps that are currently building using the legacy build system will continue to use the legacy build system. No updates will be needed from developers. This will only impact newly generated apps. I updated the examples so no one would crack open those Xcode projects and notice the legacy build system and get confused.

I am reluctant to migrate the build system under an existing app since that build system is stricter, and we could be exposing new compilation issues. It's definitely more risky, but if you think it's worth it @jonahwilliams and @cbracken, I can try. It might be annoying to developers that they updated flutter, then have to track down new Xcode compilation errors when their .dart code didn't change (though I suppose that probably exact scenario happens nearly every time Xcode is updated).

  • Do we also need to update the templates so that new applications are set up correctly?

I updated the Podfile templates so new applications are set up correctly. I tested by flutter create app then flutter build ios and made sure the Xcode workspace builds. Then I added a package, rebuilt, and confirmed the Xcode workspace builds, and that the [CP] Embed Pods Frameworks build phase does not have input or output files.

Is there another template I'm missing?

@jonahwilliams
Copy link
Contributor

Is there another template I'm missing?

Ahh, no that's fine - I just missed it

@jonahwilliams
Copy link
Contributor

. This has the unfortunate side-effect of also preventing any input files, which means the Pods-Runner-frameworks.sh script will run on every incremental build.

Is there any path forward here?

@goderbauer goderbauer added the tool Affects the "flutter" command-line tool. See also t: labels. label May 31, 2019
@jmagman
Copy link
Member Author
jmagman commented May 31, 2019

. This has the unfortunate side-effect of also preventing any input files, which means the Pods-Runner-frameworks.sh script will run on every incremental build.

Is there any path forward here?

Not really... @cbracken and I discussed this and we think it's an okay medium-term solution, and a trade-off that's worth getting off the legacy build system in case Xcode pulls the plug.

From #20685 (comment)

If we get to the point where we're seeing plugin Cocoapods that are really expensive to compile, we can deal with that as an optimisation.

@jmagman jmagman added t: xcode "xcodebuild" on iOS and general Xcode project management platform-ios iOS applications specifically labels May 31, 2019
@jmagman jmagman force-pushed the xcode-build-system branch from 742eb0f to 9292630 Compare May 31, 2019 23:46
Copy link
Contributor
@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I confirmed that the macOS plugin flow works with the added line (and that it eliminates the strident warning in the Xcode UI about duplicate output files for the current macOS runner).

@jmagman jmagman merged commit ef792fc into flutter:master Jun 3, 2019
@jmagman jmagman deleted the xcode-build-system branch June 3, 2019 23:12
jmagman added a commit that referenced this pull request Jun 4, 2019
… and adopt new Xcode build system (#33684)"

This reverts commit ef792fc.
@jmagman
Copy link
Member Author
jmagman commented Jun 4, 2019

This change increased the minimum required version of CocoaPods, which caused a build failure on macOS slaves running an older version. I reverted this change with 9734f4e while the new version requirement configuration propagates through the CI system.
I will un-revert tomorrow.

@jmagman
Copy link
Member Author
jmagman commented Jun 4, 2019

Un-reverted with d053fe5.

kiku-jw pushed a commit to kiku-jw/flutter that referenced this pull request Jun 14, 2019
…pt new Xcode build system (flutter#33684)

Updates the Podfile template to use the CocoaPod disable_input_output_paths installation option which prevents the [CP] Embed Pods Frameworks build phase from outputting the Flutter.framework files.
kiku-jw pushed a commit to kiku-jw/flutter that referenced this pull request Jun 14, 2019
… and adopt new Xcode build system (flutter#33684)"

This reverts commit ef792fc.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
platform-ios iOS applications specifically t: xcode "xcodebuild" on iOS and general Xcode project management tool Affects the "flutter" command-line tool. See also t: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Multiple commands produce '/build/ios/Debug-iphonesimulator/Runner.app/Frameworks/Flutter.framework
5 participants
0