Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add GAP's Semigroups package to gap_packages #27295

Closed
nthiery opened this issue Feb 15, 2019 · 54 comments
Closed

Add GAP's Semigroups package to gap_packages #27295

nthiery opened this issue Feb 15, 2019 · 54 comments

Comments

@nthiery
Copy link
Contributor

nthiery commented Feb 15, 2019

Semigroup depends on a number of GAP packages; io (cf. #26930), Digraphs (that uses bliss), orb, and genss.

So they all need to be added.

Depends on #27396
Depends on #28088

Upstream: Reported upstream. Developers acknowledge bug.

CC: @dimpase @isuruf

Component: interfaces

Keywords: GAP packages, semigroups

Author: Dima Pasechnik

Branch: 426edfe

Reviewer: Isuru Fernando

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

@nthiery nthiery added this to the sage-8.7 milestone Feb 15, 2019
@dimpase
Copy link
Member

dimpase commented Mar 1, 2019

Dependencies: #26930

@dimpase

This comment has been minimized.

@dimpase
Copy link
Member

dimpase commented Mar 1, 2019

comment:2

of these, only Semigroups doesn't want to get installed.

#W dlopen() error: libsemigroups.so.0: cannot open shared object file: No such file or directory
Error, module '/mnt/opt/Sage/sage-dev/local/share/gap/pkg/semigroups-3.0.20/bin/x86_64-pc-linux-gnu-default64/semigroups.so' not found in
  if not LOAD_DYN( arg[1], false ) then
    Error( "no support for dynamic loading" );
fi; at /mnt/opt/Sage/sage-dev/local/share/gap/lib/files.gd:583 called from 
<function "LoadDynamicModule">( <arguments> )
 called from read-eval loop at /mnt/opt/Sage/sage-dev/local/share/gap/pkg/semigroups-3.0.20/init.g:26
Error, was not in any namespace at /mnt/opt/Sage/sage-dev/local/share/gap/lib/variable.g:291 called from
LEAVE_NAMESPACE(  ); at /mnt/opt/Sage/sage-dev/local/share/gap/lib/package.gi:1253 called from
ReadPackage( pkgname, "init.g" ); at /mnt/opt/Sage/sage-dev/local/share/gap/lib/package.gi:1616 called from
<function "LoadPackage">( <arguments> )
 called from read-eval loop at *stdin*:1
...

And indeed:

$ find local/ -name semigroups.so
local/lib/gap/pkg/semigroups-3.0.20/bin/x86_64-pc-linux-gnu-default64/semigroups.so
$ find local/ -name libsemigroups.so
local/lib/gap/pkg/semigroups-3.0.20/bin/lib/libsemigroups.so
$ ldd `find local/ -name libsemigroups.so`
	linux-vdso.so.1 (0x00007ffd85de3000)
	libpthread.so.0 => /lib64/libpthread.so.0 (0x00007fa1ffc9e000)
	libstdc++.so.6 => /usr/lib/gcc/x86_64-pc-linux-gnu/8.3.0/libstdc++.so.6 (0x00007fa1ffa95000)
	libm.so.6 => /lib64/libm.so.6 (0x00007fa1ff91a000)
	libc.so.6 => /lib64/libc.so.6 (0x00007fa1ff74b000)
	libgcc_s.so.1 => /usr/lib/gcc/x86_64-pc-linux-gnu/8.3.0/libgcc_s.so.1 (0x00007fa1ff731000)
	/lib64/ld-linux-x86-64.so.2 (0x00007fa1ffd35000)
$ ldd `find local/ -name semigroups.so`
	linux-vdso.so.1 (0x00007ffd8e160000)
	libsemigroups.so.0 => not found
	libpthread.so.0 => /lib64/libpthread.so.0 (0x00007fd7baaf4000)
	libstdc++.so.6 => /usr/lib/gcc/x86_64-pc-linux-gnu/8.3.0/libstdc++.so.6 (0x00007fd7ba8eb000)
	libm.so.6 => /lib64/libm.so.6 (0x00007fd7ba770000)
	libc.so.6 => /lib64/libc.so.6 (0x00007fd7ba5a1000)
	libgcc_s.so.1 => /usr/lib/gcc/x86_64-pc-linux-gnu/8.3.0/libgcc_s.so.1 (0x00007fd7ba585000)
	/lib64/ld-linux-x86-64.so.2 (0x00007fd7bab88000)

@dimpase
Copy link
Member

dimpase commented Mar 1, 2019

comment:3

This is after applying

diff --git a/build/pkgs/gap_packages/spkg-install b/build/pkgs/gap_packages/spkg-install
index d8260b9aa0..4592675553 100644
--- a/build/pkgs/gap_packages/spkg-install
+++ b/build/pkgs/gap_packages/spkg-install
@@ -19,6 +19,7 @@ sdh_install \
     crystcat \
     design \
     gbnp \
+    genss-* \
     Hap* \
     HAPcryst \
     hecke-* \
@@ -76,7 +77,7 @@ done
 # These packages have a new-style autoconf ./configure
 # that takes --with-gaproot
 
-for pkg in nq-* io-*
+for pkg in nq-* io-* orb-* digraphs-* semigroups-*
 do
     cd "$PKG_SRC_DIR/$pkg"
     sdh_configure --with-gaproot="$GAP_ROOT"

to the branch of #26930.

@dimpase
Copy link
Member

dimpase commented Mar 1, 2019

comment:4

This is a sort of upstream issue - they install a libsemigroup, a C++ library, in the tree, etc etc. I've opened semigroups/Semigroups#580
---a clean way would be to install in advance, libsemigroup separately.

@dimpase
Copy link
Member

dimpase commented Mar 1, 2019

Upstream: Reported upstream. No feedback yet.

@dimpase
Copy link
Member

dimpase commented Mar 2, 2019

Changed upstream from Reported upstream. No feedback yet. to Reported upstream. Developers acknowledge bug.

@dimpase
Copy link
Member

dimpase commented Mar 2, 2019

comment:5

upstream welcomes PRs, so I'll try doing this in a proper way.

@dimpase
Copy link
Member

dimpase commented Mar 2, 2019

Changed dependencies from #26930 to #26930, #27396

@dimpase
Copy link
Member

dimpase commented Mar 2, 2019

Changed keywords from none to GAP packages, semigroups

@kiwifb
Copy link
Member

kiwifb commented Mar 4, 2019

comment:8

digraphs includes its own copy of bliss and just trying to do the right thing they just digraphs name_spaced bliss functions.

fbissey@moonloop ~/sandbox/gap-4.10.0/pkg/digraphs-0.13.0 $ diff -u /dev/shm/portage/sci-libs/bliss-0.73-r1/work/bliss-0.73/bliss_C.h src/bliss-0.73/bliss_C.h 
--- /dev/shm/portage/sci-libs/bliss-0.73-r1/work/bliss-0.73/bliss_C.h   2015-09-01 19:23:10.000000000 +1200
+++ src/bliss-0.73/bliss_C.h    2018-11-02 11:56:12.000000000 +1300
@@ -1,5 +1,5 @@
-#ifndef BLISS_C_H
-#define BLISS_C_H
+#ifndef bliss_digraphs_C_H
+#define bliss_digraphs_C_H
 
 /*
   Copyright (c) 2003-2015 Tommi Junttila
@@ -37,14 +37,14 @@
 /**
  * \brief The true bliss graph is hiding behind this typedef.
  */
-typedef struct bliss_graph_struct BlissGraph;
+typedef struct bliss_digraphs_graph_struct BlissGraph;
....

@dimpase
Copy link
Member

dimpase commented Mar 4, 2019

comment:9

I don't seem to have a problem with digraphs, it installs just fine, and uses a static copy of bliss, it seems.

Do you mean to say one has to watch out for something, still?

@kiwifb
Copy link
Member

kiwifb commented Mar 4, 2019

comment:10

Replying to @dimpase:

I don't seem to have a problem with digraphs, it installs just fine, and uses a static copy of bliss, it seems.

Do you mean to say one has to watch out for something, still?

No, just don't like it I guess.

@dimpase
Copy link
Member

dimpase commented Mar 7, 2019

comment:11

At least with semigroups, hopefully after my semigroups/Semigroups#584 is in, it can be done in a clean way.

@kiwifb
Copy link
Member

kiwifb commented Mar 7, 2019

comment:12

Replying to @dimpase:

At least with semigroups, hopefully after my semigroups/Semigroups#584 is in, it can be done in a clean way.

Looks good. It looks like that will have to wait gap-4.10.2 for it. I just finished updating all my sage-on-gentoo gap packages to offer 4.10.1 and I must say I was shocked when orb ditched autotools for a gac setup a la crypting which I think is an inferior solution. I hope it doesn't become a standard practise across gap packages.

@dimpase
Copy link
Member

dimpase commented Mar 19, 2019

comment:13

Upstream has https://github.com/gap-packages/Semigroups/tree/v3.1.2 which has all the needed changes. Should we make a separate optional package for this?

@embray
Copy link
Contributor

embray commented Mar 25, 2019

comment:14

Ticket retargeted after milestone closed (if you don't believe this ticket is appropriate for the Sage 8.8 release please retarget manually)

@embray embray modified the milestones: sage-8.7, sage-8.8 Mar 25, 2019
@embray
Copy link
Contributor

embray commented Jun 14, 2019

comment:15

As the Sage-8.8 release milestone is pending, we should delete the sage-8.8 milestone for tickets that are not actively being worked on or that still require significant work to move forward. If you feel that this ticket should be included in the next Sage release at the soonest please set its milestone to the next release milestone (sage-8.9).

@embray embray removed this from the sage-8.8 milestone Jun 14, 2019
@dimpase
Copy link
Member

dimpase commented Jul 1, 2019

Changed dependencies from #26930, #27396 to #27396, #28088

@dimpase dimpase added this to the sage-8.9 milestone Jul 1, 2019
@dimpase
Copy link
Member

dimpase commented Jul 1, 2019

Branch: u/dimpase/packages/semigroupsgap

@dimpase
Copy link
Member

dimpase commented Jul 1, 2019

Commit: 6fbdec9

@dimpase
Copy link
Member

dimpase commented Jul 1, 2019

New commits:

32aab07update to GAP 4.10.2
e1d5ab1add libsemigroups package
6fbdec9add GAP package semigroups and its deps

@isuruf
Copy link
Member

isuruf commented Jul 4, 2019

comment:24

Only patch needed by planarity is extern patch that sage already has.

Does this imply that this ticket is (abot) broken, as it adds Digraphs in an "old" way? How does one test?

Probably. Digraphs adds planarity as a subdirectory just like semigroups does with libsemigroups and therefore planarity inside digraphs is not installed. Since planarity is installed in sage and the two versions of planarity are ABI compatible, you might not see a missing planarity issue. This would break if planarity versions differ in Digraphs and Sage which is pretty unlikely since planarity changes rarely.

@dimpase
Copy link
Member

dimpase commented Jul 4, 2019

comment:25

Replying to @isuruf:

Only patch needed by planarity is extern patch that sage already has.

Does this imply that this ticket is (abot) broken, as it adds Digraphs in an "old" way? How does one test?

Probably. Digraphs adds planarity as a subdirectory just like semigroups does with libsemigroups and therefore planarity inside digraphs is not installed. Since planarity is installed in sage and the two versions of planarity are ABI compatible, you might not see a missing planarity issue. This would break if planarity versions differ in Digraphs and Sage which is pretty unlikely since planarity changes rarely.

You're right that Digraphs pick up Sage's libplanarity.so:

$ ldd local/lib/gap/pkg/digraphs-0.15.2/bin/x86_64-pc-linux-gnu-default64-kv3/digraphs.so
	linux-vdso.so.1 (0x00007fff9bdb7000)
	libplanarity.so.0 => /mnt/opt/Sage/sage-dev/local/lib/libplanarity.so.0 (0x00007f55c349d000)
	libstdc++.so.6 => /usr/lib/gcc/x86_64-pc-linux-gnu/9.1.0/libstdc++.so.6 (0x00007f55c3200000)
	libm.so.6 => /lib64/libm.so.6 (0x00007f55c30c3000)
	libc.so.6 => /lib64/libc.so.6 (0x00007f55c2ef3000)
	libgcc_s.so.1 => /usr/lib/gcc/x86_64-pc-linux-gnu/9.1.0/libgcc_s.so.1 (0x00007f55c2ed9000)
	/lib64/ld-linux-x86-64.so.2 (0x00007f55c350e000)

So in this sense the only thing to fix is to add planarity to dependencies of gap_packages. I've checked that Digraphs passes its own testsuite, just in case.

@embray
Copy link
Contributor

embray commented Jul 4, 2019

comment:26

Would still be best to use --with-external-planarity if possible. Or if nothing else, delete the vendored copy it builds before installing into Sage if we definitely don't need it.

@isuruf
Copy link
Member

isuruf commented Jul 4, 2019

comment:27

Since there hasn't been a release of Digraphs, it wouldn't work, but it's still okay to add --with-external-planarity and released Digraph's configure just ignores it and you wouldn't have to do anything when a new release comes in.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 4, 2019

Changed commit from d948da7 to cec604d

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 4, 2019

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

cec604dcorrect checksum for the tarball

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 4, 2019

Changed commit from cec604d to 2e6ec3c

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 4, 2019

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

2e6ec3cplanarity is a dependency of Digraph GAP package

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 4, 2019

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

6f88343libsemigroup and planarity are dependencies of Digraph and Semigroups GAP packages

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 4, 2019

Changed commit from 2e6ec3c to 6f88343

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 4, 2019

Changed commit from 6f88343 to 426edfe

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 4, 2019

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

426edfeconfigure params array for packages (for passing --with-... if needed)

@dimpase
Copy link
Member

dimpase commented Jul 4, 2019

comment:32

Replying to @isuruf:

Since there hasn't been a release of Digraphs, it wouldn't work, but it's still okay to add --with-external-planarity and released Digraph's configure just ignores it and you wouldn't have to do anything when a new release comes in.

I've added this, and missing deps, and the correct tarball checksum for libsemigroups from #27396. I didn't try to remove dead wood copy of planarity installed by
Digraph, as this appears to be harmless, and won't be needed once we get updated Digraph.

@kiwifb
Copy link
Member

kiwifb commented Jul 4, 2019

comment:33

Dear me, I forgot those horrors so fast. No only does Digraphs use plain "planarity" it also include its own vendored (and prefixed so it doesn't clash) version of bliss-0.73. I had some fun patching that in 0.13.0 and 0.15.0. No I may have to reproduce it for 0.15.2.

@isuruf
Copy link
Member

isuruf commented Jul 17, 2019

comment:35

This package vendors bliss. Is that okay?

@dimpase
Copy link
Member

dimpase commented Jul 17, 2019

comment:36

We know about vendored bliss. It builds it statically, so it should not be a show-stopper. It should be unvendored upstream.

Some of us actually do research in semigroup algorithms, so we want it in...

@isuruf
Copy link
Member

isuruf commented Jul 17, 2019

Reviewer: Isuru Fernando

@isuruf
Copy link
Member

isuruf commented Jul 17, 2019

comment:37

Do you mind creating a new trac ticket for unvendoring bliss?

@dimpase
Copy link
Member

dimpase commented Jul 17, 2019

comment:38

To be more precise, it's Digraphs, a dependency of Semigroups, that vendors bliss.

So this is in fact digraphs/Digraphs#205

@kiwifb
Copy link
Member

kiwifb commented Jul 17, 2019

comment:39

I have an unvendoring patch in sage-on-gentoo, but it is minimal. I pass LIBS="-lbliss -lplanarity -lgap" to configure, it should be improved to get configure to figure it itself. https://github.com/cschwan/sage-on-gentoo/blob/master/dev-gap/digraphs/files/digraphs-0.15.2-system_pkg.patch

@kiwifb
Copy link
Member

kiwifb commented Jul 17, 2019

comment:40

Improved unvendoring patch: https://github.com/cschwan/sage-on-gentoo/blob/master/dev-gap/digraphs/files/digraphs-0.15.2-system_pkg-r1.patch

configure now searches for the libraries by itself.

@vbraun
Copy link
Member

vbraun commented Aug 10, 2019

Changed branch from u/dimpase/packages/semigroupsgap to 426edfe

@embray
Copy link
Contributor

embray commented Aug 15, 2019

comment:42

Seems to work on Cygwin too, FWIW.

@embray
Copy link
Contributor

embray commented Aug 15, 2019

Changed commit from 426edfe to none

This was referenced Aug 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants