add GAP io and crypting pkgs

[dimpase]

These GAP packages extend GAP kernel functionality.
On the libgap side, this requires tinkering with the loader, see
on #22626 Erik proposed a patch.

Here is a branch that implements this.

We also need to fix GAP's gac/libtool mini-toolchain, used by some GAP pkgs. This involves patching of gac, and installing libtool, done in #27218.

Depends on #27218
Depends on #27230
Depends on #27380

CC: @embray @kiwifb @timokau @jhpalmieri @AndrewAtLarge

Component: packages: optional

Author: Dima Pasechnik, Erik Bray

Branch/Commit: c84753f

Reviewer: Erik Bray

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

[dimpase]

comment:1

on OSX the dlopen call does not work. However, it does if given full path to the dll, i.e.

--- a/src/sage/libs/gap/util.pyx
+++ b/src/sage/libs/gap/util.pyx
@@ -263,7 +263,8 @@ cdef initialize():
     # Note: we could use RTLD_NOLOAD and avoid the subsequent dlclose() but
     # this isn't portable
     cdef void* handle
-    handle = dlopen("libgap.so", RTLD_NOW | RTLD_GLOBAL)
+    # handle = dlopen("libgap.so", RTLD_NOW | RTLD_GLOBAL)
+    handle = dlopen("/Users/dima/sagetrac-mirror/local/lib/python2.7/site-packages/sage/libs/gap/libgap.so", RTLD_NOW | RTLD_GLOBAL)
     if handle == NULL:
         raise RuntimeError(
                 "Could not dlopen() libgap.so even though it should already "

name resolution procedures are different on OSX. Actually, I don't know what's happening on Linux so that libgap.so is found.
Perhaps, it's already loaded on Linux, but not on OSX?

[embray]

comment:2

That's not really the intended "libgap.so". I meant the one from GAP itself, in $SAGE_LOCAL/lib/libgap.so. The re-dlopening the Python module might do it as well.

The other option, as I mentioned, would be to use Python's built-in sys.setdlopenflags(...), but that has to be called before importing any modules (e.g. sage.lib.gap.libgap) that are linked with libgap. It would probably be good afterwards to set it back to its default which is RTLD_LAZY.

But yes, it's probably more portable either way to give a full path to the relevant shared object.

[dimpase]

comment:3

Replying to @embray:

That's not really the intended "libgap.so". I meant the one from GAP itself, in $SAGE_LOCAL/lib/libgap.so.

oh, right --- except it's .dylib, not .so. (I searched for libgap.so, absent-mindedly, found unique match, tried it :-))

And indeed simply replacing libgap.so with libgap.dylib in the dlopen call suffices,
no need for the full path.

So this is easy to fix - except that in C I would have gone for a macro with the right extension, how is one supposed to do this in Cython?

[simon-king-jena]

comment:4

Your plan isn't clear from the ticket description. Is your plan to add IO to the gap_packages optional package? By the way: Could you remind me of the current status? I.e., is database_gap now part of gap_packages, or is it still independent? Is it supposed to be standard soon?

[dimpase]

comment:5

Replying to @simon-king-jena:

Your plan isn't clear from the ticket description. Is your plan to add IO to the gap_packages optional package? By the way: Could you remind me of the current status? I.e., is database_gap now part of gap_packages, or is it still independent? Is it supposed to be standard soon?

database_gap spkg is there no more, it's part of gap spkg; in fact, already
on #22626.
On #26856 Erik prefers to keep gap_packages optional, at least for the upcoming (quick) release 8.6, to make Debian deadline of 12 Jan.

We have not included IO there, as it'd be an extra over the current gap_packages, and something that needs more testing. Then we can extend gap_packages (and merge everything into gap, as we are already using the same tarball for gap and gap_packages anyway).

[simon-king-jena]

comment:6

And now a question on the IO package: From its documentation, I see that it allows to pickle any GAP object into a file object. But that's cumbersome for serialisation in Sage.

In Sage, we want a __reduce__ method (or a different serialisation protocol) to return a data tuple that is eventually dumped to a string by standard Python tools. But it seems wrong to me if we have a complicated object (say, a cohomology ring) with several attributes, one of them being defined in libgap, and to write only this attribute directly to a file.

So, how do you envision libgap serialisation?

[dimpase]

comment:7

Would you rather prefer IO dumping to a string? (i.e. into memory, right?)
I'm not sure, but perhaps this is doable via their streams; it sounds like a reasonable feature to add to IO, if it's not there yet.

As Python serialisation was never my concern, I have little idea about it.

[simon-king-jena]

comment:8

Replying to @dimpase:

As Python serialisation was never my concern, I have little idea about it.

This is a SageMath ticket. So, Python serialisation has to come into play sooner or later.

Having IO installed and working is a nice first step. But eventually,

sage: G = libgap.DihedralGroup(8)
sage: loads(dumps(G))

should work. And also something like the following should work:

class Foo(Ring):
    def __init__(self, G):
        assert G in Groups()
        self.G = G
        ...
    def __reduce__(self):
        return Foo, self.G

and then

sage: loads(dumps(Foo(libgap.DihedralGroup(8))))

But that would be for two follow-up tickets. One for pickling, the other for making libgap aware of Sage's categories (thus, translating G in Groups() into bool(G.IsGroup())).

[sagetrac-git]

Changed commit from 3b47f26 to eed5de7

[sagetrac-git]

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

35abd34	Merge branch 'develop' of trac.sagemath.org:sage into gap410pkg
db3f63e	more libGAP in as_permutation(), less pexpect mess
2d77eee	follow up pyflake hints (all but one)
7dd5fa6	reviewer's example and fix
ca05b24	Merge branch 'u/dimpase/groups/better_as_permutation' of trac.sagemath.org:sage into gap410pkg
bd849fb	don't clean too early
ff7722e	suppress "#I.." warnings
bf40575	silencing of #I warning from GAP in the function body
37e2329	tagged # random - answer is always mathematically OK, outputs differ
eed5de7	inital support for IO, following Erik on #22626

[dimpase]

comment:10

rebased over the branch on #26856, added OSX (and perhaps even Cygwin?) support.

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

5f49332	fix sdh_install of symlinks with invalid targets
9854c3a	Merge branch 'u/embray/build/sdh-install-symlinks' into public/ticket-26856
963757f	don't clean too early
da38d2c	tagged # random - answer is always mathematically OK, outputs differ
d8b912d	inital support for IO, following Erik on #22626
0e6f33c	IO has landed, no need for this workaround
8bf1044	disable doctesting of these modules
a7c5ccd	Merge branch 'public/ticket-26856' of trac.sagemath.org:sage into gap_io
c12daa6	Merge branch 'u/dimpase/packages/gap_io' of trac.sagemath.org:sage into gap_io
9328111	no need to suppress warining

[sagetrac-git]

Changed commit from eed5de7 to 9328111

[sagetrac-git]

Changed commit from 9328111 to 20c676a

[sagetrac-git]

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

20c676a

Merge remote-tracking branch 'trac/develop' into HEAD

[dimpase]

comment:14

merged 8.6.rc0 in.

[embray]

comment:15

Retarging tickets optimistically to the next milestone. If you are responsible for this ticket (either its reporter or owner) and don't believe you are likely to complete this ticket before the next release (8.7) please retarget this ticket's milestone to sage-pending or sage-wishlist.

[jdemeyer]

comment:17

Minor comment: if you are changing the spkg-install script of a package, you should also bump the patchlevel to force a rebuild.

[jdemeyer]

comment:18

Since dlopen takes a const char*, it's safer to use byte literals here:

    for suffix in [b"so", b"dylib", b"dll"]:
        handle = dlopen(b"libgap." + suffix, RTLD_NOW | RTLD_GLOBAL)

[jdemeyer]

Author: Dima Pasechnik, Erik Bray

[jdemeyer]

Changed dependencies from #22626, #26856 to none

[jdemeyer]

comment:21

Copying a file into $SAGE_LOCAL like this is very fishy. At least it needs to be better explained why it's needed:

cp -f "$SAGE_LOCAL/include/gap/config.h" "$GAP_ROOT/src/"

[jhpalmieri]

comment:86

Seems to work on OS X. At least there is more stuff (io, crypting) in local/share/gap/pkg with this branch than without it. I'm not sure what the breakage was from #27380: gap_packages installed for me on top of a clean install of 8.7.beta5 without these branches.

What else would you like me to check?

[dimpase]

comment:87

Replying to @jhpalmieri:

Seems to work on OS X. At least there is more stuff (io, crypting) in local/share/gap/pkg with this branch than without it. I'm not sure what the breakage was from #27380: gap_packages installed for me on top of a clean install of 8.7.beta5 without these branches.

but some GAP packages were not fully functional, as their binaries (only few GAP packages have these) were installed in wrong directories...

What else would you like me to check?

could you run tests, just to have another data point? (I tested on OSX 10.13, I don't have 10.14, my OSX box is too old for it).

[jhpalmieri]

comment:88

All tests pass for me. I didn't run full tests with gap_packages but without this branch, but I ran limited tests and saw one failure in sage/groups. So this looks good to me.

[AndrewMathas]

comment:89

I don't have a great internet connection at the moment but I can test on macosx 10.14.2 by Monday, if not earlier. This said, since this is a package can some one please tell me what needs to be tested. Is sage -i io && sage -i macosx && make ptestlong enough?

[dimpase]

comment:90

Replying to @AndrewAtLarge:

I don't have a great internet connection at the moment but I can test on macosx 10.14.2 by Monday, if not earlier. This said, since this is a package can some one please tell me what needs to be tested. Is sage -i io && sage -i macosx && make ptestlong enough?

You just need to pull the branch, the GAP tarball is the same. So can be done over GSM :-)

After merging the branch, you'll need to do

./sage -f gap_packages && make ptestlong

And if there are errors in tests, please posts them (from logs/ptestlong.log file).
Thanks!

[embray]

comment:91

On Cygwin I'm getting a few cases of this failure in src/sage/coding/linear_code.py:

sage -t --long src/sage/coding/linear_code.py
**********************************************************************
File "src/sage/coding/linear_code.py", line 3134, in sage.coding.linear_code.AbstractLinearCode.weight_distribution
Failed example:
    C.weight_distribution(algorithm="leon")   # optional - gap_packages (Guava package)
Exception raised:
    Traceback (most recent call last):
      File "/home/embray/src/sagemath/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 671, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/embray/src/sagemath/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 1095, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.coding.linear_code.AbstractLinearCode.weight_distribution[8]>", line 1, in <module>
        C.weight_distribution(algorithm="leon")   # optional - gap_packages (Guava package)
      File "sage/misc/cachefunc.pyx", line 1950, in sage.misc.cachefunc.CachedMethodCaller.__call__ (build/cythonized/sage/misc/cachefunc.c:10265)
        w = self._instance_call(*args, **kwds)
      File "sage/misc/cachefunc.pyx", line 1826, in sage.misc.cachefunc.CachedMethodCaller._instance_call (build/cythonized/sage/misc/cachefunc.c:9750)
        return self.f(self._instance, *args, **kwds)
      File "/home/embray/src/sagemath/sage/local/lib/python2.7/site-packages/sage/coding/linear_code.py", line 3184, in weight_distribution
        lines = subprocess.check_output([os.path.join(guava_bin_dir, 'wtdist'), input])
      File "/home/embray/src/sagemath/sage/local/lib/python2.7/subprocess.py", line 223, in check_output
        raise CalledProcessError(retcode, cmd, output=output)
    CalledProcessError: Command '['/home/embray/src/sagemath/sage/local/share/gap/pkg/guava-3.14/bin/x86_64-unknown-cygwin-default64/wtdist', '/home/embray/.sage/temp/NAVI-Brick/6764/tmp_7ZlhoX::code']' returned non-zero exit status -11

The "exit status -11" indicates that the wtdist program, whatever that is, from the guava package is segfaulting. Which is odd because I don't remember having any problem with that before, and it's not clear why it would start with this ticket (unless maybe it's doing something with the IO module that it didn't do when it was unavailable).

I'm going to try backing out the changes from this ticket and rebuilding gap_packages again just to rule out the possibility that the problem is somehow related to this ticket.

[dimpase]

comment:92

we didn't build this part of guava before, that's why...

[embray]

comment:93

I don't see anything explicitly in this ticket that would cause it to start being built though, so I'm a little confused. Is it because we didn't have #27218? Does it just, silently not build or something?

[embray]

comment:94

AFAICT the code that uses it doesn't check if the wtdist program exists or not or anything like that, so it should have just failed before with a command not found or something if it wasn't being built...

[embray]

comment:95

Well I was able to reproduce that issue with gap_packages prior to this ticket so I guess it's unrelated. I'll make a new ticket for it. I'm not overly concerned about it in this case since gap_packages is not installed by default anyways.

As a side note, I realized in testing this that it this ticket should also bump the patch version on the SPKG version.

We should also clean up the commit history. I can go ahead and do that...

[dimpase]

comment:96

oops, sorry, I meant it was not installed before GAP 4.10.
I gather it could be something trivial like whether or not there is .exe suffix...

Yes, please, bump up the version and rebase. The follow-up to this is #27295.

[embray]

comment:97

Okay, I'm fine with this now, especially since you tested on OSX.

---

New commits:

ece3d59	extra / breaks sdh_install on OSX
77a2f32	Trac #26930: add setting GAP_SO to env.py
985e63a	Trac: #26930: add GAP IO and crypting packages
bb77ab1	Trac #26903: Bump package version

[embray]

Reviewer: Erik Bray

[embray]

Changed commit from 6160039 to bb77ab1

[embray]

Changed branch from u/dimpase/gap_io to public/gap_io

[sagetrac-git]

Changed commit from bb77ab1 to c84753f

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. This was a forced push. New commits:

7e85cc6	Trac #26930: add GAP IO and crypting packages
c84753f	Trac #26903: Bump package version

[embray]

comment:99

Just reworded one of the commit messages.

[vbraun]

Changed branch from public/gap_io to c84753f