Skip to content
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

Added static lib support for cocoapods #968

Merged
merged 3 commits into from
Nov 20, 2020
Merged

Added static lib support for cocoapods #968

merged 3 commits into from
Nov 20, 2020

Conversation

migs647
Copy link
Contributor

@migs647 migs647 commented Nov 13, 2020

What does this PR do?
Added the capability for the pod to be built as a static framework

Where should the reviewer start?
Test with Swift and Objective-C. It may be appropriate to use use_modular_headers! or use_frameworks! :linkage => :static in the Podfile

How should this be manually tested?
See above

Any background context you want to provide?
Github issue: #752

What are the relevant tickets?
https://segment.atlassian.net/browse/LIBMOBILE-211

Screenshots or screencasts (if UI/UX change)

Questions:

  • Does the docs need an update?
  • Are there any security concerns?
  • Do we need to update engineering / success?

Copy link
Contributor

@bsneed bsneed left a comment

Choose a reason for hiding this comment

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

There should probably be additions to the readme about anything extra people might need to do or want to do to get static vs. dynamic.

README.md Outdated Show resolved Hide resolved
Co-authored-by: LRubin <sanscontext@users.noreply.github.com>
@migs647 migs647 merged commit c40da41 into master Nov 20, 2020
@migs647 migs647 deleted the migs647/podsstatic branch November 20, 2020 19:42
@fangmobile
Copy link

@sanscontext @bsneed
We use Segment with corp license and use it as a dependency in our pod spec.
.dependency 'Analytics', '~> 4.1'

with
s.static_framework = true

our app that uses our internal library will get this error,
target has transitive dependencies that include statically linked binaries: (Analytics)

why are we making Segment as a static framework?

@fangmobile
Copy link

@migs647 any idea to work around the error?

@bsneed
Copy link
Contributor

bsneed commented Feb 4, 2021

@fangmobile can you see if these options work for you? https://guides.cocoapods.org/syntax/podfile.html#use_frameworks_bang

@fangmobile
Copy link

@bsneed thanks :linkage => :static works in the app Podfile.
We are also evaluating adding s.static_framework = true in our library pod spec. What are the pro and cons?

Cons: Since it is statically loaded App core memory usage is higher. App load time might be affected as well. There are some extra steps to do involving multiple copies of files produced by Cocoapods scripts.

Pros?

any thoughts are appreciated.

@bsneed
Copy link
Contributor

bsneed commented Feb 4, 2021

Being statically linked, the app load time will be shorter. As for memory usage, that should be lower as well as there's no overhead of the frameworks. If you're seeing a memory difference it could be because some frameworks are being lazy loaded, but if they are being used, they'll get loaded eventually. Apple's had a few sessions that go over the benefits of static linking vs. dynamic, namely that they optimize the system such that performance is still good at, I think they said 4?? dynamic frameworks, and more than that it drops precipitously. YMMV though.

This article covers some of it as well as links to the WWDC sessions on the subject: https://medium.com/ios-expert-series-or-interview-series/ios-app-launch-static-binding-vs-dynamic-binding-linking-vs-embedded-e69ea9f03f72

I was thinking if you set the :linkage to be dynamic, it would override what we're setting, but not sure without checking.

@Noobish1
Copy link

Noobish1 commented Mar 2, 2021

I'm seeing similar to @fangmobile where we have an internal CocoaPod and this change forces the Segment framework to be built as a static framework so we get transitive dependency errors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants