sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.45k stars 482 forks source link

giac: Make cliquer a dependency, libnauty an optional dependency #31403

Closed mkoeppe closed 2 years ago

mkoeppe commented 3 years ago

giac uses libnauty if it finds it, so it should be an optional dependency.

Also, giac uses cliquer if it finds it, see https://groups.google.com/g/sage-release/c/R-8gvElDIUo/m/tHvIEdaQAAAJ

We add these missing dependencies.

Follow up: if libnauty is not configured to be installed (nor present as a system package), then we should disable its use by giac. The corresponding test in giac's configure script is not robust - it explicitly checks in /usr/local; which may be disabled on macOS using -isysroot.

CC: @kiwifb @dimpase @wdjoyner @sagetrac-parisse @jamesjer

Component: packages: standard

Author: Matthias Koeppe

Branch/Commit: 06c4df1

Reviewer: Dima Pasechnik

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

mkoeppe commented 3 years ago
comment:1

Sage development has entered the release candidate phase for 9.3. Setting a new milestone for this ticket based on a cursory review of ticket status, priority, and last modification date.

mkoeppe commented 2 years ago

Branch: u/mkoeppe/giac__make_cliquer_a_dependency__libnauty_an_optional_dependency

mkoeppe commented 2 years ago

Commit: 06c4df1

mkoeppe commented 2 years ago

Author: Matthias Koeppe

mkoeppe commented 2 years ago

New commits:

06c4df1build/pkgs/giac/dependencies: Add cliquer, add libnauty as an optional dep
mkoeppe commented 2 years ago

Description changed:

--- 
+++ 
@@ -1,4 +1,8 @@
 giac uses `libnauty` if it finds it, so it should be an optional dependency.

-Also, if `libnauty` is not configured to be installed (nor present as a system package), then we should disable its use by `giac`. The corresponding test in `giac`'s configure script is not robust - it explicitly checks in `/usr/local`; which may be disabled on macOS using `-isysroot`.
+Also, giac uses `cliquer` if it finds it, see https://groups.google.com/g/sage-release/c/R-8gvElDIUo/m/tHvIEdaQAAAJ

+We add these missing dependencies.
+
+Follow up: if `libnauty` is not configured to be installed (nor present as a system package), then we should disable its use by `giac`. The corresponding test in `giac`'s configure script is not robust - it explicitly checks in `/usr/local`; which may be disabled on macOS using `-isysroot`.
+
kiwifb commented 2 years ago
comment:10

This is just wrong. libcliquer is not a direct dependency of giac, it only depends on it through nauty.

$ readelf -d /usr/lib64/libgiac.so

Dynamic section at offset 0x1377818 contains 42 entries:
  Tag        Type                         Name/Value
 0x0000000000000001 (NEEDED)             Shared library: [libntl.so.43]
 0x0000000000000001 (NEEDED)             Shared library: [libpari-gmp-tls.so.7]
 0x0000000000000001 (NEEDED)             Shared library: [libgsl.so.27]
 0x0000000000000001 (NEEDED)             Shared library: [liblapack.so.3]
 0x0000000000000001 (NEEDED)             Shared library: [libblas.so.3]
 0x0000000000000001 (NEEDED)             Shared library: [libnauty.so.2]
 0x0000000000000001 (NEEDED)             Shared library: [libcurl.so.4]
 0x0000000000000001 (NEEDED)             Shared library: [libglpk.so.40]
 0x0000000000000001 (NEEDED)             Shared library: [libpng16.so.16]
 0x0000000000000001 (NEEDED)             Shared library: [libecm.so.1]
 0x0000000000000001 (NEEDED)             Shared library: [libmpfi.so.0]
 0x0000000000000001 (NEEDED)             Shared library: [libmpfr.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libgmp.so.10]
 0x0000000000000001 (NEEDED)             Shared library: [libstdc++.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libm.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libc.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [ld-linux-x86-64.so.2]
 0x0000000000000001 (NEEDED)             Shared library: [libgcc_s.so.1]
 0x000000000000000e (SONAME)             Library soname: [libgiac.so.0]

No libcliquer above. But

$ readelf -d /usr/lib64/libnauty.so

Dynamic section at offset 0x76d08 contains 29 entries:
  Tag        Type                         Name/Value
 0x0000000000000001 (NEEDED)             Shared library: [libcliquer.so.1]
 0x0000000000000001 (NEEDED)             Shared library: [libc.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [ld-linux-x86-64.so.2]
 0x000000000000000e (SONAME)             Library soname: [libnauty.so.2]

libcliquer is present here. Of course when you look at ldd's output you see all the libraries that are needed in a transitive fashion, so libcliquer shows up there. But libcliquer doesn't appear to be directly linked to libgiac or giac for that matter.

mkoeppe commented 2 years ago
comment:11

But giac's configure script checks for it, as you can see in David's log file - https://06677265663247414541.googlegroups.com/attach/90d611c87bb4/giac-1.6.0.47p3.p0.log?part=0.1&view=1&vt=ANaJVrGLaLydupV_kOaooQ8utUp20bny9N8_ERIh8lxI5JIpJksif7pfvvxlCWp667Mh0l22SddAo_Dtwb0WEJnPhTnzx-6oedhUoxLfNsIYiz-YAzemBrQ

kiwifb commented 2 years ago
comment:12

I am just looking at the configure script for giac

AC_CHECK_LIB(cliquer,main) 
AC_CHECK_LIB(nauty,main)
AC_CHECK_HEADERS(nauty/naututil.h)

It is all automagic. I am fairly sure the libcliquer detection is there so that when nauty is found the libcliquer flags are properly included too. Notice how no headers are checked for cliquer, just adding the library for linking.

kiwifb commented 2 years ago
comment:13

Another point is that the giac actively uses HAVE_LIBNAUTY in some code but HAVE_LIBCLIQUER is only present in configure objects. So nauty is optional and cliquer is just its dependency.

mkoeppe commented 2 years ago
comment:14

Our nauty package also does not declare a dependency on cliquer. The dependency has to go somewhere

kiwifb commented 2 years ago
comment:15

We definitely need to add cliquer to nauty then.

mkoeppe commented 2 years ago
comment:16

Replying to @kiwifb:

Another point is that the giac actively uses HAVE_LIBNAUTY in some code but HAVE_LIBCLIQUER is only present in configure objects. So nauty is optional and cliquer is just its dependency.

This certainly makes sense, given upstream nauty's inadequate build system.

mkoeppe commented 2 years ago
comment:17

Replying to @kiwifb:

We definitely need to add cliquer to nauty then.

Actually it looks like upstream nauty (and thus our package) uses a vendored copy of cliquer.

mkoeppe commented 2 years ago
comment:18

So on systems with nauty linking to an unvendored cliquer (Debian?), I think it will be nondeterministic whether the GIAC build picks up our libcliquer or system libcliquer.

mkoeppe commented 2 years ago
comment:19

Adding the dependency will remove this nondeterminism.

kiwifb commented 2 years ago
comment:20

You are posting too fast for me to make coherent answers :) I guess removing the non-deterministic behavior is the best course of action. debian, gentoo, probably fedora and arch, are unvendoring. I don't know about others or anaconda and brew.

mkoeppe commented 2 years ago
comment:21

Sorry - I should take a break

kiwifb commented 2 years ago
comment:22

It doesn't help that it is 9:50am here and I am in the middle of meetings and trying to help people with my "day job" at the same time.

dimpase commented 2 years ago
comment:24

This misses the issue of system-wide nauty, which should only be used if it's linked to libcliquer, no? E.g. on Debian stable I see

$ ldd /usr/lib/x86_64-linux-gnu/libnauty.so
        linux-vdso.so.1 (0x00007ffe223f8000)
        libcliquer.so.1 => /lib/libcliquer.so.1 (0x00007e4c9cd43000)
        libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007e4c9cb7e000)
        /lib64/ld-linux-x86-64.so.2 (0x00007e4c9cfe4000)

but apparently this is not the case on Ubuntu 21.

kiwifb commented 2 years ago
comment:25

If ubuntu 21 uses a vendored version of cliquer, it doesn't have to be linked to an external library.

That's one of the issue we have here. Some distribution will use external cliquer while some other will not. That's why the branch is just going with "let's just make sure it is always present".

dimpase commented 2 years ago
comment:26

Replying to @kiwifb:

If ubuntu 21 uses a vendored version of cliquer, it doesn't have to be linked to an external library.

Huh? The problem is that nauty must be linked to libcliquer (vendored or nor), not the other way around.

kiwifb commented 2 years ago
comment:27

OK my phrasing may not have been the best. If ubuntu 21's nauty uses a vendored libcliquer - like the sage optional package does - it may not need libcliquer.

If ubuntu 21's nauty does use an external libcliquer and it isn't installed along nauty then it is a bug in that version of ubuntu.

dimpase commented 2 years ago
comment:28

Replying to @kiwifb:

OK my phrasing may not have been the best. If ubuntu 21's nauty uses a vendored libcliquer - like the sage optional package does - it may not need libcliquer.

If it used a vendored libcliquer we won't be seeing this bug reported. According to https://ubuntu.pkgs.org/21.10/ubuntu-universe-amd64/libnauty2_2.7r1+ds-2_amd64.deb.html libcliquer is "Required".

If ubuntu 21's nauty does use an external libcliquer and it isn't installed along nauty then it is a bug in that version of ubuntu.

cc-ing Fedora's people.

mkoeppe commented 2 years ago
comment:30

https://groups.google.com/g/sage-support/c/YfyNeyKupH4

dimpase commented 2 years ago
comment:31

lgtm

dimpase commented 2 years ago

Reviewer: Dima Pasechnik

mkoeppe commented 2 years ago
comment:32

Thanks!

vbraun commented 2 years ago

Changed branch from u/mkoeppe/giac__make_cliquer_a_dependency__libnauty_an_optional_dependency to 06c4df1