Move iOS Podfile logic into tool by jmagman · Pull Request #59044 · flutter/flutter

jmagman · 2020-06-09T03:16:07Z

Description

Move Podfile logic into the tool so changes can be easily made without the user needing to migrate or regenerate their Podfile.

Related Issues

Fixes #45197
Will make #39659 easier

Tests

Add checks to plugin_lint_mac.

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.

fluttergithubbot · 2020-06-09T18:54:13Z

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

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

jmagman · 2020-06-09T18:59:00Z

The upcoming fix for #39659 will prompt existing projects to regenerate their Podfile like https://github.com/flutter/flutter/pull/42029/files#diff-dd627ef56fd3ab20096b1ac40467cf30R346.

jmagman · 2020-06-09T19:00:48Z

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

-  generated_key_values
+def ios_application_path
+  ios_application_path = File.dirname(defined_in_file.realpath) if self.respond_to?(:defined_in_file)
+  ios_application_path ||= File.dirname(File.realpath(__FILE__))


Avoid __dir__, I used it in add-to-app version of this and people have had issues.

jmagman · 2020-06-09T19:01:59Z

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

-  end
-  generated_key_values
+def ios_application_path
+  ios_application_path = File.dirname(defined_in_file.realpath) if self.respond_to?(:defined_in_file)


Hopefully CocoaPods never removes defined_in_file (the path to the Podfile) but if they do, fallback to... this file.

should we log a warning if it doesn't repond to defined_in_file?

I was sensitive about this due to #42513 but we know the location of the Podfile--it's this file.
I think I'm over-engineering this a bit. I just need to pass in this file. We can keep the fallback to defined_in_file and subsequent raise in the podhelper.

jmagman · 2020-06-09T19:02:51Z

packages/flutter_tools/bin/podhelper.rb

+# end
+# @param [XCBuildConfiguration] build_configuration Build configuration for Pod target.
+def flutter_additional_ios_build_settings(build_configuration)
+  build_configuration.build_settings['ENABLE_BITCODE'] = 'NO'


This gives us flexibility to support bitcode (someday).

jmagman · 2020-06-09T19:03:40Z

packages/flutter_tools/bin/podhelper.rb

+  end
+end
+
+def flutter_parse_plugins_file(file, separator='=')


This logic will be updated to do #39659.

christopherfujino · 2020-06-09T19:37:37Z

dev/integration_tests/release_smoke_test/ios/Podfile

+    matches = line.match(/FLUTTER_ROOT\=(.*)/)
+    return matches[1].strip if matches
+  end
+  raise 'FLUTTER_ROOT not found in Generated.xcconfig. Try deleting Generated.xcconfig, then run flutter pub get'


nit. including generated_xcode_build_settings_path in the message would be helpful

Changed it to include the path

[!] Invalid `Podfile` file: /Users/m/Projects/test_ruby/ios/Flutter/Generated.xcconfig must exist. If you're running pod install manually, make sure flutter pub get is executed first.

and

[!] Invalid `Podfile` file: FLUTTER_ROOT not found in /Users/m/Projects/test_ruby/ios/Flutter/Generated.xcconfig. Try deleting Generated.xcconfig, then run flutter pub get.

xster · 2020-06-09T20:28:43Z

dev/integration_tests/flutter_gallery/ios/Podfile

+end

-  # Plugin Pods
+require File.expand_path(File.join('packages', 'flutter_tools', 'bin', 'podhelper'), flutter_root)


xster · 2020-06-09T20:42:42Z

packages/flutter_tools/bin/podhelper.rb

@@ -0,0 +1,126 @@
+# Copyright 2014 The Flutter Authors. All rights reserved.


what's the transition plan for add-to-app? Can we change this line in https://flutter.dev/docs/development/add-to-app/ios/project-setup#option-a---embed-with-cocoapods-and-the-flutter-sdk to load File.join(ENV["FLUTTER_ROOT"], '.ios', 'Flutter', 'podhelper.rb') or some such?

Currently the CocoaPods add-to-app module lays down a similar but not quite the same podhelper in the ephemeral directory, which gets recreated on a flutter clean. As long as we didn't rename those methods and break existing host Podfiles (unfortunately the names don't reference iOS or the module: install_all_flutter_pods, install_flutter_engine_pod, install_flutter_plugin_pods) we could transition them to this bin podhelper, but I don't know what it would really buy us (other than they would get updates instantly instead on a clean). I'm concerned it would make the CI story more complicated because flutter would need to be available on every machine, even if the entire module and podspecs and binaries were already placed in the right spot.

P.S. We wouldn't want them to need FLUTTER_ROOT on their path, it would have to be parsed from the Generated files the same way this one is.

ya, it should be a separate task.

What I had in mind actually may or may not work. One small ergonomic annoyance now is if someone checks out the iOS and flutter projects, it currently takes 3 steps to build again. Go to flutter, flutter pub get to make the .ios folder with podhelper.rb, then go to iOS, call pod install, then open xcode, run.

Perhaps we can save one step if the various flutter_install_* code called flutter pub get to make the underlying xcodeproj if needed etc?

That would be cool. Want to create a new issue?

jmagman · 2020-06-09T21:05:55Z

packages/flutter_tools/bin/podhelper.rb

+# target 'Runner' do
+# ...
+# end
+def flutter_ios_podfile_setup


Added this hook for future use (could have been used for the old install! 'cocoapods', :disable_input_output_paths => true, for example)

jmagman · 2020-06-10T18:43:29Z

packages/flutter_tools/bin/podhelper.rb

+#   end
+# end
+# @param [PBXAggregateTarget] target Pod target.
+def flutter_additional_ios_build_settings(target)


One more change to make more future proof: pass in the target instead of the build configuration. This will give us access to the target name so in the future we can just make changes to the Flutter-specific targets (like adding a Flutter linker flag).

jmagman added platform-ios iOS applications specifically tool Affects the "flutter" command-line tool. See also t: labels. labels Jun 9, 2020

jmagman self-assigned this Jun 9, 2020

fluttergithubbot added c: contributor-productivity Team-specific productivity, code health, technical debt. work in progress; do not review labels Jun 9, 2020

googlebot added the cla: yes label Jun 9, 2020

Move Podfile logic into tool

71a42e4

jmagman force-pushed the podhelper branch from 2b76087 to 71a42e4 Compare June 9, 2020 18:42

jmagman removed the work in progress; do not review label Jun 9, 2020

jmagman marked this pull request as ready for review June 9, 2020 18:54

jmagman requested review from christopherfujino and cyanglaz June 9, 2020 18:55

jmagman commented Jun 9, 2020

View reviewed changes

Strip match

5aca0eb

christopherfujino reviewed Jun 9, 2020

View reviewed changes

Add path to exception

4958ec2

jmagman force-pushed the podhelper branch from 4da5455 to 4958ec2 Compare June 9, 2020 20:24

xster approved these changes Jun 9, 2020

View reviewed changes

Add flutter_ios_podfile_setup hook, drop defined_in_file

40b21e5

jmagman commented Jun 9, 2020

View reviewed changes

Extra )

d735dee

jmagman added this to the 1.21 - July 2020 milestone Jun 9, 2020

jmagman added 2 commits June 9, 2020 16:56

Fix [] bug, add min ruby version comment

381dd6d

Pass in target instead of build configuration to podhelper

30013a0

jmagman commented Jun 10, 2020

View reviewed changes

jmagman added waiting for tree to go green and removed waiting for tree to go green labels Jun 10, 2020

jmagman changed the title ~~Move Podfile logic into tool~~ Move iOS Podfile logic into tool Jun 10, 2020

jmagman merged commit 319a61f into flutter:master Jun 10, 2020

jmagman deleted the podhelper branch June 10, 2020 21:12

This was referenced Jun 10, 2020

Add iOS Podfile migration warning to support federated plugins #59201

Merged

Downgrading from master to stable, flutter run fails, Invalid Podfile file: cannot load such file - /flutter/packages/flutter_tools/bin/podhelper #59522

Closed

zljj0818 pushed a commit to zljj0818/flutter that referenced this pull request Jun 22, 2020

Move iOS Podfile logic into tool (flutter#59044)

5c723c2

This was referenced Aug 6, 2020

Warning: Podfile is out of date #60054

Closed

[1.20] Warning: Podfile is out of date - error: no such module X #63018

Closed

Move logic from user project's Podfile to a Ruby script in Flutter tools #45197

Closed

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

Move iOS Podfile logic into tool (flutter#59044)

f75024d

jmagman mentioned this pull request Sep 18, 2020

Plugin target deployments are out of range, causes Xcode warning #40109

Closed

brunobowden mentioned this pull request Sep 27, 2020

[connectivity] No Location Permission preprocessor flag flutter/plugins#2771

Closed

13 tasks

jmagman mentioned this pull request Nov 18, 2020

Force regeneration of old Podfile #70735

Merged

This was referenced Dec 8, 2020

Move embedding and linking macOS Flutter frameworks into the tool #71764

Merged

Move macOS Podfile logic into the tool #72020

Merged

jmagman mentioned this pull request Mar 23, 2021

Update Podfile.lock modified timestamp when pod install is run #78823

Merged

stuartmorgan-g mentioned this pull request Aug 9, 2023

Plugins don't use --local-engine framework on macOS/iOS #132228

Open

		@@ -0,0 +1,126 @@
		# Copyright 2014 The Flutter Authors. All rights reserved.

Conversation

jmagman commented Jun 9, 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!

fluttergithubbot commented Jun 9, 2020

Uh oh!

jmagman commented Jun 9, 2020

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!

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

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!

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

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jmagman commented Jun 9, 2020 •

edited

Loading

jmagman Jun 9, 2020 •

edited

Loading

jmagman Jun 9, 2020 •

edited

Loading