search

facebookarchive / pop

An extensible iOS and OS X animation library, useful for physics-based interactions.
Other
19.66k stars 2.88k forks source link

Fix OS X framework install path #315

Closed eyeplum closed 8 years ago

eyeplum commented 8 years ago

Previously, the value of the build setting INSTALL_PATH is $(SYMROOT)/Headers which may prevent the dynamic linking from working correctly.

Usually, a dynamic linked OS X framework is copied to APP_BUNDLE/Contents/Frameworks, while the executable is usually located at APP_BUNDLE/Content/MacOS/EXECUTABLE_NAME. As per apple document, the suggested install path of an OS X framework is @executable_path/../Frameworks.

This pull request changes the build setting INSTALL_PATH for target pop-osx-framework to @executable_path/../Frameworks.

facebook-github-bot commented 8 years ago

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

eyeplum commented 8 years ago

@facebook-github-bot CLA signed 😃

facebook-github-bot commented 8 years ago

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

nlutsenko commented 8 years ago

Awesome catch! Let's use @rpath in this case, which is the most flexible option of all. This change would basically require for pop to always be embedded with the application, which might not be the case, where say a framework is shared between two or three applications, every one of which links it differently.

@rpath asks the dynamic linker to search a list of locations for the library. That list is embedded in the application, and can be controlled by the application's build process. This basically means that a single copy of a framework can thus work for multiple purposes.

eyeplum commented 8 years ago

@nlutsenko Thanks for the review.

I've changed the INSTALL_PATH to @rpath and have tested it in a simple Cocoa app, it works.

Also, it seems that the PUBLIC_HEADERS_FOLDER_PATH was set to a folder adjacent to the framework build product, as a result, users of the framework may have to manually copy the header files in order to make use of the framework. I changed the PUBLIC_HEADERS_FOLDER_PATH to $(CONTENTS_FOLDER_PATH)/Headers so the header files will be correctly copied into the framework bundle. Thus users can use the framework as-is.

nlutsenko commented 8 years ago

Yup, that's another amazing catch. I think we should just move all of the configurations to use our xctoolchain :)

Waiting for build and merging. Thanks again!

eyeplum commented 8 years ago

🎉 Great! Glad I can help!