Opened 5 years ago

Last modified 3 years ago

#22626 closed enhancement

Upgrade to GAP 4.9 — at Version 14

Reported by: nthiery Owned by:
Priority: critical Milestone: sage-8.6
Component: interfaces Keywords: days85, libgap
Cc: alexk, dimpase, embray, fbissey, arojas, gh-sebasguts, jpflori, markuspf, nthiery, slelievre, vbraun, wstein, gh-timokau Merged in:
Authors: Nicolas M. Thiéry, ... Reviewers:
Report Upstream: N/A Work issues: Wait for gap 4.9 release
Branch: u/nthiery/upgrade_to_gap_4_9 (Commits, GitHub, GitLab) Commit: 3011ac0908d667c0f245ca21859e336511106b5f
Dependencies: Stopgaps:

Status badges

Description (last modified by nthiery)

GAP 4.9 will come with a completely rewritten build system that will simplify our packaging. In fact, it may well enable Sage to use a vanilla GAP installation as provided by the distribution. It will also enable building GAP as a library, so that we can get rid of our separate libgap package.

The branch attached to this ticket updates Sage to run on top of a branch of GAP by Markus Pfeiffer that adds libgap compilation and |will be merged soon in the devel version of GAP.

Tentatively, all tests pass!

What the branch does:

  • Remove the libgap spkg
  • Update the gap spkg to the new build system and build and install libgap
  • Replace gap.shi.patch by a plain gap startup script for Sage

Rationale: GAP used to provide a startup shell script. The GAP devs are in the process of getting rid of it and provide a very minimal one. They recommend to just write our own rather than patching it.

  • Update a few doctests w.r.t. changes of output of some GAP functions
  • Possibly controversial: The new libgap currently *does not come* with symbol rewriting (Foo -> libGAP_Foo). This avoids messing around with GAP's sources; in particular opening the door for using a stock GAP from the OS distribution. However there always is a risk of name conflict. And indeed, GAP's constants (actually cpp macros) T_INT, T_FLOAT, ... conflict with Python's constants. This is worked around by duplicating and forcing their value in gap_include.pxd. Another conflict in objects.h currently needs to be worked around by hand (see below). Something similar was started at #19915.
  • Removes configure.patch: it was patching configure for better GMP detection under Cygwin (#13954). This should not be needed anymore with the new build system and use of --with-gmp. If it is, upstream asked for it to be reported and they will fix it.
  • Revert #19726 (not needed anymore)

TODO:

  • Automatic handling of headers (see below for how to do it by hand). GAP's build system will eventuall provide a rule to install headers which will make this trivial.
  • Update the documentation in sage.libs.gap.libgap.pyx to not mention the libgap_ prefix
  • Check against #19915 to see if any of the changes there should be ported here. Then close as won't fix.
  • ???

Fetching Markus's GAP sources:

    git clone git@github.com:markuspf/gap.git $LIBGAP
    cd $LIBGAP
    git remote add markuspf git@github.com:markuspf/gap.git
    git fetch markuspf
    git checkout -b markuspf/hpc-merge-libgap
    ./autogen.sh
    ./configure
    make bootstrap-pkg-minimal

Testing libgap:

    ./configure --enable-libgap
    make -j4 libgap
    make test-libgap

Build and install a tardist for Sage, and rebuild the spkg:

    make distclean
    ./autogen.sh
    ./configure
    make manuals
    make clean

    (cd ..; tar zcvf $SAGE/upstream/$GAP.tar.gz --exclude .git $GAP)

    sage --package fix-checksum
    sage -f gap                      # -s

Header files:

  • Copy GAP's header files, as well as gen/config.h to $SAGE/local/include
  • Fix them to adapt the include path: #include <src/...> -> #include <gap/...>
  • Replace T_INT by 0 in TNUM_OBJ, around line 414 of objects.h

Run:

    sage -b

Basic tests on libgap:

    sage: libgap.eval("GAPInfo.Version")
    sage: libgap.DihedralGroup(10).CharacterTable()
    CharacterTable( <pc group of size 10 with 2 generators> )
    sage: libgap.Group(libgap.eval("[(1,2,3),(1,2)]")).Size()
    6

Running most relevant tests:

    sage -tp 8 sage/groups sage/libs/gap

Current status: all tests pass!

Testing packages with dynamic loading (e.g. IO):

Install IO:

    cd $SAGE/local/gap/latest/pkg
    wget http://www.gap-system.org/pub/gap/gap4/tar.gz/packages/io-4.4.6.tar.gz
    tar xvf /tmp/io-4.4.6.tar.gz
    mv io-4.4.6 io
    cd io
    ./configure
    make

Test it locally:

    cd ../..
    ./gap -l .
    gap> LoadPackage("IO");
    true

This does not yet work:

    sage: libgap.LoadPackage("IO")
    ValueError: libGAP: Error, module '/opt/sage-git/local/gap/latest/pkg/io/bin/x86_64-pc-linux-gnu-gcc-default64/io.so' not found

This should be fixed once GAP's gap binary is built on top of libgap. See: https://github.com/markuspf/gap/issues/1.

Change History (14)

comment:1 Changed 5 years ago by nthiery

  • Description modified (diff)

comment:2 Changed 5 years ago by nthiery

  • Description modified (diff)

comment:3 Changed 5 years ago by nthiery

  • Description modified (diff)

comment:4 Changed 5 years ago by nthiery

  • Description modified (diff)

comment:5 Changed 5 years ago by nthiery

  • Description modified (diff)

comment:6 Changed 5 years ago by nthiery

  • Branch set to u/nthiery/upgrade_to_gap_4_9

comment:7 Changed 5 years ago by git

  • Commit set to 3011ac0908d667c0f245ca21859e336511106b5f

Branch pushed to git repo; I updated commit sha1. New commits:

1c645e0#22626: remove GAP's symbol prefixing in libgap: libGAP_Foo -> Foo
1ecc67b#22626: doctest update w.r.t. minor changes of output in GAP
e8ebbca#22626: GMP detection patch for cygwin should not be needed anymore
fd65b06#22626: Remove libgap spkg
9511733#22626: replace patch for GAP's startup script template in favor of a custom script
550625e#22626: remove GAP's symbol prefixing in libgap: libGAP_Foo -> Foo, and workaround GAP <-> Python symbol conflict
a9c859c#22626: updated gap spkg w.r.t. GAP's devel version and its new build system; also include compilation and installation of libgap
3011ac0Merge branch 'develop' into t/22626/upgrade_to_gap_4_9

comment:8 Changed 5 years ago by nthiery

  • Description modified (diff)

comment:9 Changed 5 years ago by nthiery

  • Description modified (diff)

comment:10 Changed 5 years ago by nthiery

  • Description modified (diff)

comment:11 Changed 5 years ago by nthiery

  • Description modified (diff)

comment:12 Changed 5 years ago by nthiery

  • Description modified (diff)

comment:13 Changed 5 years ago by nthiery

  • Cc vbraun dimpase wstein added
  • Keywords days85 libgap added
  • Work issues set to Wait for gap 4.9 release

comment:14 Changed 5 years ago by nthiery

  • Description modified (diff)
Note: See TracTickets for help on using tickets.