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

Provide static binaries? #249

Closed
keith opened this issue May 17, 2018 · 52 comments
Closed

Provide static binaries? #249

keith opened this issue May 17, 2018 · 52 comments
Assignees

Comments

@keith
Copy link
Contributor

keith commented May 17, 2018

Is there any chance that you all could provide static framework versions of Instabug as well as or in place of the dynamic ones? CocoaPods supports this for the past few versions. There are 2 benefits I think we'd see.

  1. Any unused symbols could be removed at link time. Meaning if consumers of the SDK don't end up using the whole thing, they can end up with a smaller binary size increase from integrating Instabug.

  2. This decreases the amount of dynamic linking time when the apps are being launched, which helps the app launch faster.

Thoughts?

@Kmohamed
Copy link
Contributor

@keith Make sense let me check it and get back to you.
Thanks a lot 🙏

@HeshamMegid
Copy link
Contributor

CocoaPods supports this for the past few versions.

@keith as far as I understand, the newly introduced s.static_framework = true is only relevant for source pods. Since we distribute a binary, we would have to maintain 2 separate podspecs and 2 different sets of binaries if we want to give the pod consumer the liberty of choosing whether to integrate as a static or dynamic framework.

We’re looking into whether we should maintain versions of both static and dynamic, and if dropping the dynamic framework wouldn’t work for certain use cases. We’ll get back to you soon.

Also, out of the 2 benefits you mentioned, which one is more important? I'm asking since the first one could be achieved by breaking our framework into smaller subspecs.

@keith
Copy link
Contributor Author

keith commented May 17, 2018

@keith as far as I understand, the newly introduced s.static_framework = true is only relevant for source pods.

Ah right I'm sorry, this is actually irrelevant because CocoaPods has supported this for a while. The GoogleMaps pods for example are pre-compiled static frameworks.

Since we distribute a binary, we would have to maintain 2 separate podspecs and 2 different sets of binaries if we want to give the pod consumer the liberty of choosing whether to integrate as a static or dynamic framework.

While you would have to have 2 sets of binaries if you wanted to support both, you could maintain a single pod spec using subspecs, which would simplify things a bit.

if dropping the dynamic framework wouldn’t work for certain use cases

Unless anyone is doing something crazy, there shouldn't be a difference to consumers. The only thing you might have to change is your resource loading

Also, out of the 2 benefits you mentioned, which one is more important? I'm asking since the first one could be achieved by breaking our framework into smaller subspecs.

We're interested in this for some more reasons that just those I mentioned. Currently Instabug is our only dependency that isn't static, so we're trying to unify that to simplify our build tooling.

@kastiglione
Copy link

kastiglione commented May 17, 2018

if dropping the dynamic framework wouldn’t work for certain use cases

A dynamic framework (MH_DYLIB) can always be produced out of a static library/framework, but of course the reverse is not true. It's possible to ship one binary, a static one, and then add a step that transforms the static binary into a dynamic framework, for those that need it. Happy to help figure this out, if there's interest.

@HeshamMegid
Copy link
Contributor

A dynamic framework (MH_DYLIB) can always be produced out of a static library/framework, but of course the reverse is not true.

We were not aware of that. That will definitely make things easier.

Thanks @keith and @kastiglione! We will continue discussing this internally and get back to you soon.

@keith
Copy link
Contributor Author

keith commented May 19, 2018

Another benefit I just realized to providing a precompiled static framework, is you could remove this script referenced in your readme since this would be handled automatically by the linking process.

@HeshamMegid
Copy link
Contributor

We’ve decided to switch to a static framework. We’ll be working on it this week, so it should be available in either the next release or the one after. Will let you know once it’s released.

@kastiglione I couldn’t find much that talks about transforming a static binary into a dynamic one. If you have any resources that talk about this, I’d really appreciate if you share them.

@keith
Copy link
Contributor Author

keith commented May 20, 2018 via email

@kastiglione
Copy link

kastiglione commented May 23, 2018

@HeshamMegid I don't know if there are resources that talk about it. Here's an overview.

The two main kinds of binary files in this discussion are:

  1. Intermediate build files (object files, static libraries)
  2. Product binaries (executables, dynamic frameworks, dynamic libraries)

Source files are compiled into intermediate build files, and those intermediates are in turn linked into binaries that can be loaded and run.

Dynamic frameworks (and executables) can be produced from any combination of .o object files and static libraries. From an Xcode perspective, you could imagine three targets:

  1. Application
  2. Static library
  3. Framework

These three targets could be structured with the majority of the source files as part of the static library. The application and framework would in turn link in the static library via Xcode's "Linked Frameworks and Libraries" section. In this example, Xcode would build a static library, and then use that to produce the app and the framework.

To do all this linking, Xcode isn't doing anything fancy, it calls commands in the toolchain. At its most basic, converting a static library to a framework would look like this:

mkdir -p path/to/X.framework
ld -dylib path/to/libX.a -o path/to/X.framework/X

Typically build systems don't use ld directly, they use clang or swift to invoke the linker. Xcode for example does something like this:

/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang
-arch x86_64 \
-dynamiclib \
-isysroot path/to/iPhoneSimulator11.3.sdk \
-Lpath/to/deriveddata/Build/Products/Debug-iphonesimulator \
-Fpath/to/deriveddata/Build/Products/Debug-iphonesimulator \
-lStaticTargetName \
-o path/to/TargetName.framework/TargetName

I've reduced this a bunch to focus on what would be relevant for the discussion. The flags above are the most noteworthy (but not necessarily required). Despite the use of clang, no source code is bing compiled, instead the clang command is a "driver" that in turn runs ld. The point being that you can use clang or you can use ld, whatever suits the needs best. To see the underlying ld command line, you can add a -### flag to the clang command.

Hopefully this helps some, I'm happy to answer more questions. To support both static and dynamic, you probably have two primary options: 1. Use the Xcode structure I mentioned above: a static library target and a framework target, where the framework has no code and simply links in the entirety of the static library, or alternatively 2. Release binaries that contain just static frameworks, but also provide scripts that generate a dynamic framework on the developer's machine.

@Instabug Instabug deleted a comment May 23, 2018
@HeshamMegid
Copy link
Contributor

@kastiglione thank you so much for taking the time to write this up! We'll give it a try as soon as we change our build scripts to produce a static framework.

@Kmohamed
Copy link
Contributor

Kmohamed commented Jun 5, 2018

@keith I face some problems with cocoapods and static library I hope that you can help me to fix it
I wanna using development pods inside static library target but for some reason the static target can not find the development pods. I tried to remove use_frameworks but it did not work.
any suggestions ?

@kastiglione
Copy link

@Kmohamed heads up that Keith may not be able to reply until early next week.

@Kmohamed
Copy link
Contributor

Kmohamed commented Jun 7, 2018

@kastiglione Thanks

@keith
Copy link
Contributor Author

keith commented Jun 9, 2018

@Kmohamed can you elaborate on the structure you're trying to make work?

@Kmohamed
Copy link
Contributor

Kmohamed commented Jun 9, 2018

@keith As you know we split Instabug into two frameworks (Instabug and InstabugCore)
so internally we have two projects (Instabug and InstabugCore). Instabug is using a lot of classes from InstabugCore so in order to build Instabug framework we add InstabugCore as development pod so Instabug can see InstabugCore classes. this works fine because Instabug target is dynamic framework and cocoapods generates a dynamic framework but when switching to static library Instabug can not see InstabugCore anymore whether cocoapods create a static library from InstabugCore or a dynamic one.

@keith
Copy link
Contributor Author

keith commented Jun 9, 2018

So you're vendoring InstabugCore as a development pod and as a precompiled static framework? Or you have a separate internal podspec using the sources?

@Kmohamed
Copy link
Contributor

Kmohamed commented Jun 9, 2018

Yes, We have a separate internal pod spec using sources

@keith
Copy link
Contributor Author

keith commented Jun 9, 2018

If I'm following correctly, here's a sample project that shows this layout working. Instabug is the main target which is a static framework, and I'm including InstabugCore with CP
Instabug.zip

@Kmohamed
Copy link
Contributor

Kmohamed commented Jun 9, 2018

Thanks a lot 🙏 🙏🙏
I will check it and get back to you

@keith
Copy link
Contributor Author

keith commented Jun 28, 2018

I noticed one thing that you'll likely have to handle before shipping static binaries, it looks like both Instabug and InstabugCore have these duplicate symbols:

_kIBGBackIcon
_kIBGTickIcon

I assume it will be easy enough for you to either make the static, or reference them from InstabugCore in Instabug, just FYI

@Kmohamed
Copy link
Contributor

You are right and I already fixed it. Related to static library I would like to apologize for being late in providing a static library from Instabug it takes some time because we need to make some changes in our codebase architecture so in order not block your migration from dynamic frameworks to static libraries here is a quick version from Instabug that you can use and we will include it in our releases very soon.
Another thing thanks a lot for your support 🙏

@keith
Copy link
Contributor Author

keith commented Jun 29, 2018

Thanks for sending this! 2 quick pieces of feedback:

  1. The headers should live in Instabug.framework/Headers instead of at the top level
  2. Normally for dependencies that are vendored this way, the resource bundle would also be in Instabug.framework/Resources. This way it's easy to move around the single directory and have everything you need (even though this resource bundle would still be copied into the final .app)

@Kmohamed
Copy link
Contributor

Sure, will do.

@keith
Copy link
Contributor Author

keith commented Jun 29, 2018

Found another issue, it looks like some of the object files here were built with a minimum deployment target of 11.4, instead of something lower:

ld: warning: object file (Instabug/current/Instabug.framework/Instabug(NSObject+Description.o)) was built for newer iOS version (11.4) than being linked (10.0)

@Kmohamed
Copy link
Contributor

@keith sorry for that here is the new library
I have one question related to insert bundle inside the framework.
Instead of you drag both framework and bundle in one step to Xcode project you will need to drag the framework and put it inside Xcode then go back to drag the bundle and drop it inside the Project what do you think ?

@Kmohamed
Copy link
Contributor

btw @keith you need to add -ObjC in Other linker flag and add libz

@keith
Copy link
Contributor Author

keith commented Jul 2, 2018

Instead of you drag both framework and bundle in one step to Xcode project you will need to drag the framework and put it inside Xcode then go back to drag the bundle and drop it inside the Project what do you think ?

I think this is expected for static frameworks for those including manually. For those including with CocoaPods or something, they won't notice this.

btw you need to add -ObjC in Other linker flag and add libz

Yep! No problem, we were already using those, it appears to work correctly now!

@Kmohamed
Copy link
Contributor

Kmohamed commented Jul 2, 2018

Great 🎉

@keith
Copy link
Contributor Author

keith commented Jul 3, 2018

Do you plan to make an official release with this soon?

@Kmohamed
Copy link
Contributor

Kmohamed commented Jul 3, 2018

Do you mean using cocoapods ?

@keith
Copy link
Contributor Author

keith commented Jul 3, 2018

Or just on your release page

@Kmohamed
Copy link
Contributor

Kmohamed commented Jul 3, 2018

Yeah sure,
We will provide a custom build from each release to you until we officially support static library.

@keith
Copy link
Contributor Author

keith commented Jul 3, 2018

We haven't integrated this yet (besides my testing) just because it'd be nice to do it through the official release channels

@Korazy Korazy assigned ramiimagdi and unassigned Korazy Jul 4, 2018
@Kmohamed
Copy link
Contributor

Next release will include static library on the Github releases.
is it ok for you to be on Github releases ?

@keith
Copy link
Contributor Author

keith commented Jul 10, 2018 via email

@Kmohamed
Copy link
Contributor

Hello @keith
I wanna ask you a question. I don't know if it possible or not but I wanna link static library to a specific target using XcodeProj
I manage to link dynamic framework only :(

@Kmohamed
Copy link
Contributor

I succeed to make it thanks 🙏

@Kmohamed
Copy link
Contributor

@keith @kastiglione This release contains support for static library
Sorry for being late it require some changes in our code structure and some scripts to automate producing both (dynamic and static).

@keith
Copy link
Contributor Author

keith commented Jul 30, 2018 via email

@keith
Copy link
Contributor Author

keith commented Jul 30, 2018

FYI, probably not an issue, but it looks like this object file:

InstabugStatic.framework/InstabugStatic(UINavigationItem+Subtitle-E7779BB05EEA0660.o):

Is included in your binary twice. But there aren't actually any symbols provided by it.

@keith
Copy link
Contributor Author

keith commented Jul 30, 2018

There are also some other files with no symbols:

InstabugStatic.framework/InstabugStatic(NSData+Additions.o):

InstabugStatic.framework/InstabugStatic(IBGEventLog+CoreDataProperties.o):

@Kmohamed
Copy link
Contributor

Actually UINavigationItem+Subtitle is duplicate inside the target we will fix it
But NSData+Additions is already added could you double check that you already add -ObjC to Other linker ?

@Kmohamed
Copy link
Contributor

Is this a build time error ?

@keith
Copy link
Contributor Author

keith commented Jul 30, 2018

This isn't an error, I just noticed this in the binary right after download, not related to our project

@Kmohamed
Copy link
Contributor

Does all Instabug categories have symbols expecpt these files ?

@keith
Copy link
Contributor Author

keith commented Jul 30, 2018

Looks like there are quite a few others that don't as well. You can see this with nm -arch arm64 InstabugStatic.framework/InstabugStatic maybe all these files have compiler conditionals that result in no symbols for this architecture or public builds?

@Kmohamed
Copy link
Contributor

Thanks a lot i will check it and get back to you

@Kmohamed
Copy link
Contributor

Kmohamed commented Jul 31, 2018

screen shot 2018-07-31 at 5 43 30 pm

@keith is this what you mean by there is no symbol ? _OBJC_CLASS_$_NSJSONSerialization

@keith
Copy link
Contributor Author

keith commented Jul 31, 2018

Ah sorry, I was filtering symbols such that undefined and local symbols were hidden. False alarm!

@Kmohamed
Copy link
Contributor

No worries 😉

@keith
Copy link
Contributor Author

keith commented Aug 3, 2018

@Kmohamed looks like #249 (comment) isn't fixed in the new release

@Kmohamed
Copy link
Contributor

Kmohamed commented Aug 3, 2018

Yes it’s PR didn’t merge yet. I will make sure to release it soon

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

No branches or pull requests

8 participants
@kastiglione @keith @Kmohamed @kamelnabil @HeshamMegid @Korazy @ramiimagdi and others