Rerun pod install on changed Xcode project, Podfile #17274

cbracken · 2018-05-04T01:16:39Z

If the developer changes their Xcode build settings and their project
has plugins, pod install is required, (e.g. to pick up changes to the
target architecture).

Similarly, manual edits to the Podfile should trigger a pod install.

cbracken · 2018-05-04T01:21:49Z

@mravn-google the parameter name change from flutterPodChanged to dependenciesChanged likely requires a patch to your tests in #17210.

It may also eliminate the need for the timestamp check on Podfile in your patch since we're now checking the contents. It may be worth keeping out of paranoia, though. If a fingerprint were ever written after a failed pod install, it would prevent reinstall -- that shouldn't happen with the current code since failure to pod install results in throwToolExit today.

xster

LGTM

xster · 2018-05-04T01:20:33Z

packages/flutter_tools/lib/src/ios/mac.dart

can we add some dartdocs to fingerprint.dart or tweak the name? I couldn't figure out what this does. Feel free to do in a different PR

You answered this one

Will do. The intent for properties is to be build options/flags (e.g. build mode, target platform, or anything other than file contents that's user-specified and affects the build) it's marked as @required in Fingerprinter because in almost every case this is something we don't want to forget, but in this case there's not much to put in there.

xster · 2018-05-04T01:22:59Z

packages/flutter_tools/lib/src/ios/mac.dart

Is it possible that the user doesn't even have pod installed, runs processPods, dumps an error message but goes through, writes a fingerprint. The user installs pods but it never runs again because the fingerprint matches?

Maybe a question for @mravn-google, should we check for cocoapods being installed during inject plugins before it gets here? I don't have a strong opinion either way.

I've added a boolean return value from processPods that indicates whether we actually ran pod install or not, and only write if so.

cbracken · 2018-05-04T01:30:03Z

Related #2089

If the developer changes their Xcode build settings and their project has plugins, pod install is required, (e.g. to pick up changes to the target architecture). Similarly, manual edits to the Podfile should trigger a pod install.

cbracken · 2018-05-04T02:41:24Z

Landed -- @mravn-google please hack up/tweak/fix/revert as necessary if this causes problems with your change.

mravn-google

LGTM

mravn-google · 2018-05-04T06:37:17Z

packages/flutter_tools/lib/src/ios/mac.dart

+    // If the Xcode project, Podfile, or Generated.xcconfig have changed since
+    // last run, pods should be updated.
+    final Fingerprinter fingerprinter = new Fingerprinter(
+      fingerprintPath: fs.path.join(getIosBuildDirectory(), 'pod_inputs.fingerprint'),


The fingerprint file should probably be .gitignored in our project templates. Maybe just add a *.fingerprint pattern at the root, and have the Fingerprinter class help by enforcing the use of the .fingerprint extension.

Nevermind. I see now that the fingerprint file has been placed in the build folder.

If the developer changes their Xcode build settings and their project has plugins, pod install is required, (e.g. to pick up changes to the target architecture). Similarly, manual edits to the Podfile should trigger a pod install.

git log 2d42c74..d7ad50c --first-parent --oneline d7ad50c Roll src/third_party/skia c97c90072499..6cb8168c2b9b (1 commits) (flutter#17287) 704a869 Roll src/third_party/dart cbd67124d10e..8246621d4da3 (39 commits) (flutter#17286) 33895cf Roll src/third_party/skia d1988219065a..c97c90072499 (2 commits) (flutter#17285) 48d1165 Roll fuchsia/sdk/core/linux-amd64 from h8m0G... to y2Piw... (flutter#17283) 0015b19 Roll fuchsia/sdk/core/mac-amd64 from WaNBa... to gkIC0... (flutter#17282) 518c9e0 Remove checks for the always true using_fuchsia_sdk flag in all GN files. (flutter#17261) 1eea062 Roll src/third_party/skia 3c358420df4a..d1988219065a (3 commits) (flutter#17279) ad8ccf4 [Fuchsia] Move physical shape layer compositing to Flutter (flutter#17005) c490cb9 Roll src/third_party/skia 165b68ebb6ad..3c358420df4a (5 commits) (flutter#17277) 4c41b14 [web] Introduce js interop to enable experimental flags on web (flutter#17099) 21f5d7f Roll src/third_party/skia 4098719375d8..165b68ebb6ad (7 commits) (flutter#17274) 6fef0d0 Removed a text input trait that causes VoiceOver to be incorrect when tapping a text input. (flutter#17203) 9343c5e added missing mock to MockEngine for iOS unit tests (flutter#17273) af056a8 Roll src/third_party/skia 6f60a859c628..4098719375d8 (6 commits) (flutter#17271) 33a21d1 Add DartProject for Windows embedding API (flutter#17210) 8a8b298 Added a log message when sharing a FlutterEngine across multiple FlutterViewControllers. (flutter#17186) 11f5521 Roll src/third_party/skia f04f21a5c913..6f60a859c628 (2 commits) (flutter#17268) da9e778 Roll fuchsia/sdk/core/linux-amd64 from k4zbT... to h8m0G... (flutter#17264) c86a4cc Roll fuchsia/sdk/core/mac-amd64 from bgp6U... to WaNBa... (flutter#17263) 35e8303 Roll src/third_party/skia 538e358b0d82..f04f21a5c913 (2 commits) (flutter#17260) e56b335 Allow external texture sources when using the Metal backend. (flutter#17154) c0deb23 Don’t depend on an implicit transaction when no external view embedder is present. (flutter#17258) 80846b0 Roll src/third_party/dart b7455646f90d..cbd67124d10e (3 commits) (flutter#17257) 9c64029 Documentation cleanups to RuntimeController. (flutter#17256) 209e786 Roll fuchsia/sdk/core/linux-amd64 from kGu6P... to k4zbT... (flutter#17255) aa26106 Roll fuchsia/sdk/core/mac-amd64 from nmCXC... to bgp6U... (flutter#17254) eec7447 Roll src/third_party/dart bdf14543576a..b7455646f90d (1 commits) (flutter#17253) ba26ce3 Roll src/third_party/dart 861708483217..bdf14543576a (5 commits) (flutter#17252) c9a24af Roll src/third_party/skia cf3594b60362..538e358b0d82 (4 commits) (flutter#17251) 59406cc Roll src/third_party/skia e41fa2d5e44d..cf3594b60362 (1 commits) (flutter#17249) 7d21f97 Roll src/third_party/dart f247613737ec..861708483217 (1 commits) (flutter#17248) db02fa1 Roll fuchsia/sdk/core/linux-amd64 from pTJOm... to kGu6P... (flutter#17247) f5314ea Roll fuchsia/sdk/core/mac-amd64 from uyg4y... to nmCXC... (flutter#17246) 31c4727 Document flutter::RuntimeController. (flutter#17250) 7b9a678 Roll src/third_party/dart 82f8a5451fae..f247613737ec (3 commits) (flutter#17245) 3dd688c Roll src/third_party/dart ba8baa46b452..82f8a5451fae (40 commits) (flutter#17244) cafa097 Revert "Roll src/third_party/dart ba8baa46b452..fbd0c8a46813 (35 commits) (flutter#17241)" (flutter#17243) 2f769be Soften shadows (flutter#17231) 48902d1 Felt add integration (flutter#17190) 047c03b Roll src/third_party/dart ba8baa46b452..fbd0c8a46813 (35 commits) (flutter#17241) f5d25da Reland: Implement unobstructed Platform Views on iOS (flutter#17237) d79170f Roll src/third_party/skia 933926683991..e41fa2d5e44d (2 commits) (flutter#17240) 46cc35f Roll fuchsia/sdk/core/mac-amd64 from MKYwg... to uyg4y... (flutter#17234) 060cbc4 Revert "Roll src/third_party/dart ba8baa46b452..0296286c03f6 (11 commits) (flutter#17226)" (flutter#17239) 1152709 Roll fuchsia/sdk/core/linux-amd64 from ZmlfS... to pTJOm... (flutter#17235) 47ae84b Run web framework tests on third-party rolls (flutter#17238) e2ec94b Roll src/third_party/skia 7b7b78cb9f1b..933926683991 (6 commits) (flutter#17232) f58967f Re-land "Enable unified OpenGL/Metal builds." (flutter#17230) 70f6d18 Revert "Implement unobstructed Platform Views on iOS (flutter#17049)" (flutter#17233) bba1a3c Roll buildroot to a0fb98a (flutter#17229) 2627634 Implement unobstructed Platform Views on iOS (flutter#17049) 49f8dba Sync Cirrus web test config from flutter/flutter (flutter#17228) fdf4f78 Roll src/third_party/skia 85755f46a881..7b7b78cb9f1b (6 commits) (flutter#17227) 6c757f3 Roll src/third_party/dart ba8baa46b452..0296286c03f6 (11 commits) (flutter#17226)

cbracken requested review from mravn-google and xster May 4, 2018 01:16

googlebot added the cla: yes label May 4, 2018

xster approved these changes May 4, 2018

View reviewed changes

Rerun pod install on changed Xcode project, Podfile

61296fb

If the developer changes their Xcode build settings and their project has plugins, pod install is required, (e.g. to pick up changes to the target architecture). Similarly, manual edits to the Podfile should trigger a pod install.

cbracken force-pushed the pod-install-on-xcode-project-changes branch from 5fee8ac to 61296fb Compare May 4, 2018 01:49

cbracken merged commit cdbdafa into flutter:master May 4, 2018

cbracken deleted the pod-install-on-xcode-project-changes branch May 4, 2018 02:40

mravn-google reviewed May 4, 2018

View reviewed changes

cbracken mentioned this pull request May 25, 2018

Running with --local-engine doesn't trigger Podfile.lock update #10183

Closed

gaaclarke mentioned this pull request Mar 24, 2020

Roll engine 2d42c74a348d..d7ad50c96f05 (54 commits) #53198

Closed

gaaclarke mentioned this pull request Mar 24, 2020

Roll engine 2d42c74a348d..d7ad50c96f05 (54 commits) #53200

Merged

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

Rerun pod install on changed Xcode project, Podfile #17274

Rerun pod install on changed Xcode project, Podfile #17274

Uh oh!

Conversation

cbracken commented May 4, 2018

Uh oh!

cbracken commented May 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xster left a comment

Choose a reason for hiding this comment

Uh oh!

xster May 4, 2018

Choose a reason for hiding this comment

Uh oh!

xster May 4, 2018

Choose a reason for hiding this comment

Uh oh!

cbracken May 4, 2018 • 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!

xster May 4, 2018

Choose a reason for hiding this comment

Uh oh!

xster May 4, 2018

Choose a reason for hiding this comment

Uh oh!

cbracken May 4, 2018

Choose a reason for hiding this comment

Uh oh!

cbracken commented May 4, 2018

Uh oh!

cbracken commented May 4, 2018

Uh oh!

mravn-google left a comment

Choose a reason for hiding this comment

Uh oh!

mravn-google May 4, 2018

Choose a reason for hiding this comment

Uh oh!

mravn-google May 4, 2018

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

cbracken commented May 4, 2018 •

edited

Loading

cbracken May 4, 2018 •

edited

Loading