fingolfin / gap

GAP - Groups, Algorithms, Programming - a System for Computational Discrete Algebra
http://www.gap-system.org
GNU General Public License v2.0
0 stars 0 forks source link

Add compatibility mode so that existing packages with kernel extensions can be used #5

Closed markuspf closed 7 years ago

markuspf commented 8 years ago

From TODO file: add fake GAP arch for compatibility mode

fingolfin commented 7 years ago

We have to essentially cater for two kinds of packages: Those using autoconf and m4/ac_find_gap.m4; and those using gac. For the latter, things are actually easier because gac is a shell script and we can modify it to do our bidding.

So I'll focus on those using m4/ac_find_gap.m4. There are two setups to support:

A. The easy one is to support in-tree builds of GAP, i.e. where you simply run ./configure && make within the GAP source tree. B. The harder one is to support out-of-tree builds.

Let's consider case A first. Here is what AC_FIND_GAP does and how to deal with it:

  1. It looks for $GAPROOT/sysinfo.gap, which we therefore should add.
  2. It parses that file, and then looks for two more files: $GAPROOT/src/compiled.h and $GAPROOT/bin/$GAPARCH/config.h. The former is of course fine. To make the latter work, we could set GAPARCH to ../src; but it's probably better to set it to some fake value, and then symlink config.h.
  3. It looks for $GAPROOT/bin/$GAPARCH/extern/gmp/include/gmp.h to see if a bundled GMP was compiled; if so, it adds $GAPROOT/bin/$GAPARCH/extern/gmp/include to the list of include paths. To resolve this, add a suitable symlink.
  4. It sets GAP_CPPFLAGS="-I$GAPROOT -I$GAPROOT/bin/$GAPARCH". Nothing needs to be done.

Now let's consider case B. This is a bit more complicated. Suppose the GAP source tree is in REAL_GAPROOT, but we chose to run configure out of tree, in directory GAPROOT. We then later point the package's configure script at GAPROOT. We need to deal with the same four points:

  1. $GAPROOT/sysinfo.gap is again no problem (indeed, we can just let configure create it for us from sysinfo.gap.in, and it will automatically end up in the right place
  2. Ensuring that config.h is found is again trivial. But now we also need to worry about src/compiled.h. Let's postpone that for the moment
  3. Finding GMP: same as above
  4. To get the include flags right, we could try to set GAPARCH to a "crazy" value so that $GAPROOT/bin/$GAPARCH ends up pointing at REAL_GAPROOT. But some packages will use GAPARCH for other things. So again it's better to work with a symlink.

So for steps 2 and 4, we need to ensure that the GAP header files from REAL_GAPROOT/src/*.h can also be found under GAPROOT/src/. We can solve this in multiple ways, with different pros and cons:

i. Symlink the directory REAL_GAPROOT/src to GAPROOT/src. But we currently output config.h and .o files into GAPROOT/src. Solution: Output the .o files to e.g. GAPROOT/obj and config.h to e.g. just GAPROOT (but then we need to adjust how we include it). Another drawback of this method: I am not sure it'll work on Windows. ii. Symlink the filesREAL_GAPROOT/src/.hintoGAPROOT/src(and the same forsrc/hpc/.h`). But be careful to add/remove symlinks whenever header files are added/removed. And again, Windows support might be a problem. iii. The same as in the previous approach, but copy (and delete) headers as appropriate. This would certainly work on Windows. However, this is very error prone and requires utmost care, lest a developer edits a file in the wrong place.

I kind of prefer approach i., as it seems to require the least work and attention to minute detail.

fingolfin commented 7 years ago

One problem shared by all these approaches, even the in-tree builds, is that in HPC-GAP mode, some of the .h files actually come from REAL_GAPROOT/hpcgap/src. We could try to workaround that, too, but I am tempted to say "well, for HPC-GAP, you just have to jump through some extra hoops until packages are adapted to the new scheme".

Here, "extra hoops" likely means that instead of running ./configure in a package, you have to run something like ./configure CFLAGS="-IREAL_GAPROOT/hpcgap/src -IREAL_GAPROOT/src". That's not super nice, but also not super horrible.

fingolfin commented 7 years ago

This seems to work with io so far. We should, however, test it with more packages.

fingolfin commented 7 years ago

I.e. perhaps we should have one Travis test which tries to build as many packages as possible with bin/BuildPackages.sh.

Since some packages may require external dependencies not installed on travis, they would fail; to avoid such failures from causing problems, we could simply rm -rf such packages, until bin/BuildPackages.sh completes...

@alex-konovalov perhaps you are interested in looking into doing that?

olexandr-konovalov commented 7 years ago

At the moment, it does not work at all - just starts from the last package:

$ ../bin/BuildPackages.sh 
Using GAP root /Users/alexk/GITREPS/gap-fingolfin
Attempting to build GAP packages.
Note that many GAP packages require extra programs to be installed,
and some are quite difficult to build. Please read the documentation for
packages which fail to build correctly, and only worry about packages
you require!

Tue 14 Mar 2017 09:36:34 GMT

==== Checking xmodalg-1.12
No building required for xmodalg-1.12

Packages which failed to build are in ./log/fail.log
olexandr-konovalov commented 7 years ago

That is more general problem, same script does the same on macOS in the master branch of the main GAP repository as well.

olexandr-konovalov commented 7 years ago

This is the full list of packages that fail to load on our Jenkins Linux slave in 64-bit mode:

olexandr-konovalov commented 7 years ago

Of course real issues here are browse and gauss because they need gac (see #39)

fingolfin commented 7 years ago

@alex-konovalov Can you run your checks again?

Perhaps we can also setup a single Travis test (say, in 64bit out-of-tree non-HPC mode) which tries to compile as many packages as possible using BuildPackages... hmmm

olexandr-konovalov commented 7 years ago

@fingolfin run checks based on this commit:

commit f393ee406b83466174a9439cb9ea43df3f0beca4
Author: Max Horn <max@quendi.de>
Date:   Fri Mar 17 20:21:55 2017 +0100

    Makefile.rules: add -no-undefined to libgap.la (needed on cygwin)

Browse build fails as below (64-bit, Linux)

Running './configure' '/data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/64build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot' 
Using config in /data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/64build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot/sysinfo.gap-default64
Created ./Makefile-default64 with link from ./Makefile
Running 'gmake' 
if test ! -d bin;  then mkdir bin;  fi
if test ! -d bin/x86_64-pc-linux-gnu-gcc-default64;  then mkdir -p bin/x86_64-pc-linux-gnu-gcc-default64;  fi
/data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/64build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot/bin/x86_64-pc-linux-gnu-gcc-default64/gac -p "" -d -o bin/x86_64-pc-linux-gnu-gcc-default64/ncurses.so \
        src/ncurses.c -L "-lpanel -lncurses"
/bin/sh /data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/64build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot/libtool --mode=compile gcc -std=gnu99 -o /tmp/gac18369/18369_ncurses.lo -I/data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/64build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot -I/data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/64build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot -DCONFIG_H -c src/ncurses.c
libtool: compile:  gcc -std=gnu99 -I/data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/64build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot -I/data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/64build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot -DCONFIG_H -c src/ncurses.c  -fPIC -DPIC -o /tmp/gac18369/.libs/18369_ncurses.o
src/ncurses.c:19:75: fatal error: src/string.h: No such file or directory
#include        "src/string.h"            /* need GAP strings           */
^
compilation terminated.
gmake: *** [bin/x86_64-pc-linux-gnu-gcc-default64/ncurses.so] Error 1

WARNING: Failed to build Browse

but gauss seems to build:

==== Checking Gauss
Running './configure' '/data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/64build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot' 
Using gac to compile.
If you do not want this, run this script with --without-gac.
Running 'gmake' 
if test ! -d bin;  then mkdir bin;  fi
if test ! -d bin/x86_64-pc-linux-gnu-gcc-default64;  then mkdir -p bin/x86_64-pc-linux-gnu-gcc-default64;  fi
/data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/64build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot/bin/x86_64-pc-linux-gnu-gcc-default64/gac -d -o bin/x86_64-pc-linux-gnu-gcc-default64/gauss.so src/gauss.c
/bin/sh /data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/64build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot/libtool --mode=compile gcc -std=gnu99 -o /tmp/gac30601/30601_gauss.lo -I/data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/64build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot -I/data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/64build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot -DCONFIG_H -c src/gauss.c
libtool: compile:  gcc -std=gnu99 -I/data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/64build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot -I/data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/64build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot -DCONFIG_H -c src/gauss.c  -fPIC -DPIC -o /tmp/gac30601/.libs/30601_gauss.o
/bin/sh /data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/64build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot/libtool --mode=link gcc -std=gnu99 -module -avoid-version -rpath /usr/local/lib -m64 -o bin/x86_64-pc-linux-gnu-gcc-default64/gauss.so /tmp/gac30601/30601_gauss.lo
libtool: link: gcc -std=gnu99 -shared  -fPIC -DPIC  /tmp/gac30601/.libs/30601_gauss.o    -m64   -Wl,-soname -Wl,gauss.so -o bin/x86_64-pc-linux-gnu-gcc-default64/.libs/gauss.so
libtool: link: ( cd "bin/x86_64-pc-linux-gnu-gcc-default64/.libs" && rm -f "gauss.la" && ln -s "../gauss.la" "gauss.la" )
rm -f /tmp/gac30601/30601_gauss.lo
olexandr-konovalov commented 7 years ago

In addition, 32-bit build of Browse fails too, with more warnings

==== Checking Browse
Running './configure' '/data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/32build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot' 
Using config in /data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/32build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot/sysinfo.gap-default32
Created ./Makefile-default32 with link from ./Makefile
Running 'gmake' 
if test ! -d bin;  then mkdir bin;  fi
if test ! -d bin/x86_64-pc-linux-gnu-gcc-default32;  then mkdir -p bin/x86_64-pc-linux-gnu-gcc-default32;  fi
/data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/32build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot/bin/x86_64-pc-linux-gnu-gcc-default32/gac -p "" -d -o bin/x86_64-pc-linux-gnu-gcc-default32/ncurses.so \
        src/ncurses.c -L "-lpanel -lncurses"
/bin/sh /data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/32build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot/libtool --mode=compile gcc -std=gnu99 -o /tmp/gac5608/5608_ncurses.lo -I/data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/32build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot -I/data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/32build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot -DCONFIG_H -c src/ncurses.c
libtool: compile:  gcc -std=gnu99 -I/data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/32build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot -I/data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/32build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot -DCONFIG_H -c src/ncurses.c  -fPIC -DPIC -o /tmp/gac5608/.libs/5608_ncurses.o
In file included from /data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/32build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot/src/compiled.h:20:0,
from src/ncurses.c:18:
/data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/32build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot/src/objects.h: In function 'prod_intobjs':
/data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/32build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot/src/objects.h:191:12: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
if (l == (Int)INTOBJ_INT(0) || r == (Int)INTOBJ_INT(0))
^
/data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/32build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot/src/objects.h:191:39: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
if (l == (Int)INTOBJ_INT(0) || r == (Int)INTOBJ_INT(0))
^
/data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/32build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot/src/objects.h:193:12: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
if (l == (Int)INTOBJ_INT(1))
^
/data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/32build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot/src/objects.h:194:12: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
return (Obj)r;
^
/data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/32build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot/src/objects.h:195:12: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
if (r == (Int)INTOBJ_INT(1))
^
/data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/32build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot/src/objects.h:196:12: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
return (Obj)l;
^
/data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/32build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot/src/objects.h:204:12: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
return (Obj) prod;
^
/data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/32build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot/src/objects.h:208:12: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
return (Obj) prod;
^
In file included from /data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/32build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot/src/compiled.h:33:0,
from src/ncurses.c:18:
/data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/32build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot/src/gmpints.h: At top level:
/data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/32build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot/src/gmpints.h:45:2: error: #error Aborting compile: unexpected GMP limb size
#error Aborting compile: unexpected GMP limb size
^
In file included from /data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/32build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot/src/compiled.h:20:0,
from src/ncurses.c:18:
/data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/32build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot/src/objfgelm.h: In function 'NewWord':
/data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/32build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot/src/objects.h:88:6: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
((Int)(o) >> 2)
^
/data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/32build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot/src/objfgelm.h:36:7: note: in expansion of macro 'INT_INTOBJ'
( INT_INTOBJ( ELM_PLIST( (type), AWP_NR_BITS_PAIR ) ) )
^
/data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/32build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot/src/objfgelm.h:128:47: note: in expansion of macro 'BITS_WORDTYPE'
word = NewBag(T_DATOBJ,2*sizeof(Obj)+npairs*BITS_WORDTYPE(type)/8L);
^
/data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/32build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot/src/objects.h:74:6: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
((Obj)(((UInt)(Int)(i) << 2) + 0x01))
^
/data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/32build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot/src/objfgelm.h:129:24: note: in expansion of macro 'INTOBJ_INT'
(ADDR_OBJ(word)[1] = INTOBJ_INT(npairs));
^
/data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/32build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot/src/objfgelm.h: In function 'RESIZE_WORD':
/data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/32build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot/src/objects.h:88:6: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
((Int)(o) >> 2)
^
/data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/32build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot/src/objfgelm.h:36:7: note: in expansion of macro 'INT_INTOBJ'
( INT_INTOBJ( ELM_PLIST( (type), AWP_NR_BITS_PAIR ) ) )
^
/data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/32build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot/src/objfgelm.h:69:7: note: in expansion of macro 'BITS_WORDTYPE'
( BITS_WORDTYPE( TYPE_DATOBJ( (word) ) ) )
^
/data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/32build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot/src/objfgelm.h:146:46: note: in expansion of macro 'BITS_WORD'
ResizeBag( (word), 2*sizeof(Obj)+((npairs)*BITS_WORD((word))/8L));
^
/data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/32build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot/src/objects.h:74:6: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
((Obj)(((UInt)(Int)(i) << 2) + 0x01))
^
/data/gap-jenkins/workspace/GAP-merge-test/GAPCOPTS/32build/GAPTARGET/packagesload/label/kovacs/GAP-merge-snapshot/src/objfgelm.h:147:26: note: in expansion of macro 'INTOBJ_INT'
(ADDR_OBJ((word))[1] = INTOBJ_INT((npairs)));
^
src/ncurses.c: At top level:
src/ncurses.c:19:75: fatal error: src/string.h: No such file or directory
#include        "src/string.h"            /* need GAP strings           */
^
compilation terminated.
gmake: *** [bin/x86_64-pc-linux-gnu-gcc-default32/ncurses.so] Error 1

WARNING: Failed to build Browse
fingolfin commented 7 years ago

Ah, right, my version of Browse is patched, and I removed some redundant includes, which is why it worked for me. In particular, I removed the string.h include -- and also removed src/string.h in GAP to avoid conflicts... So one fix for now would be to add src/string.h back. Of course it would be even better if there was a new Browse release.... cough.

fingolfin commented 7 years ago

Closing this: AFAIK an updated for Browse is planned; as for other packages, I am not aware of any build failures; if there are still any, these should be reported on the main GAP issue tracker.