Further libffi fixes

[jdemeyer]

with #27219 done, all it remains is just to fix spkg-configure.m4

Depends on #27109
Depends on #27219

CC: @embray @strogdon

Component: packages: standard

Author: Erik Bray

Branch: 07e5561

Reviewer: Erik Bray, Dima Pasechnik

Issue created by migration from https://trac.sagemath.org/ticket/27114

[jdemeyer]

Branch: u/embray/build/ticket-27109

[jdemeyer]

Commit: 4d9ccb4

[jdemeyer]

New commits:

bfa7c90	Trac #27109: Include upstream patch to libffi for --disable-multi-os-directory
4d9ccb4	Trac #27109: Use pkg-config if possible to detect libffi

[jdemeyer]

Dependencies: #27109

[embray]

comment:4

Since Steven already said this worked, I'm inclined to go ahead and set this to positive review. Though I might like to try on OSX since it doesn't have pkg-config and I'm not 100% sure what it will do in this case (I think it will "fail correctly" but I'm not certain).

[strogdon]

comment:5

Replying to @embray:

Since Steven already said this worked, I'm inclined to go ahead and set this to positive review. Though I might like to try on OSX since it doesn't have pkg-config and I'm not 100% sure what it will do in this case (I think it will "fail correctly" but I'm not certain).

I'm fine with this. I too think it should be checked on other distros.

[strogdon]

comment:6

Perhaps we should try to get this in as soon as possible, else there will be Linuxes with system libffi installed where the Sage version will be built and used. A system libffi will then not be used until an upgrade or a manual removal of the Sage installed version.

[tscrim]

comment:7

See https://groups.google.com/forum/?pli=1#!topic/sage-devel/PvcY2LHU1fo

[strogdon]

comment:8

Is there any reason why the patchbot emmits:

configure:8323: error: possibly undefined macro: AC_SEARCH_LIBS
configure:8324: error: possibly undefined macro: AC_CHECK_HEADERS

Shouldn't it have autotools installed?

[embray]

comment:9

On OSX this "works" okay, or doesn't work as the case may be, in that it (correctly) fails to find pkg-config, and thus does not try to use pkg-config.

I am seeing some other troubling problems failing to detect a system libffi on OSX, but I think whatever that problem is predates this ticket.

[embray]

comment:10

I have no idea about the patchbot. It is probably missing some dependency for running autoconf. Maybe this ticket needs a configure.tar.gz package, though it really should't...

[embray]

Reviewer: Erik Bray

[embray]

comment:12

Let's just ask Volker and see.

[sagetrac-git]

Changed commit from 4d9ccb4 to 3d2eb36

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

761c978	AC_CHECK_HEADERS should come before AC_SEARCH_LIBS
3d2eb36	move ffi/ffi.h first since it seems to be more common too

[embray]

comment:14

With apologies for being a bit sloppy about it, here are a couple more fixes I found while testing this on OSX.

[dimpase]

comment:16

just to confirm that libffi's spkg-configure.m4 on this branch works with the pretty non-standard settings on Gentoo, cf. #27175.

[dimpase]

comment:17

However, supporting this properly on Homebrew really needs a call to pkg-config.
(as there ffi.h really goes to a weird location).

[vbraun]

comment:18

Fails ./bootstrap on kucalc (autoconf-2.13), maybe the script could be made more backward compatible?

make[1]: warning: -jN forced in submake: disabling jobserver mode.
configure.ac:308: installing 'config/compile'
configure.ac:105: installing 'config/config.guess'
configure.ac:105: installing 'config/config.sub'
configure.ac:68: installing 'config/install-sh'
configure.ac:68: installing 'config/missing'
configure.ac:13: error: possibly undefined macro: dnl
      If this token and others are legitimate, please use m4_pattern_allow.
      See the Autoconf documentation.
configure:9028: error: possibly undefined macro: AC_CHECK_HEADERS
configure:9029: error: possibly undefined macro: AC_SEARCH_LIBS
Makefile:197: recipe for target 'configure' failed
make[1]: Leaving directory '/var/lib/buildbot/slave/sage_git/build'
make[1]: *** [configure] Error 1
Makefile:31: recipe for target 'base-toolchain' failed
make: *** [base-toolchain] Error 2
make: Target 'start' not remade because of errors.

[dimpase]

comment:21

Replying to @vbraun:

This is Ubuntu 16.04.5 LTS. Sorry I had the wrong autoconf version (multiple are installed)

vbraun@kucalc:~$ autoconf --version
autoconf (GNU Autoconf) 2.69

But the error messages are from a very old autoconf...
What is the problem then, what "needs_work"?

Are you saying you want a check that autoconf is new enough, and better error messages?

[embray]

comment:22

Volker, please clarify: Should this just have a new configure tarball? There's no reason for this to fail unless that machine is missing some dependencies to build the configure script, or some of the other tooling is misconfigured. I cannot reproduce the same issue.

[embray]

comment:23

Okay, I was able to reproduce in a container. Probably something is missing but I'm not sure what...

[embray]

comment:24

I see. These errors indicate that pkg-config is not installed, since it is required to have the PKG_CHECK_MODULES macro.

I could put in a fallback so that it works when PKG_CHECK_MODULES is not defined, but that would still build to a partially-crippled configure script. So can we just require pkg-config to be installed to build the configure script?

Alternatively, it might work if I include a copy of pkg.m4 in our m4/ directory, but I would rather not have to.

[embray]

comment:25

Replying to @embray:

Alternatively, it might work if I include a copy of pkg.m4 in our m4/ directory, but I would rather not have to.

Update: this does work, but it's not ideal. I would rather just have pkg-config installed.

[dimpase]

comment:26

If this dependence is really prebuilt- only, then why not, I see no problem in requiring it along with autotools

[vbraun]

comment:27

I don't mind requiring pkgconfig to actually run autoconf, but having bootstrap error out and not download the confball is not an option ;-)

[dimpase]

comment:28

Please see #27219 to deal with the bootstrap issue.

[vbraun]

Changed dependencies from #27109 to #27109, #27219

[dimpase]

Changed reviewer from Erik Bray to Erik Bray, Dima Pasechnik

[dimpase]

comment:30

Erik, are you fine with this?

[dimpase]

Changed branch from u/embray/build/ticket-27109 to public/build/better_libffi_config

[dimpase]

Changed commit from 3d2eb36 to 07e5561

[embray]

comment:31

Yep, per my comment on the other ticket. Thanks!

[vbraun]

Changed branch from public/build/better_libffi_config to 07e5561

[dimpase]

Changed commit from 07e5561 to none