Allow libffi snapshots
ClosedPublic

Authored by angerman on May 11 2017, 9:38 PM.

Details

Summary

This is rather annoying. I'd prefer to have a stable release to
use. However libffi-3.2.1 has been released November 12, 2014, and
libffi-4 is TBD. See also https://github.com/libffi/libffi/issues/296

The core reason for this change is that llvm changed the supported
assembly to unified syntax, which libffi-3.2.1 does not use, and hence
fails to compile for arm with llvm. For refence, see the following
issue: https://github.com/libffi/libffi/issues/191.

This diff contains a script to generate a tarball for the libffi-tarballs
repository from the libffi GitHub repository; as well as the necessary
changes to the build system.

There are a very large number of changes, so older changes are hidden. Show Older Changes

@bgamari I guess I need some CI help with this.

bgamari edited edge metadata.May 12 2017, 10:35 AM
configure.ac:187: error: possibly undefined macro: LT_SYS_SYMBOL_USCORE

O_O

I have no idea what is going on here. I'll have to try to reproduce this locally.

angerman updated this revision to Diff 12556.May 13 2017, 8:32 AM
  • [libffi] fix autoreconf
  • [libffi] fix up
angerman updated this revision to Diff 12567.May 13 2017, 8:16 PM
  • [libffi] drop doc
angerman updated this revision to Diff 12568.May 13 2017, 9:31 PM
  • Revert "[libffi] drop doc"
angerman updated this revision to Diff 12569.May 13 2017, 11:34 PM
  • send help
angerman updated this revision to Diff 12570.May 13 2017, 11:44 PM
  • fix leftover
angerman updated this revision to Diff 12571.May 13 2017, 11:54 PM
  • path fix

linux ci fails with

autoreconf: Entering directory `.'
1036	autoreconf: configure.ac: not using Gettext
1037	autoreconf: running: aclocal -I m4
1038	autoreconf: configure.ac: tracing
1039	autoreconf: running: libtoolize --copy
1040	libtoolize: `/usr/share/aclocal/ltdl.m4' does not exist.
1041	autoreconf: libtoolize failed with exit status: 1

However, libltdl-dev-2.4.2-1.7ubuntu1 should have provided that file.

angerman updated this revision to Diff 12572.May 14 2017, 12:17 AM
  • ship ltdl.m4
angerman updated this revision to Diff 12573.May 14 2017, 1:25 AM
  • bump diff
erikd added a comment.May 14 2017, 4:55 AM

*If* we add this, it should only be a temporary measure. It should never be part of the source distribution and the source distribution should *always* work with a recent *release* version.

In D3574#101801, @erikd wrote:

*If* we add this, it should only be a temporary measure. It should never be part of the source distribution and the source distribution should *always* work with a recent *release* version.

Yes this is supposed to be a temporary solution to work around a temporary(?) deficiency in the upstream library. This would therefor be dropped as soon as a proper libffi release is cut, and if that hasn't happened by the time a new ghc version is released we need to figure out how to find a permanent solution that doesn't break our llvm code gen for 32 bit arm.

I tried running autogen.sh instead; it appears to work.

libffi/ghc.mk
66

Rather run ./autogen.sh here

angerman updated this revision to Diff 12581.May 14 2017, 7:51 PM
  • Try with autogen.sh
dpkg -l '*libtool*'
dpkg -l '*libltdl*'

results in

Desired=Unknown/Install/Remove/Purge/Hold
1655	| Status=Not/Inst/Conf-files/Unpacked/halF-conf/Half-inst/trig-aWait/Trig-pend
1656	|/ Err?=(none)/Reinst-required (Status,Err: uppercase=bad)
1657	||/ Name                                Version                           Architecture Description
1658	+++-===================================-=================================-============-===============================================================================
1659	ii  libtool                             2.4.2-1.7ubuntu1                  amd64        Generic library support script
1660	un  libtool-bin                         <none>                            <none>       (no description available)
1661	un  libtool-doc                         <none>                            <none>       (no description available)
1662	un  libtool1.4                          <none>                            <none>       (no description available)
1663	Desired=Unknown/Install/Remove/Purge/Hold
1664	| Status=Not/Inst/Conf-files/Unpacked/halF-conf/Half-inst/trig-aWait/Trig-pend
1665	|/ Err?=(none)/Reinst-required (Status,Err: uppercase=bad)
1666	||/ Name                                Version                           Architecture Description
1667	+++-===================================-=================================-============-===============================================================================
1668	un  libltdl-dev                         <none>                            <none>       (no description available)
angerman updated this revision to Diff 12589.May 14 2017, 10:09 PM
  • Drop CI debug
  • rebase

I hear something about a new dependency. What is it and why is it needed?

I hear something about a new dependency. What is it and why is it needed?

Recent llvm versions only support the unified assembly syntax for arm. This is incompatible with the latest
release of libffi (see https://github.com/libffi/libffi/issues/191). Requests for a new libffi release (the last
one has been in 2014) have been unanswered for quite some time now. (See https://github.com/libffi/libffi/issues/296,
and more recently https://sourceware.org/ml/libffi-discuss/2017/msg00002.html).

Therefore to be able to build and test ghc on arm via the llvm backend, and only until a proper libffi
release is cut, the proposal is to add libffi as a submodule to the master branch.

Obtaining a proper libffi release, is still of high priority, yet preventing ghc to compile to arm via llvm,
seems like a rather big defect to me.

I do not advocate that this ends up being the final state in a proper release, yet, I'd really like to
unbreak the development branch of GHC.

That's helpful but wasn't what I wanted to know about. What's this stuff about libtool?

That's helpful but wasn't what I wanted to know about. What's this stuff about libtool?

Ahh. Ok. libffis configure.ac does

LT_SYS_SYMBOL_USCORE
if test "x$sys_symbol_underscore" = xyes; then
    AC_DEFINE(SYMBOL_UNDERSCORE, 1, [Define if symbols are underscored.])
fi

these days.

This is usually part of libtool, however ubuntu, splits libtool int libtool and libtldl-dev, where the latter package contains the tldl.m4 macro that contains the LT_SYS_SYMBOL_USCORE macro.

So when libffi eventually does have a new release will we still need the new dependency libtldl-dev to build GHC?

So when libffi eventually does have a new release will we still need the new dependency libtldl-dev to build GHC?

Unless they ship the m4, macros, or somehow inline them, I do believe we do.

If we are willing to add that dependency, we could probably kill off the following code in aclocal.m4 as well, and replace it with the LT check

# FP_LEADING_UNDERSCORE
# ---------------------
# Test for determining whether symbol names have a leading underscore. We assume
# that they _haven't_ if anything goes wrong. Sets the output variable
# LeadingUnderscore to YES or NO and defines LEADING_UNDERSCORE correspondingly.
#
# Some nlist implementations seem to try to be compatible by ignoring a leading
# underscore sometimes (eg. FreeBSD). We therefore have to work around this by
# checking for *no* leading underscore first. Sigh.  --SDM
#
# Similarly on OpenBSD, but this test doesn't help. -- dons
#
AC_DEFUN([FP_LEADING_UNDERSCORE],
[AC_CHECK_LIB([elf], [nlist], [LIBS="-lelf $LIBS"])
AC_CACHE_CHECK([leading underscore in symbol names], [fptools_cv_leading_underscore], [
# Hack!: nlist() under Digital UNIX insist on there being an _,
# but symbol table listings shows none. What is going on here?!?
case $TargetPlatform in
    # Apples mach-o platforms use leading underscores
    *-apple-*) fptools_cv_leading_underscore=yes;;
    *linux-android*) fptools_cv_leading_underscore=no;;
    *openbsd*) # x86 openbsd is ELF from 3.4 >, meaning no leading uscore
      case $build in
        i386-*2\.@<:@0-9@:>@ | i386-*3\.@<:@0-3@:>@ ) fptools_cv_leading_underscore=yes ;;
        *) fptools_cv_leading_underscore=no ;;
      esac ;;
    i386-unknown-mingw32) fptools_cv_leading_underscore=yes;;
    x86_64-unknown-mingw32) fptools_cv_leading_underscore=no;;
    *) AC_RUN_IFELSE([AC_LANG_SOURCE([[#ifdef HAVE_NLIST_H
#include <nlist.h>
struct nlist xYzzY1[] = {{"xYzzY1", 0},{0}};
struct nlist xYzzY2[] = {{"_xYzzY2", 0},{0}};
#endif

int main(argc, argv)
int argc;
char **argv;
{
#ifdef HAVE_NLIST_H
    if(nlist(argv[0], xYzzY1) == 0 && xYzzY1[0].n_value != 0)
        exit(1);
    if(nlist(argv[0], xYzzY2) == 0 && xYzzY2[0].n_value != 0)
        exit(0);
#endif
    exit(1);
}]])],[fptools_cv_leading_underscore=yes],[fptools_cv_leading_underscore=no],[fptools_cv_leading_underscore=no])
;;
esac]);
AC_SUBST([LeadingUnderscore], [`echo $fptools_cv_leading_underscore | sed 'y/yesno/YESNO/'`])
if test x"$fptools_cv_leading_underscore" = xyes; then
   AC_DEFINE([LEADING_UNDERSCORE], [1], [Define to 1 if C symbols have a leading underscore added by the compiler.])
fi])# FP_LEADING_UNDERSCORE
erikd added a comment.May 15 2017, 3:46 PM

So when libffi eventually does have a new release will we still need the new dependency libtldl-dev to build GHC?

No we won't.

You are confused about the difference between using libffi from git (which will be missing auto-generated files and m4 macros) and a libffi tarball whcih should include auto-generated files and m4 macros.

In D3574#101948, @erikd wrote:

So when libffi eventually does have a new release will we still need the new dependency libtldl-dev to build GHC?

No we won't.

You are confused about the difference between using libffi from git (which will be missing auto-generated files and m4 macros) and a libffi tarball whcih should include auto-generated files and m4 macros.

Ah I see, thanks.

In that case I would prefer that we simply not do this. Adding a new dependency is a big pain for CI as you already discovered, but also a small pain for a large number of developers. If this is only going to be a temporary measure anyways then I'd rather avoid going through all of this.

Those few developers who need a recent build of libffi can (while we wait for a libffi release) build it themselves and specify the location at configure time with --with-system-libffi, --with-ffi-includes and --with-ffi-libraries, right?

Alright.

Why do we ship libffi in the first place?

angerman abandoned this revision.May 15 2017, 7:50 PM
angerman reclaimed this revision.Sep 29 2017, 2:15 AM
angerman updated this revision to Diff 14196.Sep 29 2017, 8:04 AM
  • package up libffi

As we saw last time, using the libffi submodule results in the requirement for additional dependencies. Specifically libtool, which some distributions like to split up as well.

We now have a custom libffi snapshot script, that creates a libffi-<revision>.tar.gz file from the upstream git repository. This should work with our libffi tarball system
with almost no required modifications.

The only modification is the line in the ghc.mk file, as the header are no more found in lib/libffi-<version>/include, but in include instead.

Of course this patch only contains the "logic", not the updated tarball, for which access to the libffi-tarballs repository is required.

Of course this patch only contains the "logic", not the updated tarball, for which access to the libffi-tarballs repository is required.

This looks reasonable and I'm willing to merge it. However, could you update the diff title and description?

angerman retitled this revision from Add libffi via submodule to Allow libffi snapshots.Sep 29 2017, 6:23 PM
angerman edited the summary of this revision. (Show Details)
angerman updated this revision to Diff 14202.Sep 30 2017, 1:41 AM
  • bump submodule
hvr requested changes to this revision.Sep 30 2017, 3:20 AM
hvr added a subscriber: hvr.
hvr added inline comments.
libffi-tarballs
1 ↗(On Diff #14202)

This ought to point to an orphan branch

This revision now requires changes to proceed.Sep 30 2017, 3:20 AM
hvr added inline comments.Sep 30 2017, 3:26 AM
utils/libffi/make-libffi-snapshot.sh
18

This also needs to construct a better versioning; a name like libffi-93d8e7d.tar.gz is too opaque. Ideally, we'd have something like a Debian-style versioning for git snapshots; like e.g.

libhybris 0.1.0+git20151016+6d424c9

where 0.1.0 denotes the version announced by the code, a git-date inferred from the most recent commit, and a git hash

angerman updated this revision to Diff 14203.Sep 30 2017, 5:54 AM
  • Better script
angerman marked an inline comment as done.Sep 30 2017, 5:57 AM
angerman added inline comments.
utils/libffi/make-libffi-snapshot.sh
18

this now ends up being libffi-3.99999+git20170927+93d8e7d.tar.gz.

18

The stupid mktemp -d logic. is to make sure we don't have install.sh in ., .., or ../...

This revision was automatically updated to reflect the committed changes.