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

extconf: add sdk helper, primarily for macOS #757

Closed
wants to merge 1 commit into from

Conversation

DomT4
Copy link

@DomT4 DomT4 commented Mar 26, 2020

macOS removed /usr/include in recent versions of macOS, which has broken the logic in this file and forced the usage of the internal libffi when macOS has a perfectly usable one itself.

checking for ffi.h... no
checking for ffi.h in /usr/local/include,/usr/include/ffi... no

The internal libffi also seems to have some issues building on at least macOS 10.15/Catalina, and seemed to be the cause of my recent issues with other gems that depend on this one.

linking shared-object ffi_c.bundle
ld: warning: ignoring file ext/ffi_c/libffi-x86_64-darwin19/.libs/libffi_convenience.a,
building for macOS-x86_64 but attempting to link with file built for unknown-unsupported file format
( 0x21 0x3C 0x61 0x72 0x63 0x68 0x3E 0x0A 0x2F 0x20 0x20 0x20 0x20 0x20 0x20 0x20 )

This change allows us to use the built-in libffi, and works fine:

checking for ffi.h... no
checking for ffi.h in /usr/local/include,/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/ffi... yes
checking for ffi_call() in -lffi... yes
checking for ffi_closure_alloc()... yes
checking for shlwapi.h... no
checking for rb_thread_call_without_gvl()... yes
checking for ruby_native_thread_p()... yes
checking for ruby_thread_has_gvl_p()... yes
checking for ffi_prep_cif_var()... yes
checking for ffi_raw_call()... yes
checking for ffi_prep_raw_closure()... yes

macOS removed `/usr/include` in recent versions of macOS, which has
broken the logic in this file and forced the usage of the internal
libffi when macOS has a perfectly usable one itself.

```
checking for ffi.h... no
checking for ffi.h in /usr/local/include,/usr/include/ffi... no
```

The internal libffi also seems to have some issues building on at least
macOS 10.15/Catalina, and seemed to be the cause of my recent issues
with other gems that depend on this one.

```
linking shared-object ffi_c.bundle
ld: warning: ignoring file ext/ffi_c/libffi-x86_64-darwin19/.libs/libffi_convenience.a,
building for macOS-x86_64 but attempting to link with file built for unknown-unsupported file format
( 0x21 0x3C 0x61 0x72 0x63 0x68 0x3E 0x0A 0x2F 0x20 0x20 0x20 0x20 0x20 0x20 0x20 )
```

This change allows us to use the built-in libffi, and works fine:

```
checking for ffi.h... no
checking for ffi.h in /usr/local/include,/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/ffi... yes
checking for ffi_call() in -lffi... yes
checking for ffi_closure_alloc()... yes
checking for shlwapi.h... no
checking for rb_thread_call_without_gvl()... yes
checking for ruby_native_thread_p()... yes
checking for ruby_thread_has_gvl_p()... yes
checking for ffi_prep_cif_var()... yes
checking for ffi_raw_call()... yes
checking for ffi_prep_raw_closure()... yes
```
larskanis added a commit to larskanis/ffi that referenced this pull request Mar 27, 2020
macOS removed `/usr/include` in recent versions of macOS.
This forces the usage of the internal libffi although macOS has a usable one itself.

Related to ffi#757
@larskanis
Copy link
Member

@DomT4 Thank you very much for this PR! Unfortunately I don't have a Mac and have little to no knowledge about macOS. So any contributions are most welcome.

I think the paths can be moved directly into the find_header call. What do you think about #758 ?

Also could you please have a look at the build error with builtin libffi? You can force it as described in the README. Both ways to install should work on common systems.

@DomT4
Copy link
Author

DomT4 commented Mar 29, 2020

What do you think about #758 ?

#758 would work fine 👍.

I mostly wrote it this way to avoid having to check redundant paths on older versions of macOS or different platforms, and because it's a little more verbose about why we're checking paths other than the standard ones; figured that could be handy in a year or two when Apple inevitably make some other kind of major change that force further changes here 🙃.

Also could you please have a look at the build error with builtin libffi?

I'm not completely sure what/why is going on yet but it seems libffi suffers from some kind of environment contamination issue.

If I do a gem install ffi --verbose -- --disable-system-libffi I get the build error quoted in the opening post here. If I do:

PATH="/Users/D/.rbenv/shims:/Users/D/.rbenv/bin:/usr/bin:/bin:/usr/sbin:/sbin" gem install ffi --verbose -- --disable-system-libffi

there is no such error. I'm guessing that aforementioned "environment contamination" is Homebrew given any time I put /usr/local/bin back in the path the original error returns. Homebrew typically occupies /usr/local and is something a lot of people have installed on macOS, but again, not completely sure what it's catching on yet.

For what it's worth the issue seems to persist both before and after 0b4e779.

larskanis added a commit to larskanis/ffi that referenced this pull request Mar 30, 2020
macOS removed `/usr/include` in recent versions of macOS.
This forces the usage of the internal libffi although macOS has a usable one itself.

Related to ffi#757
@larskanis larskanis mentioned this pull request Mar 30, 2020
larskanis added a commit to larskanis/ffi that referenced this pull request Mar 30, 2020
macOS removed `/usr/include` in recent versions of macOS.
This forces the usage of the internal libffi although macOS has a usable one itself.

Related to ffi#757
larskanis added a commit to larskanis/ffi that referenced this pull request Apr 15, 2020
MacOS removed `/usr/include` in recent versions of macOS.
That forced the usage of the internal libffi although macOS has a usable one itself.

This commit adds typical paths of the SDK which contain libffi.
Since it's possible to use non default SDK paths, there is another fallback to determine the path per "xcrun" command.

Fixes ffi#757
Closes ffi#765
Closes ffi#758
@larskanis larskanis closed this in 31db412 Apr 15, 2020
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.

None yet

2 participants