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

fix Makefile.am in src dir #41

Merged
merged 1 commit into from Jul 27, 2016
Merged

fix Makefile.am in src dir #41

merged 1 commit into from Jul 27, 2016

Conversation

ghost
Copy link

@ghost ghost commented Jul 27, 2016

  1. I am not sure if this is a fix, I am on Crux Linux, this is a fix for me. (Thanks to @condy0919 )
  2. Also you guys said installing libffi-devel can fix, I am curious about how that did the trick.
  3. it's a great project, thanks,

@taviso Can you amswer my question and review this PR? Thanks for your time.

@ghost
Copy link
Author

ghost commented Jul 27, 2016

Also Crux Linux, no devel pkg such thing. so i am really curious :)

@cemeyer
Copy link
Collaborator

cemeyer commented Jul 27, 2016

I would guess that on most Linux distributions, installing the devel package installs the headers directly in /usr/include, which is the global include path. If the headers don't get installed in global /usr/include (e.g. Crux's libffi does this: https://crux.nu/ports/crux-3.2/opt/libffi/.footprint ), then we need $(FFI_CFLAGS) as the patch suggests.

(Fedora just drops them directly in /usr/include:)

$ rpm -ql libffi-devel
/usr/include/ffi-x86_64.h
/usr/include/ffi.h
/usr/include/ffitarget-x86_64.h
/usr/include/ffitarget.h
...

Anyway, the suggested fix seems fine to me.

@taviso
Copy link
Owner

taviso commented Jul 27, 2016

I thought the libstruct code didn't use any libffi stuff...but it seems like a harmless patch, so I think it's okay if this fixes it for you.

I can merge.

@taviso taviso merged commit 90899bc into taviso:master Jul 27, 2016
@taviso
Copy link
Owner

taviso commented Jul 27, 2016

Done, thanks @rtlanceroad!

@cemeyer
Copy link
Collaborator

cemeyer commented Jul 27, 2016

struct.c includes ffi.h. :-)

@ghost ghost deleted the Makefile-fix branch July 27, 2016 15:00
@ghost
Copy link
Author

ghost commented Jul 27, 2016

Thanks for the explaination for devel pkg @cemeyer :) And also merging this PR @taviso :)

@ghost
Copy link
Author

ghost commented Jul 28, 2016

I only see in struct.c, there are less than 10 lines related to ffi.h, maybe in the furture we want to drop libffi support?

@cemeyer
Copy link
Collaborator

cemeyer commented Jul 28, 2016

Why bother? We use ffi widely in the rest of ctypes.sh, so you would still need it installed to compile and use ctypes.sh.

@ghost
Copy link
Author

ghost commented Jul 29, 2016

ok ;)

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.

2 participants