Package libffi
Closed this issue · 27 comments
This is required for compiling Python 3.7
Tarball: ftp://sourceware.org/pub/libffi/libffi-3.2.1.tar.gz
CC: @embray
Component: packages: standard
Author: Jeroen Demeyer
Branch: e45cf27
Reviewer: Frédéric Chapoton, Erik Bray
Issue created by migration from https://trac.sagemath.org/ticket/25900
Description changed:
---
+++
@@ -1 +1,3 @@
This is required for compiling Python 3.7
+
+**Tarball**: ftp://sourceware.org/pub/libffi/libffi-3.2.1.tar.gzBranch: u/jdemeyer/package_libffi
ok, should be good. Tested on my machine with py2 (and py3 also). Let's ask the buildbots.
Reviewer: Frédéric Chapoton
Afaik libffi has been a de facto dependency of ecl (and, therefore, Sage) for a long time. We could just document said dependency...
On a side note, the cygwin patchbot is unhappy with it
The only problem with the patchbot is that it was unable to find the tarball. Normally, with tickets that add new packages, it's supposed to be able to parse out the path to the source tarball from the ticket description, but that seems to have failed in this case. I'll look into why.
AFAIK libffi works fine on Cygwin--I have used it there plenty myself (and have even fixed some bugs in it :)
Let me test on Cygwin though before setting this to positive review.
Replying to @vbraun:
Afaik libffi has been a de facto dependency of ecl (and, therefore, Sage) for a long time. We could just document said dependency...
I'm fine with adding an spkg for it. I think there should be one as some libffi versions do have bugs that could theoretically affect us.
But I do think there should be a configure-time check for it to avoid needlessly installing the spkg. I would like to start adding this for all new packages. For now it could go directly in configure.ac, but if some version of #24919 ever gets merged then the check might be moved.
If ECL really required libffi, I'm sure that we would have noticed by now. For example, I have a system where Python 3.7 failed to build because libffi wasn't installed. But this problem did not occur for ECL.
Erik: this ticket had positive_review and it's a dependency for Python 3.7. So it's fine if you want to add the logic to check for a system-wide install, but please don't stall this ticket.
I'm only stalling it to check that it actually works on Cygwin. I'm happy to add the system check either now, or as a separate ticket. I'd like to discuss making this required for adding any new spkgs, though since we haven't actually done that yet I won't hold this up over that.
Any news?
I'll try it today or tomorrow--soon as I'm done testing 8.4.beta1 on my Windows build.
Odd that libffi puts its headers in lib/libffi-3.2.1/include/. I found this to be the case on Cygwin and on Linux. I guess it's not really a problem since pkg-config reports that location correctly, and I bet there's probably a good reason for it. Just...odd.
Could you add something like:
diff --git a/configure.ac b/configure.ac
index 5a830e5..a31b53b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -435,6 +435,12 @@ AC_CACHE_CHECK([for curl 7.22], [ac_cv_path_CURL], [
[need_to_install_curl=yes; ac_cv_path_CURL=no])])
+# Only install libffi if needed; if found, we also check for its headers
+# below
+need_to_install_libffi=no
+AC_SEARCH_LIBS([ffi_call], [ffi], [], [need_to_install_libffi=yes])
+
+
###############################################################################
# Check header files
###############################################################################
@@ -446,6 +452,12 @@ then
AC_CHECK_HEADER([curl/curl.h], [], [need_to_install_curl=yes])
fi
+# If libffi is installed, we need to check for the ffi.h header file too.
+if test $need_to_install_libffi = no;
+then
+ AC_CHECK_HEADERS([ffi.h ffi/ffi.h], [break], [need_to_install_libffi=yes])
+fi
+
# complex.h is one that might not exist on older systems.
AC_LANG(C++)
AC_CHECK_HEADER([complex.h],[],[?
Otherwise, seems fine to me. I haven't tested this against Python 3.7 itself yet, but that has other build problems on Cygwin I need to work on. If any further libffi-related problems come up in the context of building Python we can deal with it there.
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
faad2c9 | Add libffi package |
Erik, I added the new-style configure check. Does this look correct to you?
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
e45cf27 | Add libffi package |
Retargeting some of my tickets.
Replying to @jdemeyer:
Erik, I added the new-style configure check. Does this look correct to you?
Sorry, I didn't see your update before. I haven't tested, but it looks good to me in principle. If we need to make the check any more specific we can do that later (e.g. there could be bugs with older libffi versions, but I'm not worried about that unless it is found to actually affect someone's system).
Changed reviewer from Frédéric Chapoton to Frédéric Chapoton, Erik Bray
Changed branch from u/jdemeyer/package_libffi to e45cf27
some issue arise from this ticket both on gentoo and ubuntu:
cp: cannot overwrite non-directory '/64bitdev/storage/sage-git_develop/sage/local/./lib64' with directory '/64bitdev/storage/sage-git_develop/sage/local/var/tmp/sage/build/libffi-3.2.1/inst/64bitdev/storage/sage-git_develop/sage/local/./lib64'