Skip to content
This repository has been archived by the owner on Dec 19, 2017. It is now read-only.

Common files valid for both platforms. #1

Closed
wants to merge 1 commit into from

Conversation

drodriguez
Copy link

When including libffi as a Cocoapod in a mixed platform project (like https://github.com/jasperblues/Typhoon), several header files are overwritten with ones for the other platform. The headers are ffi.h, fficonfig.h, and ffitarget.h (and technically ffi_common.h)

Since the files are only including other suffixed files I tested the approach in this pull request in my local copy and seems to work. Basically the common files are shared between the two platforms, and they include the preprocessor conditionals to include or not the suffixed headers (I haven't done it, but the same conditionals could be removed from the the rest of the headers).

For both the iOS project and the OS X project, when removing the other platform headers, the current platform header is used just fine, and no warning about missing includes is generated.

I haven't changed the scripts that seems to generate the headers, just in case you don't like the idea of having the common headers, or you want other approach.

PD: I have seen the work in ios-redo branch, but that branch doesn't have the podspec nor the pre-generated headers, that's why I did the change here. Besides, if you are going to include ARM64 (like it seems from the commits in that branch), a new set of includes will be needed.

@zwaldowski
Copy link
Owner

The ios/ and osx/ trees are generated by generate-ios-source-and-headers.py and generate-osx-source-and-headers.py, respectively, and they are not designed for interaction with each other at this time. This repository only has included the ios/ and osx/ trees as a convenience. As atgreen/libffi#60 merges all this work into the master repository, this repository will soon become defunct and future CocoaPods specs will execute the respective scripts from the upstream directly. I'm open to better ways of doing this, and invite you to contribute them to the upstream repo in time for the 3.0.14 release window.

@drodriguez
Copy link
Author

Ok. It is great that things are moving into the master repository.

I see the build scripts there, what I cannot see is the pod spec file, so I don't know how and when those scripts will be executed by the spec (I suspect something like a prepare_command). Do you have any idea of how it is going to be?

From what I see in the master repo scripts, I think I can merge both iOS and OS X scripts into one script, and allow the user to choose the platform, or build both by default, generating the header files like I have described in this issue. Will this be OK with you?

@zwaldowski
Copy link
Owner

That sounds fine. Re: the pod spec either prepare_command or pre_install would seem to work.

@zwaldowski
Copy link
Owner

This has now been absorbed into a revised 04eb8c7, with the added addition that the scripts now use xcrun for compiling; this solves the STDC_HEADERS problem and greatly reduces the amount of code in the script. I apologize for that commit not having your name on it @drodriguez, if it's an issue I can make that happen. Do also note that I've mentioned you in the changelog 6837c2d.

@drodriguez
Copy link
Author

No problem at all.

BTW, we found out that NSInvocation was enough for our use case, but I still have the code with libffi around, so I will test how the pod works. Thanks a lot.

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

Successfully merging this pull request may close these issues.

2 participants