-
Notifications
You must be signed in to change notification settings - Fork 24.4k
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
Exclude all FlipperKit transitive dependencies from iOS Release builds #28504
Exclude all FlipperKit transitive dependencies from iOS Release builds #28504
Conversation
…ase build The `:configuration` option from `pod` only affects the specified pod and not its dependencies [1]. Therefore in order to avoid all transitive dependencies being linked in the resulting Release IPA we need to list them in the `Podfile`. [1] https://guides.cocoapods.org/syntax/podfile.html#pod
Base commit: 3a11f05 |
Base commit: 83fee73 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking the time!
In the meantime, I’ll port this to the Actually not going to do that, we don’t want to cherry-pick things not landed in 0.62-stable
branch and get it in today’s 0.62.1
release.master
and the fix is easy enough for people to make locally.
@passy Can you get this in? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@priteshrnandgaonkar has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
This pull request was successfully merged by @javiercr in e5497ca. When will my fix make it into a release? | Upcoming Releases |
#28504) Summary: The `:configuration` option from `pod` only affects the specified pod and not its dependencies [1]. Therefore in order to avoid all transitive dependencies being linked in the resulting Release IPA we need to list them in the `Podfile`. Note that this will still build Flipper's pods when doing a Release, but it won't link it in the resulting IPA. [1] https://guides.cocoapods.org/syntax/podfile.html#pod Fixes react-native-community/upgrade-support#28 Related CocoaPods/CocoaPods#9658 * [iOS] [Fixed] - Exclude Flipper from iOS Release builds Pull Request resolved: #28504 Test Plan: Create a new React Native 0.62 project, run `pod install`, then diff: ``` ProjectName/ios/Pods/Target Support Files/Pods-ProjectName/Pods-ProjectName.debug.xcconfig` ``` and ``` ProjectName/ios/Pods/Target Support Files/Pods-ProjectName/Pods-ProjectName.relaese.xcconfig ``` ![image](https://user-images.githubusercontent.com/855995/78337679-a3fa0280-7591-11ea-8142-6f82cbc6be58.png) Reviewed By: passy Differential Revision: D20894406 Pulled By: priteshrnandgaonkar fbshipit-source-id: 680780f0f5a85fd8423b85a271a499bd12f06d00
Is there a reason this doesn't cover the following transitive dependencies?
|
@Ashoat Not as far as I know. Can you make a PR for it? |
My only worry is that in contrast with the other excluded transitive dependencies, which are definitely Flipper-specific, these four could be required by other pods. It appears that applying Ideally we would have a mechanism to exclude them from release builds only if they don't appear anywhere else in the dependency tree, but I'm not familiar enough with CocoaPods to know if something like that is possible. |
I think that's why I didn't include them in this PR. I could only exclude (actually include) the ones that I knew for sure were dependencies only for Flipper (basically the ones with Flipper in the name). |
The best solution would be to take this CocoaPods PR to completion CocoaPods/CocoaPods#9066. It’s on my TODO list, but I have no idea how long it would take before I get to that, so if there are other takers please do take a shot. |
Summary
The
:configuration
option frompod
only affects the specified pod and not its dependencies [1]. Therefore in order to avoid all transitive dependencies being linked in the resulting Release IPA we need to list them in thePodfile
.Note that this will still build Flipper's pods when doing a Release, but it won't link it in the resulting IPA.
[1] https://guides.cocoapods.org/syntax/podfile.html#pod
Fixes react-native-community/upgrade-support#28
Related CocoaPods/CocoaPods#9658
Changelog
Test Plan
Create a new React Native 0.62 project, run
pod install
, then diff:and