Package libffi

[jdemeyer]

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

[jdemeyer]

Branch: u/jdemeyer/package_libffi

[jdemeyer]

Commit: 8f6d830

[jdemeyer]

New commits:

8f6d830

Add libffi package

[fchapoton]

comment:4

ok, should be good. Tested on my machine with py2 (and py3 also). Let's ask the buildbots.

[fchapoton]

Reviewer: Frédéric Chapoton

[vbraun]

comment:5

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

[embray]

comment:6

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 :)

[embray]

comment:7

Let me test on Cygwin though before setting this to positive review.

[embray]

comment:9

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.

[jdemeyer]

comment:10

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.

[embray]

comment:11

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.

[jdemeyer]

comment:12

Any news?

[embray]

comment:13

I'll try it today or tomorrow--soon as I'm done testing 8.4.beta1 on my Windows build.

[embray]

comment:14

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.

[embray]

comment:15

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.

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

faad2c9

Add libffi package

[sagetrac-git]

Changed commit from 8f6d830 to faad2c9

[jdemeyer]

comment:18

Erik, I added the new-style configure check. Does this look correct to you?

[sagetrac-git]

Changed commit from faad2c9 to e45cf27

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

e45cf27

Add libffi package

[embray]

comment:20

Retargeting some of my tickets.

[embray]

comment:21

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).

[embray]

Changed reviewer from Frédéric Chapoton to Frédéric Chapoton, Erik Bray

[vbraun]

Changed branch from u/jdemeyer/package_libffi to e45cf27

[fchapoton]

comment:23

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'

[fchapoton]

Changed commit from e45cf27 to none