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

Add SDK path on macOS #765

Closed
wants to merge 1 commit into from
Closed

Conversation

larskanis
Copy link
Member

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 #757 . This is an alternative approach to #758 .

@@ -8,7 +8,8 @@ def system_libffi_usable?
# We need pkg_config or ffi.h
libffi_ok = pkg_config("libffi") ||
have_header("ffi.h") ||
find_header("ffi.h", "/usr/local/include", "/usr/include/ffi")
find_header("ffi.h", "/usr/local/include", "/usr/include/ffi") ||
(find_header("ffi.h", `xcrun --sdk macosx --show-sdk-path`.strip) rescue false)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This needs + /include/ffi, otherwise it doesn't find it: https://github.com/ffi/ffi/pull/765/checks?check_run_id=546591203
rescue false seems brittle to me and might swallow unwanted exceptions.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I created #766 to show what I was thinking.

Copy link
Member Author

Choose a reason for hiding this comment

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

This needs + /include/ffi

Of course! I added "usr/include/ffi" and it seems to work.

To be honest I like this PR (including the rescue) more than #766 . It calls xcrun only when necessary, fits well into the chain, is very simple and doesn't break the installation if anything goes wrong.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Indeed, I forgot the /usr in front!
As you prefer, I'm thinking just searching /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/ffi as in #758 is the simplest now.

I personally dislike any silent rescue, as it can easily become a debugging nightmare.

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