Skip to content
This repository has been archived by the owner on Mar 26, 2020. It is now read-only.

Add a script to build static fat binary lib. #341

Merged
merged 4 commits into from
Jan 3, 2018
Merged

Add a script to build static fat binary lib. #341

merged 4 commits into from
Jan 3, 2018

Conversation

yageek
Copy link
Contributor

@yageek yageek commented Dec 5, 2017

Based on the platform file provided by the ios-cmake repository, I added a script helping to generate a fat static library for apple platforms.
This only builds the OBJC part of the support-lib for now.

It basically loops over an array of valid platforms and uses lipo to merge all the different generated .a files.

Those files are generated inside the out directory.
The generated universal library is located at out/libdjinni_support_lib_UNIVERSAL.a

Based on the platform file provided by the
[ios-cmake](https://github.com/leetal/ios-cmake) repository,
the script generates a fat static library based on the architectures
specified by the `BUILD_ARCHITECTURES` variable.
@artwyman
Copy link
Contributor

artwyman commented Dec 6, 2017

Logistics first: @yageek I don't think you've contributed before, so I need you to sign the CLA: https://opensource.dropbox.com/cla/

Our CLA-checking bot is broken, but just post here when you've signed and I can confirm your username the back-end. Or if you're with an organization which signed let me know which to look for.

Copy link
Contributor

@artwyman artwyman left a comment

Choose a reason for hiding this comment

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

I'm curious why this needs to be a stand-alone script, rather than a part of the CMake rules. I don't use CMake enough to know the details.

If this is expected to be stable/supported in future it needs to be added as a target in the main Makefile (perhaps conditional on platform). make clean && make all (on Mac) is what the maintainers generally run and validate for all changes, so anything not included in that may break in future. Alternately, this could be in the same optional/community-supported state as other cmake features (which are in that state since we don't use them at Dropbox), in which case we'll do our best not to break it, but it'll be up to community members to contribute fixes if we do.

README.md Outdated
The `mac-build-support-lib.sh` helps you to build an universal static library.
It uses the platform file of the [ios-cmake](https://github.com/leetal/ios-cmake) repository.

It basically creates one universal static library per `IOS_PLATFORM` variable and uses `lipo`
Copy link
Contributor

Choose a reason for hiding this comment

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

The combination of "Mac" in the script name and "IOS" here makes me wonder how general this feature is. Does it cover all Apple platforms, or just iOS?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The name is bad, I have to admin. The generated binaries are for iOS platform, but to be compiled from a mac. I will rename the file.

#!/bin/sh

# This script helps you to build apple fat binaries.
# This uses the excellent https://github.com/leetal/ios-cmake cmake platform file.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you document somewhere exactly which version of that file was incorporated (by version if that exists, or else a git commit).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add the sha-1 of the file inside the script.

@yageek
Copy link
Contributor Author

yageek commented Dec 6, 2017

I did not have to time to dedicate to create a full CMake script/module to support fat binary. As some other platform file were existing, I took the quickest path to build the universal library.

The process is explained here: https://github.com/ruslo/sugar/wiki/Building-universal-ios-library

I never used CMake to create some kinde of loop over the same target and rebuild for different architecture. I guess a full CMake strategy is possible but I never tried.

Do you still want to consider adding the script or do you prefer to concentrate effort on a full CMake version?

EDIT: As we are cross-compiling with the same source base, it seems it may not be possible to have a full CMake solution: https://stackoverflow.com/questions/5204180/how-to-build-several-configurations-at-once-with-cmake

@artwyman
Copy link
Contributor

artwyman commented Dec 7, 2017

I wouldn't mind adding an optional helper script, so long as it's clear in docs what it does and doesn't do. It might be best kept in community-supported state, meaning don't add it to make all but just let it be used by people who need/want it. The normal approach to the support-lib in Xcode is just to add it as a source along with the generated sources, but a separate binary may certainly be useful to some people.

- Rename the name of the script
- Clarify what does the script
@@ -0,0 +1,66 @@
#!/bin/sh
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this file go somewhere other than at the top-level? I feel like it belongs in support-lib/.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have created the script at the same level as the CMakeLists.txt that builds the support library. Should I move it also?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, good point, which I didn't realize since I don't use CMake. If I were doing it from scratch, I feel like that CMakeLists.txt file belongs in the subdir too, along with the .gyp file. I'd still suggest moving the script, though. If we had some sort of a bin or scripts dir it would belong there, but support-lib seems best for now.

@artwyman
Copy link
Contributor

I can confirm now that you've signed the CLA, so you're all set. Sorry for the confusion.

@yageek
Copy link
Contributor Author

yageek commented Dec 12, 2017

No worry :) I was just wondering. I updated the script and moved it to the library subdir

Copy link
Contributor

@artwyman artwyman left a comment

Choose a reason for hiding this comment

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

LGTM
Leaving this for @xianwen to merge in his next batch.

@paulocoutinhox
Copy link

Only to comment, i made a tool that have this feature: www.ezored.com

@xianwen xianwen merged commit ddbe4d9 into dropbox:master Jan 3, 2018
mariannegru added a commit to mariannegru/djinni that referenced this pull request Jan 19, 2018
* upstream/master: (54 commits)
  Upgrade example build (dropbox#339)
  Separate `flags` to `enum_flags.djinni` (dropbox#346)
  Add a script to build static fat binary lib. (dropbox#341)
  Upgrade the example to Gradle 4.2 and Android plugin 2.3.3 (dropbox#329)
  Android parcelable support (dropbox#308)
  Add assembly to produce standalone jar. (dropbox#324)
  New "flags" archetype for enums oriented towards bitwise flags (dropbox#323)
  Add Xianwen to contacts
  Fix CFStringEncoding/NSStringEncoding confusion
  Added missing std:: in platform_threads_impl (dropbox#321)
  Avoid unused variable warning when compiling djinni_support.cpp (dropbox#319)
  [Android] Fix Djinni initialization crash in multi shared library case (dropbox#310)
  Fix rsync commands to not update timestamps on unchanged contents
  Swap the README.md change for a CONTRIBUTORS file for attribution.
  Renaming "temp" to "proxy_handle" to make more readable.
  Adding comment to warn of the potential segfault in gcc.
  Making three changes to fix compilation in our environment.
  Make Swift code more idiomatic
  move CMakeLists.txt file to root directory and rename target name to djinni_support_lib
  revert unintended generator change
  ...

# Conflicts:
#	example/generated-src/objc/TXSTextboxListener+Private.mm
#	example/objc/TextSort.xcodeproj/project.pbxproj
#	example/objc/TextSort.xcodeproj/xcshareddata/xcschemes/TextSort.xcscheme
#	src/source/ObjcppGenerator.scala
#	src/source/parser.scala
#	test-suite/djinni/common.djinni
#	test-suite/generated-src/inFileList.txt
#	test-suite/generated-src/objc/DBClientInterface+Private.mm
#	test-suite/generated-src/objc/DBEnumUsageInterface+Private.mm
#	test-suite/generated-src/objc/DBExternInterface2+Private.mm
#	test-suite/generated-src/objc/DBFirstListener+Private.mm
#	test-suite/generated-src/objc/DBObjcOnlyListener+Private.mm
#	test-suite/generated-src/objc/DBSecondListener+Private.mm
#	test-suite/generated-src/objc/DBUserToken+Private.mm
#	test-suite/generated-src/objc/DBUsesSingleLanguageListeners+Private.mm
#	test-suite/generated-src/outFileList.txt
#	test-suite/handwritten-src/java/com/dropbox/djinni/test/AllTests.java
#	test-suite/objc/DjinniObjcTest.xcodeproj/project.pbxproj
KhalilBellakrid pushed a commit to KhalilBellakrid/djinni that referenced this pull request Mar 8, 2018
* Add a script to build static fat  binary lib.

Based on the platform file provided by the
[ios-cmake](https://github.com/leetal/ios-cmake) repository,
the script generates a fat static library based on the architectures
specified by the `BUILD_ARCHITECTURES` variable.

* Update according to the discussion

- Rename the name of the script
- Clarify what does the script

* Move the build script.

* Move script to support lib directory
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants